Conversation
There was a problem hiding this comment.
리뷰를 잘 반영해주신 것 같아요~
컴포넌트를 분리해 ui 를 더 파악하기 쉽게 하면 좋을 것 같습니다.
폼 라이브러리가 얼마나 많은 곳에서 사용되는지는 잘 모르겠습니다.
(확인차 채용 공고 몇개를 봤을 때 기술스택에 기재한 곳은 없네요. 이정도 규모의 라이브러리는 잘 기재를 안하는게 아닐까 싶습니다.)
전직장에선 사용했지만 최대한 폼 관련 로직을 자체 개발해서 작업하다가
유효성이나 폼 관련 스펙을 일일히 적용하기가 어렵고, 폼 쪽은 빠르게 개발하고 다른 로직 개발에 집중하자는 차원에서 도입했었는데요.
리액트 버전과의 충돌 등으로 다시 사용하지 않는 방향으로 했습니다.
라이브러리의 장점은 여러 기능을 직접 개발하지 않고 편리하게 사용할 수 있다는 점이 있지만 단점으로는 의존성이 생겨버려 업데이트가 잘 되지 않거나 다른 패키지 버전과 충돌이 있을 수 있어서 필요에 맞게 적용하는게 중요하다고 생각합니다.
미션 고생하셨습니다 👏
| password: password, | ||
| }); | ||
| localStorage.setItem('token', data.data.accessToken); | ||
| window.location.href = '/'; |
There was a problem hiding this comment.
경로 이동은 네트워크 요청 함수의 역할을 벗어나 예측하기 어려운 부분 같습니다.
성공 시 콜백 등으로 외부에서 주입받으면 어떨까요?
| @@ -77,7 +77,7 @@ function Shared() { | |||
| /> | |||
| <S.Container> | |||
| {linkList.map((item) => ( | |||
| <Card item={item} key={item.id} /> | |||
| <Card item={item} key={item.id} setUrl={() => {}} /> | |||
There was a problem hiding this comment.
setUrl 에 의미없는 값을 전달하고 있네요.
사용처에 따라 필요가 없다면 넘기지 않아도 되는 형태로 수정해주세요~
| } catch (error) { | ||
| console.error(error); | ||
| setLoading(false); | ||
| if (id) { |
There was a problem hiding this comment.
early return 패턴으로 코드 깊이를 줄이면 좋을 것 같습니다.
if (!id) {
return;
}
// ...
| }) { | ||
| const changeFolder = () => { | ||
| setFolderId(item.id); | ||
| setFolderName(item.name); | ||
| setOnSelect({ id: item.id, name: item.name }); |
There was a problem hiding this comment.
잘 반영해주셨는데요!
props 로
setOnSelect -> onSelect: ({id, name}: {id: string; name: string}) => void;
요렇게 넘겨주면 어떨까요?
상태를 업데이트 한다는 지식을 이 컴포넌트가 갖지 않도록요~
| function FolderIcon({ | ||
| image, | ||
| children, | ||
| onOpen, | ||
| state, | ||
| }: { | ||
| image: string; | ||
| children: ReactNode; | ||
| onOpen: (modalName: string) => void; | ||
| state: string; | ||
| }) { | ||
| return ( | ||
| <S.FolderModalIcon onClick={() => onOpen(`${state}`)}> | ||
| <Image src={image} alt={`${image}`} width={20} height={20} /> | ||
| {children} | ||
| </S.FolderModalIcon> | ||
| ); | ||
| } |
There was a problem hiding this comment.
가급적 컴포넌트 파일 하나엔 하나의 컴포넌트 함수만 있도록 하거나, 최소한 파일명에 해당하는 컴포넌트를 위쪽에 배치하는게 좋을 것 같습니당
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게