-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[오다은] Week15 #478
The head ref may contain hidden characters: "part3-\uC624\uB2E4\uC740-week15"
[오다은] Week15 #478
Conversation
…e.accessToken 유무로 동작하도록 수정
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
계속해서 개선하려는 태도가 훌륭하십니당
고생하셨습니다 👏
/> | ||
{folderNames?.map((folderName, index) => ( | ||
<MenuButton | ||
<MenuLink currentFolderId={currentFolderId || '0'} /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currentFolderId 값이 없을 때 기본값을 설정해주셨는데요.
값이 없을 땐 기본값으로 '0' 을 넣어야 한다는 사실은 MenuLink 내부를 봐야 알 수 있는 부분 같습니다.
그래서 currentFolderId 는 optional 로 두고 없을 때 내부에서 기본값을 설정할 수 있도록 처리하면 어떨까요?
useEffect(() => { | ||
handleFilterItems(); | ||
}, [searchText]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
serarchText 가 변경될 때마다 필터처리를 하고 계시네요~
디바운스를 적용하면 좋을 것 같아요.
아래 글 참고해보세요!
https://www.zerocho.com/category/JavaScript/post/59a8e9cb15ac0000182794fa
} else { | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else 문은 정리해줄 수 있겠네용
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
서비스 전반적으로 사용되는 인터페이스를 별도 파일로 분리해주셨네요 👍
title: string; | ||
image_source: string; | ||
}[]; | ||
items: LinkInterface[] | undefined; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
undefined 는
items?: LinkInterface[] 형태로 작성할 수도 있습니당
{isVisibleKebabModal && ( | ||
<S.LogoutButton onClick={handleLogout}>로그아웃</S.LogoutButton> | ||
)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
로그아웃 기능도 구현해주셨네요 👏
if (response.status < 200 || response.status >= 300) { | ||
throw new Error('사용자 데이터를 불러오는데 실패했습니다.'); | ||
let response; | ||
if (localStorage.accessToken) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
localStorage 를 로그로 출력해보니 객체 형태라 해당 문법도 잘 동작하는 것 같습니다.
문서를 보면 이렇게 접근하는게 표준이 아닌 것 같으니 안내하는 대로 getItem() 으로 접근하는게 좋을 것 같습니다!
https://developer.mozilla.org/en-US/docs/Web/API/Storage/getItem
setFolders(nextFolders); | ||
const nextFolderNames = nextFolders.map((item) => item.name); | ||
const nextItemCounts = nextFolders.map((item) => item.link.count); | ||
setFolderNames(nextFolderNames); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
folders 가 있는데 folderNames 상태를 추가로 가지고 있어야 하는지 잘 모르겠어서 사용처를 확인해봤습니다.
Card 컴포넌트에서 더보기를 띄울지 말지 여부를 결정하는 조건인 것 같은데 다른 요소로 대체할 수 있을 것 같습니당
let nextLinks; | ||
nextLinks = await getLinks( | ||
user.id, | ||
currentFolderId ? Number(currentFolderId) : 0 | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
분리해서 let 으로 선언할 필요가 없어 보입니다!
요구사항
기본
심화
주요 변경사항
멘토에게