Conversation
There was a problem hiding this comment.
root에서 hook, component, layout(?)을 나뉘어서 관리하는 것이 좋을 것 같아요! 이렇게되면 아무래도 전체에서 공통으로 쓰이는 컴포넌트 및 hook들을 관리하기 어려워져서요 ㅎㅎ src/components, src/hooks 이런식은 어떨까요?
| const closeModal = () => setCurrentModal(null); | ||
| const handleKeyDown = (event) => { | ||
| if (event.key === "Escape") { | ||
| closeModal(); | ||
| } | ||
| }; | ||
| const handleKakaoClick = () => { | ||
| shareKakao({ url: shareLink, ...KAKAO_SHARE_DATA }); | ||
| }; | ||
| const handleFacebookClick = () => | ||
| window.open(`http://www.facebook.com/sharer.php?u=${shareLink}`); | ||
| const handleLinkCopyClick = () => copyToClipboard(shareLink); |
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [folderId]); | ||
|
|
||
| const mapDataFormat = ({ id, created_at, url, image_source, title, description }) => ({ |
There was a problem hiding this comment.
convertFormat~~ 과 같이 동적인 함수로 명명을 정의하면 어떨까요?
|
|
||
| useEffect(() => { | ||
| execute(); | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
여기가 조금 애매하다면
if (folderId) execute();
이렇게 작성하면 어떨까요?
| const { data: folders } = useGetFolders(); | ||
| const cardListRef = useRef(null); | ||
| const [selectedFolderId, setSelectedFolderId] = useState(null); | ||
| const [currentModal, setCurrentModal] = useState(null); |
There was a problem hiding this comment.
이 부분이 항상 나오는데 이를 useModal로 빼보면 어떨까요~? onOpen, onClose 등을 리턴하는 훅으로요.
| <EditableCard | ||
| key={link?.id} | ||
| {...link} | ||
| popoverPosition={getPopoverPosition(index)} |
There was a problem hiding this comment.
호출이 각 카드가 렌더링 될 때마다 일어나는 것이 좀 안좋을 것 같아요! 특히 위에서 state가 바뀌면 계속 호출이 일어나는 것이어서요.
해당 함수를 EditableCard안에 useCallback으로 정의하면 어떨까요~?
| selectedLinkUrl={selectedLinkUrl} | ||
| selectedFolderId={selectedFolderId} | ||
| setSelectedFolderId={setSelectedFolderId} | ||
| onAddClick={() => {}} |
There was a problem hiding this comment.
해당 callback을 빈 부분으로 넘기는 것이 어떤 이유일까요~?
| const handleKeyDown = (event) => { | ||
| if (event.key === "Escape") { | ||
| closeModal(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
handlekeyDown안에서 closeModal하는 부분도 항상 존재하네요! 이 부분도 useModal안으로 옮겨도 좋겠네요.
| cardList={ | ||
| <CardList> | ||
| {links?.map((link) => ( | ||
| <ReadOnlyCard key={link?.id} {...link} /> |
There was a problem hiding this comment.
optional한 값이라면 아예 index로 key를 설정해도 좋을 것 같아요!
| if (onBackdropClick) { | ||
| onBackdropClick(event); | ||
| } |
There was a problem hiding this comment.
onBackdropClick?.(event) 라고 써도 될 것 같아요!
요구사항
기본
주요 변경사항
스크린샷
멘토에게