Conversation
|
@hakyoung12 배포 링크가 vercel 사이트로 이동하는데 확인 부탁드립니다..! |
| const handlePasswordType = ( | ||
| setType: React.Dispatch<React.SetStateAction<"password" | "text">> | ||
| ) => { | ||
| setType((prevType) => (prevType === "password" ? "text" : "password")); |
There was a problem hiding this comment.
setType 인자를 받아서 토글로 처리해주고 계시네요!
어떤 의도인지는 모르겠으나 혹시 순수함수로 작성하기 위해서 인걸까요?
코드 파악하는 면에선 인자로 받지않고 바로 setPasswordType 를 호출해서 사용하면 어떨까요?
| validate: (value) => | ||
| value === passwordRef.current || "비밀번호가 일치하지 않아요.", | ||
| })} |
There was a problem hiding this comment.
validate 로 전달한 함수에 두번째 매개변수로 폼 전체에 대한 값을 가져올 수 있습니다.
ref 를 대체할 수 있을 것 같습니다.
validate: (value, formValues) => ...
| const onSubmit: SubmitHandler<SigninFormValues> = (data) => { | ||
| submitLoginForm(data.email, data.password); | ||
| }; | ||
| // accessTokenCheck(); |
There was a problem hiding this comment.
불필요한 코드는 pr 제출 전 삭제해주세요!
| setType((prevType) => (prevType === "password" ? "text" : "password")); | ||
| }; | ||
|
|
||
| return ( |
There was a problem hiding this comment.
라이브러리를 사용하면서 속성 관련 로직이 늘어서 한눈에 파악하기 어려운 것 같아요.
제가 큰 단위의 컴포넌트를 분리해야한다고 느끼는 지점이 슥 보고 머릿속에 어떤 컴포넌트들이 포함되어있는지 쉽게 떠올릴 수 있는가 입니당
요 기준으로 생각해보셔도 좋을 것 같아요.
There was a problem hiding this comment.
커스텀 훅을 이용해 모바일 상태임을 명확히 분리한 점 좋습니다 👍
추가로 말씀드리고 싶은건 브라우저 창을 줄일 때 리사이즈 이벤트가 많이 발생될 거라 꽤 부하가 걸릴 것 같아요.
디바운스라는 개념을 적용해보면 좋을 것 같습니다! (+ 쓰로틀도 살펴보세요!)
| function handleClose() { | ||
| setModalOpen(""); | ||
| } |
There was a problem hiding this comment.
함수 이름만 봤을 때 handleClose 는 닫았을 때 처리하는 로직을 담고있는 이벤트 핸들러일 것 같아요.
modalOpen 값이 빈 문자열일 경우 모달을 닫는다는 의미로 분리해주셨을 테니 closeModal 로 명명하면 어떨까요?
지난번 코멘트 남겼던 기억이 안났더라면 왜 빈 문자열을 전달하지..? 라고 생각할 뻔 했습니당
빈 문자열일 때 모달을 닫는다는 등의 주석을 달거나 함수명으로 역할을 명확히 해주면 좋을 것 같습니다~
| interface FolderListProp { | ||
| folders: Folder[]; | ||
| title: string; | ||
| handleTitle: (title: string, id: number | null) => void; |
There was a problem hiding this comment.
함수 props 명 네이밍 관련된 글입니당 참고해보세요!
| <FolderMenu title={title} folders={folders} handleTitle={handleTitle} /> | ||
| <AddFolderButton isMobile={isMobile} setModalOpen={setModalOpen} /> |
There was a problem hiding this comment.
잘 분리해주셨네요!
isMobile 은 사용되는 곳이 AddFolderButton 컴포넌트에 전달하는 Props 가 전부라 내부에서 호출해 사용하는게 좋을 것 같아요.
| </div> | ||
| </button> | ||
| ); | ||
| } else { |
There was a problem hiding this comment.
조건이 참일 때 리턴해주고 있어서 else 문으로 감싸지 않아도 될 것 같습니다.
| } | ||
|
|
||
| /** 로그인/회원가입 토큰체크 */ | ||
| export function accessTokenCheck() { |
요구사항
기본
심화
주요 변경사항
스크린샷
https://5-weekly-mission-nwjf3sk7e-hakyoungs-projects.vercel.app/
멘토에게