-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[4주차 기본/심화/공유 과제] 회원가입 & 로그인 #5
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
너무 잘하셨네요 ㅋㅋ,,
프로젝트 세팅부터, 시멘틱 태그활용, 로직, 커스텀 훅, 스타일링 등 그냥 다 너무 깔끔하게 잘 하셨네요
고생하셨습니다 👍👍👍
week4/src/router/Layout.tsx
Outdated
<div> | ||
<main> | ||
<Outlet /> | ||
</main> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요기도 그렇고 MyPageLayout 부분도 그렇고 혹시 겉에 div를 하나 더 감싼 이유가 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
특별한 이유는 없고 딱히 신경을 안썼었네요 ,, ㅎㅎ
제거해도 무방해보이네요 ~!
marginBottom: 14, | ||
padding: 12, | ||
borderRadius: 8, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이렇게 단위를 설정하지않으면 기본적으로 px단위로 지정되지 않나요?
font등에서는 rem단위를 사용하고 여백 등 레이아웃에서는 px단위를 사용하시는 것으로 보이는데,
폰트는 접근성을 고려하고 레이아웃은 깨지지 않게 하려고 하신걸까요?
단위를 섞어쓰면 유지보수나 일관성 측면에서 단점이 좀 생기지 않나 해서 여쭤봅니다~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4주차 과제진행할 때 vanilla extract에 익숙치 않았어서 단위를 많이 생략(ve에서는 생략시 px로 변환)했었네요 ... 빠르게 진행하다보니 ㅎㅎ.
border를 제외하고는 rem으로 바꾸는게 적절하다고 저도 생각합니다 ~
week4/src/components/input/Input.tsx
Outdated
placeholder: string; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
React.InputHTMLAttributes<HTMLInputElement>
를 상속받고 있어서 placeholder에 대한 타입은 생략해줘도 괜찮지 않을까요?
혹시 placeholder를 필수값으로 해주고 싶어서 작성하신거라면 지나가겠습니다 ~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
placeholder를 명시적으로 선언해줌으로써 필수요소임을 강조하기위해 설계했습니다 !
재사용성을 조금 더 고려해본다면 제거하는게 적절해보이네요 ! 감사합니다 ㅎ
✨ 구현 기능 명세
💡 기본 과제
취미
,내 정보
메뉴 탭로그아웃
버튼취미
,내 정보
취미 페이지, 내 정보 페이지 출력 (1개의 페이지로 구현해도 되고, url 달라도 됨)🔥 심화 과제
공유과제
제목: Protected Route로 라우트 보호하기
링크 첨부 : https://wave-web.tistory.com/124
❗️ 내가 새로 알게 된 점
❓ 구현 과정에서의 어려웠던/고민했던 부분
axios interceptors 설정
onErrorResponse 함수
HTTP 상태 코드와 서버 명세서에서 제공한 에러 코드를 통해
errorMessage
를 설정하고, 개별 컴포넌트에서alert
을 통해 에러 메세지를 띄우도록 설계했어요.해당 과정에서 에러 핸들링을 인터셉터에서 처리하는게 맞는지, 각 api 호출시 마다 직첩 처리하는 것이 나은지, 각 컴포넌트 에서 핸들링하는게 맞는지 어떤게 가장 적합한 방법인지 많이 고민했어요.
이번 과제에서는 api마다 상태코드와, 에러코드에 따라서 출력해야하는 에러메세지가 다 달라서 처음에는 각 api 호출시 에러를 처리하는 방안으로 생각했는데 코드가 너무 길어져서 인터셉터를 통해 처리했어요. 이와 관련해서 의견이 궁금합니다 !
- 커스텀 훅으로 어디까지 기능을 분리해야하는지 고민했어요.
커스텀 훅을 많이 다루어본적이 없어서 어느 로직까지 커스텀 훅으로 처리해야 할지 고민했어요.
재사용될 일이 없는 기능은 컴포넌트에서 처리하는 일이 많아지더라도 해당 컴포넌트에서만 처리하게 납둬야하는지, 분리해줘야 하는지, 서버와 통신하는 부분은 분리해줘야 하는지, ... 여러분들의 커스텀 훅에 대한 의견이 궁금해요 !
- 회원가입 폼 컴포넌트 분리에 대해서 고민했어요.
처음에는 하나의 회원가입 폼에 이름,비밀번호,취미를 입력받는 기능들이 한 곳에 위치하도록 구현했어요. 그러다보니 가독성이 많이 떨어진다고 생각했고, 각 스텝마다 컴포넌트를 분리해서 구현했어요.
SignupForm.tsx
근데 분리하고보니 컴포넌트 구조가 깊어진 것 같기도하고 잘 분리한건지 확신이 안서요. 피드백 부탁드립니다 !
(현재
Signup
->SignupForm
-> 각 스텝별 폼 으로 이어지는 구조)🥲 소요 시간
12h
🖼️ 구현 결과물
회원가입
default.mp4
회원가입 - 오류
-.mp4
로그인
default.mp4
마이페이지 - 취미
-.mp4
마이페이지 - 내정보
-.mp4