Skip to content

회원가입, 로그인, 로그아웃 UI 코드 작성#14

Open
kjw7953 wants to merge 27 commits into
snulion10thAdvancedSeminar:mainfrom
kjw7953:main
Open

회원가입, 로그인, 로그아웃 UI 코드 작성#14
kjw7953 wants to merge 27 commits into
snulion10thAdvancedSeminar:mainfrom
kjw7953:main

Conversation

@kjw7953

@kjw7953 kjw7953 commented Jul 23, 2022

Copy link
Copy Markdown

UI 작업

  • 로그인, 회원가입, 로그아웃 기능 추가

  • 서버와 관련되어 API를 호출하거나 API 응답을 처리하는 코드는 넣지 않았습니다. 따라서 로그인 여부는 세션 스토리지의 isAuthenticated 값을 사용하여 체크하는 것으로 임시 처리 되어 있습니다.

  • 현재 아무 값이나 입력만 한다면 로그인이 가능한 상태입니다.

  • 추가된 기능 모두 같은 템플릿에서 진행할 수 있게 코드 짰는데 이렇게 작성한다면 App.js 단에서 div 넣고 className template-container 넣어줘도 될 것 같습니다.

API 연동

  • 로그인, 회원가입, 로그아웃 API 연동

  • 각 기능이 실패했을 때의 처리는 따로 하지 않았습니다. 필요한 것이 있다면 말씀해주세요.

  • 현재 서버 코드 단에서 todo 객체의 CRUD와 관련하여 4주차에 추가되는 인증 작업이 반영되지 않았습니다. 서버 코드를 수정한 이후 프론트 코드도 추가 수정이 필요할 수 있습니다. @lynn0506

  • 프론트단에서 jwt로 인증 플로우 구축하는 걸 처음해봤습니다. 일단 구글링해서 코드 써넣긴 했는데 염려되는 지점들이 있습니다. 이 점들 유의해서 봐주시면 감사하겠습니다

    • 이 아티클을 참고하였습니다.
    • 위 아티클에 따라 토큰을 브라우저 스토리지에 저장하지 않으려했고, access token을 앱 내 지역 변수로 사용했습니다. (axios 디폴트 값)
    • 서버에서는 jwt 토큰을 drf simple-jwt을 활용해 구현했습니다. 이 라이브러리에서는 refresh 토큰 업데이트를 위해 클라이언트가 payload에 refresh token의 값을 넣어줘야 합니다. 하지만 위 아티클에서는 서버에서 refresh token을 관리하고 쿠키에 넣어준다는 가정을 하고 있었어요. 그래서 우선 refresh token은 세션 스토리지에서 관리하도록 구현했습니다.
    • 이렇게 처리하고 보니 '조금 번잡한 감이 있는데 보안은 신경쓰지 말고 세션 스토리지에서 다 관리하자' vs 'jwt를 사용해보는데 기왕 보안에 관해서 조금 더 생각해볼 수 있게 하자'로 고민이 됩니다. 이 점 관련해서 의견을 듣고 싶어요
  • 급하게 쓰냐고 리팩토링은 거의 하지 않았습니다... 테스트도 제일 간단한 플로우만 했어요. 똥 코드도 지적해주시면 감사하겠습니다...! 🙏

@lynn0506 lynn0506 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

정우,,, 감사링 👍

Comment on lines +6 to +8
input, input {
margin-bottom: 20px;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그냥 Input이랑 button 하나씩 적어도 스타일링 잘 먹을 것 같은데요
Input, input과 button, button 두번 쓴 이유가 궁금합니다

@kjw7953 kjw7953 Aug 21, 2022

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오 답변이 정말 늦었군요... 하나씩 적어도 스타일링이 되긴하지만 가능한 해당 코드가 반영되는 스코프를 좁히려고 했습니다.

예를 들어 input, input이면 아이디랑 비밀번호 입력 창 사이의 여백만 조정되게 됩니다.
반면에, 단일 inputmargin-bottom 값을 설정하는 방식으로 처리한다면 비밀번호 입력창과 로그인 버튼 사이의 여백에도 해당 값이 반영됩니다. 그래서 input 섹션 / 버튼 섹션으로 나눠서 섹션 단위로 여백을 조정할 때 영향을 줄 수 있다고 생각했어요.

질문의 의도에 잘 답했나 모르겠는데, css를 잘 몰라서 가능한 예상치 못한 사이드 이펙트를 줄이려고 저렇게 작성했다고 봐주심 좋을 것 같아요! 좋은 코드 아니라면 코멘트 부탁드립니다 🙏

Comment thread src/App.js
return <TodoTemplate />;
} else {
return <AuthTemplate />;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P5] else 분기 없애고, 그대로 AuthTemplate 리턴하면 좀 더 깔끔할 것 같습니다

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

f6987cd 에서 반영했습니다!

});

const onChangeFormData = (e) => {
e.preventDefault();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

질문입니다! 이건 무엇을 위한 e.preventDefault() 인가요?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

음.. 아마... 무지성으로 적은 것 같습니다!
제가 알기론 쓸모가 없을 것 같은데, 확인해주시면 삭제할게요~

});
};

const onSubmit = (e) => {

@jaehvvan jaehvvan Aug 3, 2022

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] e.preventDefault() 없어도 괜찮은가요? return false 가 막아주나요??

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

75fa912에서 수정했습니다!

Comment thread src/components/SignIn/index.js Outdated
password
} = formData;

if (!username || !password) {

@jaehvvan jaehvvan Aug 3, 2022

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P3] 공백 체크 필요할 것 같습니다.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

d6d26a0에서 수정했습니다! 오만 곳에 trim()이 붙는 것 같은데.. 보다 깔끔한 방법있으면 말씀해주세요!

@kjw7953 kjw7953 requested review from jaehvvan and lynn0506 August 21, 2022 16:23
Comment thread src/api/auth.js Outdated
}) => {
axios
.post("api/accounts/signup/", {
email: `${username}@naver.com`,

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분 처리가 이상하다고 느껴질텐데, 현재 서버 코드 단에서 회원가입 할 때 emailusername을 모두 유니크한 값으로 설정하도록 구현되어 있습니다. (로그인 단계에서는 프론트, 서버 모두 username, password만 참조합니다)
따라서 오류를 내지 않기 위해 email 값을 임시로 처리해두었고, 수정 요청 한 상태입니다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants