Conversation
There was a problem hiding this comment.
컴포넌트를 큰 단위로 분리해주셔서 파악하기가 쉬워졌습니다. 👍
큰 단위로는 복잡도를 줄이는 것만으로도 의미가 있는 것 같습니다.
ui 단위로 분리한 뒤 오류 때문에 추가했던 props 들을 살펴보시고 꼭 외부에서 주입받아야 하는지 검토해보시면 좋을 것 같아요.
커다란 폼 컴포넌트 내부 작은 단위로 분리하는 것에 대한 것도 코멘트 남겼지만
ui 파악이 쉬운지 어려운지에 따라서 분리를 하고 props 관리가 어려워지는 점이 있다면 어떻게 해결할지 고민해보시면 좋을 것 같습니다.
고생하셨습니다 👏
| <Searchbar | ||
| onSubmit={handleSubmit} | ||
| onChange={handleChange} | ||
| onClose={handleClose} | ||
| inputValue={inputValue} | ||
| /> |
| {params === "/signup" ? ( | ||
| <div>다른 방식으로 가입하기</div> | ||
| ) : ( | ||
| <div>소셜 로그인</div> | ||
| )} |
There was a problem hiding this comment.
params 에 의존해서 텍스트를 관리하게 되면 경로 텍스트가 변경됐을 때 등 상황에서 유지보수가 어려울 것 같아요.
이 컴포넌트에 props 로 텍스트를 받으면 어떨까요?
| src="/images/google.png" | ||
| alt="google" | ||
| className="p-[10px] w-[42px] h-[42px]" |
There was a problem hiding this comment.
절대적으로 지켜야하는 건 아니지만 속성 순서 배치도 생각해보면 좋을 것 같아요!
개인적으론 className 이나 id 를 상단에 두는 편입니당
생각해보니 그런 속성들은 동작을 파악하는데 크게 의미있는 부분이 아니라 코드를 위에서 아래로 볼 때 아래쪽에 배치한 것들 위주로 보기 때문인 것 같아요
https://github.com/necolas/idiomatic-html?tab=readme-ov-file#attribute-order
There was a problem hiding this comment.
className, id 를 상단에 두도록 하겠습니다!
| setInputType((prevType) => (prevType === "text" ? "password" : "text")); | ||
| }; | ||
|
|
||
| return ( |
There was a problem hiding this comment.
로그인 페이지에서 인풋이 하나 추가됐다고 많이 길어졌네용
컴포넌트를 분리하는 기준이 사람마다 다를테지만 저의 경우 jsx 를 보고 '머릿속에 어떤 요소들이 들어있는지 떠올리는게 어렵지 않은 정도' 로 볼 수 있을 것 같네요
추상적이지만.. 로그인 페이지는 파악하기 어렵지 않았고 이 페이지는 파악이 어렵네요 ㅜ
인풋별로 컴포넌트를 분리하고 재사용하는 것도 좋을 것 같아요!
There was a problem hiding this comment.
넵 확인했습니다!
분리할 수 있는 컴포넌트는 분리해보도록 하겠습니다!
There was a problem hiding this comment.
이 컴포넌트는 사용하지 않는 컴포넌트인데 삭제를 못 하고 제출한 것 같습니다...!
처음 작성 시 컴포넌트를 분리해보려 이 컴포넌트를 작성했는데 사용하지 않았습니다!
| title, | ||
| openModal, | ||
| closeModal, | ||
| modalStates, |
There was a problem hiding this comment.
modalStates 를 외부에서 주입받을 필요가 없어보여요!
모달 열고 닫는 콜백도요!
이 컴포넌트 내에서 처리하면 충분할 것 같습니다
There was a problem hiding this comment.
넵 이 컴포넌트 내부에서 처리해 보도록 하겠습니다!
| }: { | ||
| clickedButton: any; | ||
| handleButtonClick: any; | ||
| handleAllButtonClick: any; | ||
| folders: any; | ||
| openModal: any; | ||
| closeModal: any; | ||
| modalStates: any; | ||
| }) { |
There was a problem hiding this comment.
여기 Props 들도 외부에서 주입받을 필요가 있는지 점검해보세요~!
| favorite?: boolean; | ||
| } | ||
|
|
||
| export default function FolderButtons({ |
There was a problem hiding this comment.
폴더 종류라는 문맥이 있는 버튼들을 나타내는 컴포넌트이니
Button 을 Tab 으로 표현해보면 ui 가 더 잘 연상될 것 같아요!
There was a problem hiding this comment.
컴포넌트 이름을 FolderTab으로 변경하도록 하겠습니다!
| <ShareFolderModal | ||
| openModal={openModal} | ||
| closeModal={closeModal} | ||
| modalStates={modalStates} | ||
| /> | ||
| <EditFolderModal | ||
| openModal={openModal} | ||
| closeModal={closeModal} | ||
| modalStates={modalStates} | ||
| /> | ||
| <DeleteFolderModal | ||
| openModal={openModal} | ||
| closeModal={closeModal} | ||
| modalStates={modalStates} | ||
| /> |
There was a problem hiding this comment.
요 컴포넌트들도 모달이라는 이름이 모호합니당
내부를 보니 버튼과 모달이 함께 있어서요.
버튼+모달 단위로 분리하기보다
버튼을 제외한 모달 단위로 분리하고
버튼들 + 분리한 모달을 하나의 컴포넌트로 분리하면 어떨까요?
표시되고 안되고의 단위를 생각했을때요!
| <img | ||
| src="/images/pen.svg" | ||
| alt="pen" | ||
| className="cursor-pointer" | ||
| onClick={() => openModal("editModal")} | ||
| /> |
There was a problem hiding this comment.
FolderTitlebar 에 코멘트 남겼듯이
모달이라는 이름의 컴포넌트라면 딱 모달 ui 에 해당하는 영역만 관리하는게 좋을 것 같습니다
버튼 역할을 하는 이미지 태그는 밖으로 빼주세요!
요구사항
기본
https://bootcamp-api.codeit.kr/docs 에 명세된 “/api/sign-in”으로 { “email”: “[email protected]”, “password”: “sprint101” } POST 요청해서 성공 응답을 받으면 “/folder”로 이동합니다.
심화
주요 변경사항
스크린샷
멘토에게