Conversation
There was a problem hiding this comment.
법균님! 전반적으로 잘 작성해주셨네요
zod를 활용한 schema validation부터 useForm 활용까지
잘 활용하고 계십니다.
다만 몇가지 피드백 드리고 싶은 부분이 있는데요 우선 첫번째로 컨테이너 성 콤포넌트를 구조화 하는 부분이 아직 어려워 보입니다.
근데 이건 어려울 수 밖에 없는게, 특히 NextJS가 일정 부분은 서버사이드로 렌더링하고, 특정 부분에 대해서만 csr로 처리하도록 해주는 기능으로 인해 이 구조화가 훨씬 중요해지게 되었거든요. 따라서 이 부분 염두에 두고 콤포넌트를 조금 더 구조화 해볼 수 있는 방법을 찾아보면 어떨까 생각이 되어요.
고생 많으셨고, 조금 더 디테일한 피드백 코드단에 남겨두었으니 체크 부탁드릴게요!
| <Link href={'/'} className={styles.logo}> | ||
| <Image src={'/assets/images/logo.svg'} fill alt='Linkbrary' /> | ||
| </Link> | ||
|
|
There was a problem hiding this comment.
일반 string literal을 attribute에게 전달하는 경우 {'/'} 형태보단 href='/' 처럼 바로 꽂아주세요!
| text: string; | ||
| linkText: string; | ||
| link: string; |
There was a problem hiding this comment.
text, linkText, link가 생각보다 비직관적으로 와닿는데요
이 부분을 조립할 수 있도록 해보는건 어떤가요?
외부에서 사용한다면 다음과 같이 활용할 수 있도록요
<AuthHeader>
<AuthHeader.Description> 회원이 아니신가요?</AuthHeader.Description>
<AuthHeader.Link to="/signup"> 회원가입 하러가기</AuthHeader.Link>
</AuthHeader>
<AuthHeader>
<AuthHeader.Description> 이미 가입하셨나요??</AuthHeader.Description>
<AuthHeader.Link to="/login"> 로그인 하러가기</AuthHeader.Link>
</AuthHeader>There was a problem hiding this comment.
| type={type} | ||
| disabled={disabled} | ||
| > | ||
| {disabled ? '처리중' : children} |
There was a problem hiding this comment.
오 되게 잘 만들어두셨는데요?
disabled상태인 경우 처리중으로 보여질 필요는 없을 것 같아요!
버튼 콤포넌트에 disabled 처리를 함으로써 조금 뿌얘지고 - opacity 50%정도 - 커서 block이 되면 그것으로도 적당하지 않나 생각이 됩니다
| children?: React.ReactNode; | ||
| onClick?: () => void; | ||
| className?: string; | ||
| type?: 'button' | 'submit' | 'reset' | undefined; |
There was a problem hiding this comment.
type?: ... 를 했다면 굳이 undefined를 전달하지 않아도 되어요
또한 조금만 더 욕심내본다면, 우리가 Button 콤포넌트의 prop을 바로 네이티브 요소인 button에게 전달하잖아요.
그럼 일반 button 요소가 받아올 수 있는 모든 prop들은 우리 커스텀 버튼이 활용할 수 있어야겠죠?
이를 활용하기 위해 리액트에서 제공해주는 button이 기본적으로 지니는 attribute props 타입이 있습니다.
interface ButtonProps extends ... 하는 식으로 버튼이 기본적으로 지녀야 하는 상태들을 상속하도록 타입을 설정해보면 어떨까요?
| import instagramLogo from '@/public/assets/images/instagram_logo.svg'; | ||
| import styles from './Footer.module.css'; | ||
|
|
||
| export default function Footer() { |
There was a problem hiding this comment.
어떤곳은 arrow func으로 콤포넌트 선언을 하고
어떤곳은 function 선언문으로 콤포넌트 선언을 하는데요
통일 시켜보면 어떨까요?
| > | ||
| <input | ||
| {...register('email')} | ||
| type='text' |
There was a problem hiding this comment.
@bk-git-hub
type='email'
There was a problem hiding this comment.
해당 타입을 이메일로 지정할 경우 브라우저에서 알아서 유효성 검사를 실시하고 브라우저 자체 에러문구가 떠서 제가 구현한 유효성 검사를 하기위해 text로 지정해놨습니다
| export default function SocialAuth({ text }: SocialAuthProps) { | ||
| return ( | ||
| <div className={styles.socialAuthWrapper}> | ||
| <span>{text}</span> |
There was a problem hiding this comment.
이거가 prop으로 받아오는 이유가 있나요?
There was a problem hiding this comment.
text라고 받아오기보단, social-auth 콤포넌트의 제목으로 활용되는 녀석으로 보이는데 title이라고 해보는건 어떨까 의견을 드려봅니다
| <Button className={styles.addLinkButton} onClick={handleAddLinkClick}> | ||
| 링크 추가하기 | ||
| </Button> |
There was a problem hiding this comment.
링크 추가하는 버튼을 따로 콤포넌트화 해서 붙여넣으면 pages/index.tsx 는 ssr로 처리가 될 수 있을 것 같아요
단순 router.push 기능 하나만을 위해 상위에서 구현함으로써 렌더링 퍼포먼스 최적화를 손해보기엔 너무 아쉬운 작업으로 보입니다!
| link='/signin' | ||
| /> | ||
| <SignUpForm /> | ||
| <SocialAuth text='다른 방식으로 가입하기' /> |
There was a problem hiding this comment.
아 여기서 달라지는군요!
위에 social auth 관련 코멘트 무시해주세요
There was a problem hiding this comment.
이제 zod를 쓰니 굳이 필요없지 않을까 싶어요!
요구사항
기본
심화
주요 변경사항
스크린샷
멘토에게