Conversation
…ithub-actions [Fix] delete merged branch github action
[고학영] Week2
…-고학영-week7 [고학영] week7
…-고학영-week11 [고학영] week11
…-고학영-week12 [고학영] week12
hakyoung12
force-pushed
the
part3-고학영-week13
branch
from
c580fd0 to
d1f63f4
Compare
| "pages/folder.jsx", | ||
| "pages/folder.jsx", | ||
| "pages/folder.jsx", | ||
| "pages/folder.jsx" |
| "components/Input.jsx", | ||
| "pages/folder.jsx", | ||
| "pages/folder.jsx", | ||
| "pages/folder.jsx", | ||
| "pages/folder.jsx" |
There was a problem hiding this comment.
이 파일들을 include 에 명시해서 추가하신 이유가 있나요?
| "@svgr/webpack": "^8.1.0", | ||
| "@types/classnames": "^2.3.1", |
There was a problem hiding this comment.
이 패키지들은 개발 환경에서만 필요하고 실제 프로덕션 환경에선 없어도 잘 동작할 것 같아요
npm install 시 --save-dev 옵션을 살펴보시면 좋을 것 같습니다!
| /coverage | ||
|
|
||
| # production | ||
| /build |
There was a problem hiding this comment.
빌드 결과물로 생성되는 dist 폴더도 gitignore 에 추가해주세요~
There was a problem hiding this comment.
다른 페이지 컴포넌트의 파일명 컨벤션이 소문자로 시작하는 것 같습니다. 이 페이지도 맞춰주세요~
| import KebabMenu from "./KebabMenu"; | ||
| import Image from "next/image"; | ||
|
|
||
| interface CardListItemProps { |
There was a problem hiding this comment.
link, folder 의 타입도 다른 곳에서 정의한 인터페이스와 동일한 구조라면 하나로 관리해주세요~
비슷한 타입이 많은 것 같아요. 검색해서 일괄적으로 적용해주세요!
| <div className={styles.folderLinkList__folderEditBtns}> | ||
| <button | ||
| className={styles.folderLinkList__folderEditBtn} | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| setModalOpen("shareLink"); | ||
| }} | ||
| > | ||
| <div className={styles.folderLinkList__folderEditIcon}> | ||
| <Image fill src={shareBtn} alt="공유하기" /> | ||
| </div> | ||
| 공유 | ||
| </button> | ||
| <button | ||
| className={styles.folderLinkList__folderEditBtn} | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| setModalOpen("alterName"); | ||
| }} | ||
| > | ||
| <div className={styles.folderLinkList__folderEditIcon}> | ||
| <Image fill src={renameBtn} alt="이름변경" /> | ||
| </div> | ||
| 이름 변경 | ||
| </button> | ||
|
|
||
| <button | ||
| className={styles.folderLinkList__folderEditBtn} | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| setModalOpen("delete"); | ||
| }} | ||
| > |
There was a problem hiding this comment.
'전체'를 선택했을 때 이 부분만 표시되지 않는다면 조건부 렌더링되는 곳을 좁힐 수 있을 것 같아요.
<타이틀 />
{showTotal && <버튼들 />}
<카드리스트 />
이런식으로요!
| {modalOpen === "addFolder" && ( | ||
| <EditAndAddFolder | ||
| madalTitle={"폴더 추가"} | ||
| onClose={setModalOpen} |
There was a problem hiding this comment.
onClose props 에 setModalOpen 을 넘겨서 모달을 닫는다는게 직관적이지 않아요.
살펴보니 modalOpen 에 빈 문자열을 넣거나 유효한 몇몇 문자열 외의 값을 넣어야 모달 컴포넌트가 언마운트 되는 것 같네요.
EditAndAddFolder 컴포넌트 내부를 보니 onClose 에 빈 문자열을 전달해서 닫게 되는데 이 말은 '빈 문자열을 전달해서 닫는다' 라는 지식을 EditAndAddFolder 가 알고 있다는 거고, 그렇게 되면 컴포넌트를 넘나들며 확인해야해서 유지보수가 어려워지는 경우가 많습니다.
간단히는 handleClose 같은 함수를 만들어 명시적으로 처리하는 게 좋을 것 같습니다.
그후에 퍼져있는 모달 관련 로직들을 관리하는 커스텀 훅을 만들면 좋을 것 같아요!
| <button | ||
| className={styles.folderLinkList__addFolderButton__mobile} | ||
| onClick={(e) => { | ||
| e.preventDefault(); | ||
| setModalOpen("addFolder"); | ||
| }} | ||
| > | ||
| 폴더 추가 | ||
| <div className={styles.folderLinkList__addFolderIcon}> | ||
| <Image fill src={addBtnMobile} alt="폴더추가" /> | ||
| </div> | ||
| </button> |
There was a problem hiding this comment.
플로팅 형태의 폴더 추가 버튼 컴포넌트도 분리하면 어떨까요?
css mediaquery 로 표시 여부를 처리하는 것도 좋은데 모바일 사이즈임을 체크하는 커스텀 훅을 만들어 사용하면 더 파악하기 좋은 코드가 될 것 같아요.
커스텀 훅은 멘토링 시간에 다뤄보면 좋을 것 같네요!
{isMobile && <FloatingButton />}
| <button | ||
| className={ | ||
| folder.name === title | ||
| ? styles.folderLinkList__folder__active | ||
| : styles.folderLinkList__folder | ||
| } | ||
| id={id?.toString()} | ||
| key={folder.id} | ||
| onClick={() => { | ||
| handleTitle(folder.name, folder.id); | ||
| }} | ||
| > | ||
| {folder.name} | ||
| </button> |
There was a problem hiding this comment.
폴더 선택 버튼 컴포넌트를 분리해서 재사용하면 어떨까요?
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게