-
Notifications
You must be signed in to change notification settings - Fork 2
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
Feat/#382 EditProfilePage 구현 및 회원 탈퇴 기능 추가 #392
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.
고생 많으셨어요 우코 짱코!😀
이미 만들어진 페이지에 코드 스타일 맞춰가며 기능을 얹는 거라 더 힘들었을텐데 정말 감사합니다~
코멘트 확인해주시고 재요청주세요~
회원탈퇴 기능을 위해, 프로필 편집 페이지를 구현했습니다. 현재 프로필 닉네임이나 소개의 경우 변경할 수 없기 때문에, disabled 처리 했습니다.
굿 굿 입니당 ㅎ
회원 탈퇴 모달을 구현했습니다. 회원 탈퇴의 경우 취소를 primary 색으로 하긴 했는데, 이렇게 해도 괜찮은 지 의견을 듣고 싶습니다.
이거는 사용자는 쪼까... 싫어할 수 있지만, 저희 입장에서는 좋고... 저는 좋은 것 같아요 ㅋㅋㅋㅋㅋㅋㅋㅋㅋ
팀원 의견도 들어보죠~ ㅎㅎ
회원 탈퇴 시에 delete 요청을 보낸 다음, localstorage 에 accessToken을 비우고 전역 상태를 초기화해주었습니다.
확인해 봤는데, delete요청, 스토리지 삭제까지 잘되네요!
frontend/.gitignore
Outdated
@@ -1,3 +1,4 @@ | |||
node_modules | |||
dist | |||
.env | |||
.idea |
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.
eol 설정이 꺼진것 같네용
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.
헉 그렇네요! 바로 수정~
<ModalContent>{WITHDRAWAL_MESSAGE.ERASE}</ModalContent> | ||
<ModalContent>{WITHDRAWAL_MESSAGE.ASK_CONFIRM}</ModalContent> |
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.
💬 상수화 + 메세지 1, 2번 라인 분리의 이유가 있나요?
다른 모달에서 메세지가 2줄이 넘어갈때의 처리방식이랑 다른 방식이라 궁금해서 물어봅니다~
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.
모달에서 여러 줄 줄 때는 이런식으로 적용하긴 했었어요! 하나의 ModalContent에 text를 다 넣는 게 더 좋을 지 생각해봐야겠네요. 한번 적용해보겠습니다
import WithdrawalModal from '@/features/profile/components/WithdrawalModal'; | ||
import useModal from '@/shared/components/Modal/hooks/useModal'; | ||
import Spacing from '@/shared/components/Spacing'; | ||
import ROUTE_PATH from '@/shared/constants/path'; |
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.
아래 네비게이션 경로에 /
대신 root
를 적용하면 알람도 사라지겠네요!
const { user, logout } = useAuthContext(); | ||
const { isOpen, openModal, closeModal } = useModal(); | ||
|
||
const { mutateData } = useMutation(() => fetcher(`/members/${user?.memberId}`, 'DELETE')); |
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.
💬 인자를 받아 url로 fetch 하는 함수로 한번 분리되면 좋을 것 같아요~
네이밍을 잘 지어 분리한다면, 어떤
통신함수를 받아 mutate 하는지 더 알기 쉬울 것 같네요!
개인적으로는 컴포넌트는 통신 함수의 url 까지는 모르는게 자연스럽다고 생각합니다~
<Spacing direction={'vertical'} size={16} /> | ||
<SubTitle>소개</SubTitle> | ||
<Spacing direction={'vertical'} size={4} /> | ||
<TextArea value={''} disabled maxLength={100} /> |
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.
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.
아주 좋구요!
@@ -9,6 +9,7 @@ import Flex from '@/shared/components/Flex'; | |||
import Spacing from '@/shared/components/Spacing'; | |||
import SRHeading from '@/shared/components/SRHeading'; | |||
import useToastContext from '@/shared/components/Toast/hooks/useToastContext'; | |||
import ROUTE_PATH from '@/shared/constants/path'; |
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.
여기도 만든 경로 상수 사용하면 되겠어요!
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.
이거 조금 할말이 있긴한데 ㅋㅋㅋ 캠퍼스에서 이야기 하시죵
frontend/src/pages/MyPage.tsx
Outdated
@@ -41,6 +42,10 @@ const MyPage = () => { | |||
navigate('/'); | |||
}; | |||
|
|||
const editProfileRedirect = () => { |
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.
버튼을 눌러 단순히 페이지 이동을 하는거라면, redirect 라고 부르기에는 다소 어색한것 같아요.
ex) goXXXPage, 같은 단순한 네이밍도 괜찮겠네요!
💬 Link 태그를 사용하지 않고 함수를 만들어 Button에 바인딩하신 이유가 있나요~?
페이지 이동 외에 추가적인 작업이 없다면 Link 태그를 사용해도 될것 같아요~
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.
goXXPage 좋은 것 같아요! 해당 함수명으로 변경했고요. 저도 uri 이동 외에 다른 일을 하지 않아 Link가 더 적절하다곤 생각하지만, 그렇다고 반드시 구분해야 할 정도인 지는 모르겠습니다. 따라서 해당 프로필 편집 UI가 로그아웃 버튼과 동일하기 때문에 재사용하는 게 더 낫다고 판단했습니다.
@@ -37,6 +38,10 @@ const router = createBrowserRouter([ | |||
path: `${ROUTE_PATH.MY_PAGE}`, | |||
element: <MyPage />, | |||
}, | |||
{ | |||
path: `${ROUTE_PATH.EDIT_PROFILE}`, |
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,6 +4,7 @@ const ROUTE_PATH = { | |||
SONG_DETAILS: 'songs', | |||
LOGIN_REDIRECT: '/login/redirect', | |||
MY_PAGE: 'my-page', | |||
EDIT_PROFILE: 'my-page/edit', |
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.
👍 굿
|
||
const ModalContent = styled.div` | ||
font-size: 16px; | ||
color: #b5b3bc; |
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.
💬💬💬 저희 컬러칩에 비슷한 색상 없던가요? ㅋㅋㅋㅋㅋㅋ 색이 많아져서 큰일이네요 🤣🤣🤣
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.
ㅋㅋㅋㅋㅋ 💬💬💬 3개인걸 보니까 뭔가 강한 요구 같네요 ㅋㅋㅋㅋㅋㅋㅋ subText로 두어도 괜찮을 것 같아서 해당 컬러를 가져왔습니다.
66392c0
to
d3ac56f
Compare
d3ac56f
to
efafafb
Compare
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.
코멘트 반영 확인했습니다~ 고생 많으셨습니다~
bbba155
to
11b1c5f
Compare
@@ -32,7 +33,7 @@ const VoteInterface = () => { | |||
|
|||
await createKillingPart(songId, { startSecond: partStartTime, length: interval }); | |||
if (error) { | |||
window.location.href = googleAuthUrl; | |||
navigate(ROUTE_PATH.LOGIN); |
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.
💬 토큰 관련 error에 대한 처리를 위해 로그인 페이지로 보내는 거라면, 추후에 변경되어야 하겠네요!
모든 error에 대해 로그인 페이지로 가게 되니까요!
@@ -35,7 +35,7 @@ const AuthPage = () => { | |||
getAccessToken(); | |||
}, []); | |||
|
|||
return <Navigate to="/" />; | |||
return <Navigate to="/" replace />; |
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.
뒤로가기 시 history가 남아있던 AuthPage로 이동해버리는 이슈를 해결하기 위해 변경되었습니다.
📝작업 내용
#️⃣연관된 이슈
close #382