어드민 계정 분리 관련 페이지 작업 #63
Conversation
Gwak-Seungju
requested review from
ChoiWonBeen and
hanagertrudeKim
as code owners
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| { | ||
| !isPasswordCorrect | ||
| && <div style={{ color: 'red' }}>현재 비밀번호가 일치하지 않습니다.</div> | ||
| } |
There was a problem hiding this comment.
antd를 살펴보니 다이나믹룰 이라고 하면서dependencies와 rule의 validation으로 비번 확인에 활용할 수 있는게 있는 것 같아요! 버전 등으로 못쓸 수도 있으니 확인 한번 해주시면 좋을 것 가탕요!
error 관련은 따로 쓰기보다는 antd의 form의 input을 쓰고 있으니 가능하다면 rule안에서 최대한 처리하는 방식을 찾아보는 것도 좋을 것 같습니다!
| } | ||
|
|
||
| const ModalWrap = styled.div` | ||
| span.ant-btn-icon { |
There was a problem hiding this comment.
자식 컴포넌트 UI를 부모 컴포넌트에서 CSS로 수정하는 것이 왜 안티패턴일지 고민해보시고 댓글을 달아주세요.
| const ButtonContainer = styled(Flex)` | ||
| padding-top: 12px; | ||
| button { |
There was a problem hiding this comment.
CSS in JS에서 CSS 선택자로 HTML 태그를 지정하는 것이 왜 안티패턴일지 고민해보시고 댓글을 달아주세요.
|
|
||
| const [changePassword] = useChangePasswordMutation(); | ||
| const [changePasswordForm] = CustomForm.useForm(); | ||
| const { required } = CustomForm.useValidate(); |
There was a problem hiding this comment.
본인이 만든 훅이 아니지만, useValidate는 훅으로 분리될 이유가 있었던 것인지 확인해주세요.
| const refList = [idRef.current, passwordRef.current]; | ||
|
|
||
| if (refList.some((current) => (current?.input?.value === ''))) message.warning('필수 입력값을 입력해주세요.'); |
There was a problem hiding this comment.
본인이 하지 않았지만, ChangePasswordForm과 LoginForm이 다른 방식으로 validate하는 이유가 있는지 살펴보고, 바꿀 부분이 있다면 고쳐주세요.
| const hashedCurrentPassword = sha256(formData.currentPassword); | ||
| const hashedNewPassword = sha256(formData.checkPassword); | ||
| if (formData.newPassword !== formData.checkPassword) { | ||
| message.error('새 비밀번호를 다시 확인해주세요.'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
현재 사용하는 비밀번호 변경 api에서는 새 비밀번호와 새 비밀번호가 일치하는지 확인하는 동작이 없어서 그 조건을 추가했습니다!
There was a problem hiding this comment.
({ getFieldValue }) => ({
validator(_, value) {
if (!value || getFieldValue('newPassword') === value) {
return Promise.resolve();
}
return Promise.reject(new Error('새 비밀번호와 일치하지 않습니다.'));
},
}),위 코드는 일치하는 것을 확인하는 동작이 아닌가요?
| function CustomInput({ | ||
| label, name, rules, disabled, ...args | ||
| }: CustomFormItemProps & Omit<InputProps, 'name'>) { |
There was a problem hiding this comment.
CustomFormItemProps과 DependenciesProps가 왜 따로 선언되어야 하는지 이유를 설명해주세요.
이 또한 본인이 하지 않았지만, disabled가 InputProps에 포함이 되어있는데 명시한 이유도 설명해주세요.
There was a problem hiding this comment.
CustomItemProps에dependencies를 추가했을 시'dependencies' PropType is defined but prop is never used,eslint-disable-next-line react/no-unused-prop-types와 같은 lint오류가 발생했었습니다. 일단DependenciesProps를 따로 선언한 이유는 이 오류를 해결하는 과정에서 생긴 결과입니다. 결과적으로CustomFormItemProps타입을 사용하는 모든 컴포넌트에dependencies속성을 추가해야 해결되는 오류인 것을 발견했습니다. 하지만,dependencies를 추가하지 않아도 lint오류가 안 생기는 컴포넌트가 있어서 위 해결방법이 이 오류의 근본적인 원인이 아니라고 생각했던 것 같습니다. ...args라는 나머지 매개변수를 선언한 컴포넌트와 아닌 컴포넌트의 차이 때문인 것을 인지하여dependencies를 사용하지 않는CustomFormItemProps타입을 가진 컴포넌트에 나머지 매개변수를 추가했습니다 ! 67d92b1Form내부에 고정값으로 놓아야 하는 것이 있는 경우 수정 불가하게 하기 위함인 것 같습니다. 추가로, 스타일 일치를 위해 굳이 다른 컴포넌트를 사용하지 않은 것이라 생각합니다 !
There was a problem hiding this comment.
Form내부에 고정값으로 놓아야 하는 것이 있는 경우 수정 불가하게 하기 위함인 것 같습니다.
어떤 부분이 고정값으로 놓아야 한다는 것이고, 수정불가하게 하기 위함인지 모르겠습니다.
스타일 일치를 위해 굳이 다른 컴포넌트를 사용하지 않은 것이라 생각합니다 !
만약 스타일 일치를 위해서라면 CustomFormItemProps를 통해서 FormItem에 넘겨줄 값을, {Component}Props를 통해 컴포넌트에 넘겨줄 값을 정의하는 것이 스타일 일치에 도움이 된다고 생각하는데 어떤 부분이 스타일 일치를 위해로 해당되는지 설명해주실 수 있나요?
| interface PasswordFormData { | ||
| currentPassword: string, | ||
| newPassword: string, | ||
| checkPassword: string, |
There was a problem hiding this comment.
그렇군요! 좀 더 직관적이게 passwordConfirmation으로 수정했습니다! dc6868d
src/utils/hooks/useLogout.ts
Outdated
| const logout = () => { | ||
| sessionStorage.removeItem('token'); | ||
| localStorage.removeItem('refresh_token'); | ||
| setCredentials({ token: '' }); | ||
| window.location.href = '/login'; | ||
| }; |
There was a problem hiding this comment.
useLogout이 hook으로 분리될 정도로 재사용되나요?- logout 함수만 반환하는데, 이를 훅으로 만들 가치가 있었나요?
There was a problem hiding this comment.
SideNav한 곳에서만 로그아웃을 하기 때문에 재사용성은 없습니다.- 한 곳에서만 쓰이고 단순히 logout만 하는 함수이기 때문에
SideNav에서 정의해도 될 것 같습니다.
반영했습니다! 74127bb
There was a problem hiding this comment.
현재 토큰을 저장하는 용도로 redux를 사용하고 있는데, redux를 단순 저장용도로 사용하고 다른 사이드 이펙트를 React쪽에서 관리하는 이유라도 있을까요?
ISSUE 어드민 계정 분리 #62
어드민 계정 분리 #62 의 첫 번째 Progress입니다!
SideNav에 히스토리 탭과 비밀번호 변경, 로그아웃 버튼을 추가했습니다.
추가로 2차 비밀번호의 필요성도 없어지며 기존 프론트에서 관리하던 2차 비밀번호를 삭제했습니다.
ScreenShot
SideNav에 탭 추가
비밀번호 변경 모달
변수명 짓기 너무 어려워요.. ㅠ