Conversation
|
|
||
| function FolderPage() { | ||
| return ( | ||
| <> |
| const PageWrapper = styled.div` | ||
| display: flex; | ||
| flex-direction: column; | ||
| min-height: 100vh; /* 페이지의 최소 높이를 화면의 높이로 설정 */ | ||
| `; | ||
|
|
||
| const PageDisplay = styled.div` | ||
| display: flex; | ||
| row-gap: 4rem; | ||
| padding: 4rem 0 10rem; | ||
| flex-direction: column; | ||
| margin: 0 auto; | ||
| width: 106rem; | ||
|
|
||
| @media (max-width: 1199px) { | ||
| margin-left: min(3.2rem) max(auto); | ||
| margin-right: min(3.2rem) max(auto); | ||
| } | ||
|
|
||
| @media (max-width: 1123px) { | ||
| width: 70.4rem; | ||
|
|
||
| @media (max-width: 767px) { | ||
| width: 32.5rem; | ||
| } | ||
| } | ||
| `; | ||
| const FooterWrapper = styled.footer` | ||
| margin-top: auto; /* 페이지 컨텐츠 아래에 위치하도록 설정 */ | ||
| `; |
There was a problem hiding this comment.
컴포넌트 파일에선 컴포넌트 코드가 제일 중요하다고 생각하는데요.
스타일드가 컴포넌트 파일 내에 존재하게 되면 본 로직에 접근성이 떨어지는 것 같아요.
파일을 분리하거나 컴포넌트 코드 밑에 배치하면 좋을 것 같습니다!
| <div className={S.idInputWrapper}> | ||
| <label className={S.idInputLabel}>이메일</label> | ||
| <IdInput placeholder={'이메일을 입력해 주세요'} /> | ||
| </div> |
There was a problem hiding this comment.
로그인, 회원가입 페이지에서 공통으로 사용되는 컴포넌트라 분리해서 재사용하면 좋을 것 같습니다.
ui 가 복잡하지 않은 페이지라 당장 개선할 사항은 아니라고 생각돼서 작업하실 때 주석으로 ui 구분만 해둬도 파악하기 좋을 것 같네요~
There was a problem hiding this comment.
유틸 함수로 구성돼있어서 hooks 가 아닌 utils 폴더 등으로 옮기면 어떨까요?
| e.currentTarget.src = '/thumbnail.svg'; | ||
| }; | ||
|
|
||
| function Cards({ url }: { url: string }) { |
There was a problem hiding this comment.
Cards 컴포넌트의 props 로 url 을 받기보다 links 를 받아서 렌더링하는게 어떨까요?
url 을 받아 fetch 해오는 건 이 컴포넌트가 해야하는 역할 이상인 것 같아요.
저는 보통 컴포넌트의 Props 설계할 때 이런식으로 생각합니다
Cards 컴포넌트의 역할은 뭐지? -> 카드 리스트를 렌더링 하기 -> ui 에 필요한 데이터를 props 로 받자
| <div className={styles.popover}> | ||
| <div className={styles.options}> | ||
| <button | ||
| onClick={() => handleDeleteClickButton(link.description)} | ||
| > | ||
| 삭제하기 | ||
| </button> | ||
| </div> | ||
| <div className={styles.options}> | ||
| <button onClick={handleClickButton}>폴더에 추가</button> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
여기도 컴포넌트로 분리하면 좋겠네요~
| const Container = styled.div` | ||
| display: flex; | ||
| justify-content: center; | ||
| align-items: center; | ||
| width: 100%; | ||
| height: 24.4rem; | ||
| padding: 2rem 0 6rem; | ||
| background-color: var(--light-blue); | ||
| `; |
There was a problem hiding this comment.
스타일드는 컴포넌트 함수 밖으로 빼주세요!
안에 있게 되면 렌더링 될 때마다 새로운 형태로 생성될 거라 성능상 좋지 않을 것 같습니다.
| const BASE_FOLDER_ID = 'all'; | ||
|
|
||
| interface Link { | ||
| id: string; | ||
| url: string; | ||
| title: string; | ||
| description: string; | ||
| } | ||
| interface Folder { | ||
| id: string; | ||
| name: string; | ||
| } | ||
|
|
||
| interface FolderResponse { | ||
| data: Folder[]; | ||
| } | ||
|
|
||
| const BASE_URL_FOLDER = 'https://bootcamp-api.codeit.kr/api/users/1/folders'; | ||
| const BASE_URL_ALL_FOLDER = 'https://bootcamp-api.codeit.kr/api/users/1/links'; |
There was a problem hiding this comment.
상수와 타입을 별도의 파일로 분리하기만 해도 가독성이 좋아질 것 같습니다.
| function handleCloseModal() { | ||
| setIsModalOpen(false); | ||
| } | ||
| function handleClickButton() { | ||
| setIsModalOpen(true); | ||
| } | ||
|
|
||
| function handleCloseShareModal() { | ||
| setIsModalShareOpen(false); | ||
| } | ||
| function handleClickShareButton() { | ||
| setIsModalShareOpen(true); | ||
| } | ||
|
|
||
| function handleCloseRenameModal() { | ||
| setIsModalRenameOpen(false); | ||
| } | ||
| function handleClickRenameButton() { | ||
| setIsModalRenameOpen(true); | ||
| } | ||
|
|
||
| function handleCloseDeleteModal() { | ||
| setIsModalDeleteOpen(false); | ||
| } | ||
| function handleClickDeleteButton() { | ||
| setIsModalDeleteOpen(true); | ||
| } |
There was a problem hiding this comment.
반복되는 로직은 유틸 함수나 커스텀 훅으로 분리해야한다는 신호로 생각할 수 있습니다.
모달 관련 로직을 아래처럼 커스텀 훅으로 분리해 재사용할 수 있을 것 같아요.
const useModal = () => {
const [open, setOpen] = useState(false);
const handleModal = {
open: () => setOpen(true),
close: () => setOpen(false);
}
return [open, handleModal];
}
// 컴포넌트 파일
const [isOpenShare, handleShareModal] = useModal();
// ...
There was a problem hiding this comment.
또 이 컴포넌트가 하는 일을 나열해보고 두개 이상이면 분리해야하는 신호로 생각해볼 수 있습니다.
모달 처리도 하고 검색어도 관리하고 url 관리도 하고 있네요..!
| <div className={`${styles.modal} ${isModalOpen ? styles.show : ''}`}> | ||
| <div className={styles.modal__content}> | ||
| <div className={styles.modal__content__header}>{title}</div> |
There was a problem hiding this comment.
모달 컴포넌트들 간에 반복되는 Ui 가 있는 것 같아요
분리해서 재사용하면 모달들 간의 ui 일관성을 챙길 수 있습니다.
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게