Feat/#480 프로필 편집 페이지에서 닉네임 변경 기능 추가 #490
Conversation
ukkodeveloper
requested review from
Creative-Lee and
cruelladevil
as code owners
cruelladevil
force-pushed
the
feat/#480
branch
from
a3dab04
to
9a88c5b
Compare
There was a problem hiding this comment.
우코 닉네임 변경기능 만든다고 고생하셨습니다.
실수로 놓치신 부분들이나, 제안드릴 사항에 대한 피드백 남겨 보았습니다.
내용이 조금 많으니 천천히 확인해주시고 다시 요청주세요!
혹시 피드백 내용중에 이야기 나누고 싶은 부분 있으면 바로 대화요청주시고요~ㅎㅎ
별도의 디자인 제안을 하나 하려해요.
최근에 PC에서의 킬링파트 듣기 페이지 디자인 개선이 이뤄지면서,
개인적으로는 다른 모든 페이지도 마치 PC first로 개발한것 만큼의 레이아웃이 갖추어져야 겠구나 생각했어요.
현재 shook의 마이페이지는 필요 이상으로 길게 늘어져 있다고 생각해서, 왼쪽으로 정렬해보면 어떨까 제안드려요.
아래에는 오늘의집 마이페이지 인데요~!
여러 사이트의 마이페이지를 눌러봤는데 가장 깔끔하다고 생각해서 레퍼런스로 가져왔어요.
There was a problem hiding this comment.
우코가 만든 Login, withdraw, NicknameChangingModal 들은 공통점이 있어요.
모두 사용자의 의도를 묻는 contents와 취소시 모달 닫기, 수락시 특정 함수를 실행하는 Confirm 이 목적인 모달이라는거죠.
이걸 고려해서 ConfirmModal이라는 공통 컴포넌트를 만들 수 있을것 같아요.
이후엔 사용자에게 질문하려고 할 때 마다 매번 위와 같은 xxx모달 컴포넌트를 만들지 않아도 되겠네요😀
|
|
||
| return ( | ||
| <Modal isOpen={isOpen} closeModal={closeModal}> | ||
| <> |
There was a problem hiding this comment.
이 프래그먼트는 없어도 되겠네요~
withdrawalModal에도 동일한 부분에 같은 프라그 먼트가 있으니 삭제해 주세요.
| export default NicknameChangingModal; | ||
|
|
||
| const ModalContent = styled.div` | ||
| align-self: start; |
There was a problem hiding this comment.
💬 없어도 아무런 변화가 없던데, 위 속성은 어떤 역할을 하나요?
| return ( | ||
| <Modal isOpen={isOpen} closeModal={closeModal}> | ||
| <> | ||
| <ModalContent>{`닉네임 변경 시 다시 로그인을 해야합니다.\n닉네임을 ${nickname}로 바꾸겠습니까?`}</ModalContent> |
There was a problem hiding this comment.
시멘틱 + 웹접근성 측면에서
div 안에 바로 문장 내용을 보여주는 것 보단, 별도의 태그로 내용물을 분리하는게 좋을 것 같아요.
| font-weight: 700; | ||
| line-height: 1.6; |
There was a problem hiding this comment.
line-height 단위를 % or 숫자 하나로 통일하는게 좋겠네요.
| <Input | ||
| id="nickname" | ||
| value={nicknameEntered} | ||
| onChange={handleChangeNickname} | ||
| autoComplete="off" | ||
| /> |
There was a problem hiding this comment.
💬 maxlength를 활용해봐도 좋을 것 같아요
| autoComplete="off" | ||
| /> | ||
| <Spacing direction={'vertical'} size={8} /> | ||
| {hasError && <BottomError>{nicknameErrorMessage}</BottomError>} |
There was a problem hiding this comment.
💬 위치를 나타내는 것 보다 어떤 역할인지 나타내주는건 어떤가요?
| const SubmitButton = styled.button<{ disabled: boolean }>` | ||
| cursor: ${({ disabled }) => (disabled ? 'not-allowed' : 'pointer')}; | ||
| const SubmitButton = styled.button` | ||
| cursor: pointer; |
|
|
||
| ${disabledStyle}; | ||
| background-color: ${({ theme }) => theme.color.primary}; | ||
| border: none; |
| transition: box-shadow 0.3s ease; | ||
|
|
||
| &:focus { | ||
| box-shadow: 0 0 0 2px inset ${({ theme }) => theme.color.primary}; |
There was a problem hiding this comment.
💬 개인적인 의견으로는 input을 입력하다가 정책상 금지된 행동을 했을때,
붉은 계열의 테두리가 생기는게 일반적이라고 생각해요.
focus만 했는데, 붉은 계열의 테두리가 생겨서 '내가 뭘 잘못했나?' 생각할수도 있을것 같네요.
어떻게 생각하세요?
📝작업 내용
프로필 편집 페이지에서 닉네임 변경 기능 추가
💬리뷰 참고사항
닉네임 변경 정책
닉네임 변경 요청 시 access token을 파기하고 로그인 전역 상태를 초기화 합니다.
jwt에 유저 정보가 포함되어 있기 때문에 닉네임 변경 시 access token을 갱신을 해야합니다.
따라서 닉네임 변경 후에는 다시 로그인하도록 로그인 페이지로 리다이렉트 해줍니다.
닉네임은 2글자 이상 10글자 이하입니다.
응답 상태코드
400 에러처리는 따로 하지 않았습니다. 에러 핸들링 논의 해야할 것 같습니다.
현재 중복된 닉네임으로 요청시 마치 성공한 것과 같은 유저 플로우가 진행됩니다. 따라서 에러 핸들링 적용 뒤에 머지가 되어야 합니다.
#️⃣연관된 이슈
close #480