Conversation
📝 관련 이슈 |
Contributor
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthrough로그아웃 및 회원탈퇴 기능이 추가되었습니다. API 헬퍼와 React Query 훅, 관련 UI 컴포넌트 및 MSW 모의 핸들러가 추가되었고 MyEditPage가 해당 버튼들과 함께 갱신되었으며 Divider 컴포넌트가 HTML 속성 전달을 허용하도록 변경되었습니다. Changes
Poem
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/pages/my-edit/ui/TermsLink/index.styles.ts (1)
4-7:display: block(혹은inline-block) 추가 고려
styled.a의 기본 display는inline이므로,padding-top/padding-bottom이 주변 콘텐츠의 흐름에 영향을 주지 않을 수 있습니다.LogoutButton/UnregisterButton에서 사용하는styled.button은 기본이inline-block이라 수직 패딩이 올바르게 동작하는 반면,<a>는 그렇지 않습니다.♻️ 제안된 수정
export const Link = styled.a` padding: ${({ theme }) => `${theme.unit[12]} ${theme.unit[20]}`}; + display: block; ${TextVariant('body1R')}; `;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/my-edit/ui/TermsLink/index.styles.ts` around lines 4 - 7, The Link styled component (export const Link = styled.a) uses default inline display so vertical padding may not take effect; update the Link styles to include display: inline-block (or display: block if full-width behavior is desired) so padding-top/padding-bottom are applied consistently like the LogoutButton/UnregisterButton; keep the existing padding and TextVariant('body1R') but add the display rule to the component.src/features/auth/api/useUnregister.ts (1)
6-17:useLogout와onSuccess로직이 동일합니다 — 선택적으로 공통 유틸 추출을 고려해볼 수 있습니다.
useLogout와useUnregister모두onSuccess에서queryClient.clear()→navigate(ROUTE.login)순서로 동일한 로직을 수행합니다. 현재 파일 수가 적어 중복이 크지 않지만, 향후 인증 관련 mutation이 늘어날 경우 공통 헬퍼(예:handleAuthSuccess)로 추출하면 유지보수가 용이해집니다.♻️ 공통 헬퍼 추출 예시
// src/features/auth/api/authUtils.ts (신규 파일) +import { QueryClient } from '@tanstack/react-query'; +import { NavigateFunction } from 'react-router'; +import { ROUTE } from '@/shared/config/route'; + +export const clearAuthAndRedirect = ( + queryClient: QueryClient, + navigate: NavigateFunction, +) => { + queryClient.clear(); + navigate(ROUTE.login); +};// useUnregister.ts onSuccess: () => { - queryClient.clear(); - navigate(ROUTE.login); + clearAuthAndRedirect(queryClient, navigate); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/auth/api/useUnregister.ts` around lines 6 - 17, Both useUnregister and useLogout duplicate the same onSuccess behavior (calling queryClient.clear() then navigate(ROUTE.login)); extract that shared logic into a small helper (e.g., handleAuthSuccess) and call it from both mutation onSuccess handlers. Locate the useUnregister and useLogout hooks, move the sequence queryClient.clear() + navigate(ROUTE.login) into the helper and have each hook invoke handleAuthSuccess(queryClient, navigate) or similar so the onSuccess bodies become a single call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/auth/api/useLogout.ts`:
- Around line 12-15: The logout handler in useLogout (onSuccess) calls
navigate(ROUTE.login) which pushes a new history entry; change it to
navigate(ROUTE.login, { replace: true }) so the login page replaces the current
history entry to prevent back-navigation to authenticated pages; apply the same
change in the useUnregister hook where navigate(ROUTE.login) is used, leaving
queryClient.clear() intact.
In `@src/features/auth/ui/UnregisterButton/index.tsx`:
- Around line 7-11: Wrap the immediate call to unregister() behind a user
confirmation step: replace the inline onClick={() => unregister()} on S.Button
in the UnregisterButton component with an onClick handler that opens a
confirmation dialog/modal (or uses window.confirm) and only calls unregister()
if the user explicitly confirms; ensure isPending still disables the button and
shows '탈퇴 중...' while the API runs, and keep the existing unregister() call and
error/cleanup handling unchanged. Use or create a local state like isConfirmOpen
or a reusable ConfirmDialog component to present the confirmation UI so
accidental clicks do not trigger immediate account deletion.
In `@src/pages/my-edit/ui/TermsLink/index.tsx`:
- Line 5: The anchor currently uses an empty href which navigates to the root;
update TermsLink (S.Link in src/pages/my-edit/ui/TermsLink/index.tsx) to prevent
unintended navigation by removing the empty href (omit href or set it to
undefined) and instead handle clicks via an onClick handler or use role="button"
with aria-disabled when the terms page is not implemented; ensure any onClick
calls event.preventDefault() if you keep an href-like element and add
appropriate accessibility attributes (aria-disabled) so the link is
non-navigable until the real terms route/component (or router Link) is
available.
In `@src/shared/ui/Divider/index.tsx`:
- Around line 1-8: The file uses the React namespace type React.HTMLAttributes
in the DividerProps interface but doesn't import React, causing a TS error; fix
by adding a named import from 'react' (import { HTMLAttributes } from 'react')
and update DividerProps to extend HTMLAttributes<HTMLDivElement> (or
alternatively import React as a namespace), so update the DividerProps and keep
the Divider function signature as-is to use the imported type.
---
Nitpick comments:
In `@src/features/auth/api/useUnregister.ts`:
- Around line 6-17: Both useUnregister and useLogout duplicate the same
onSuccess behavior (calling queryClient.clear() then navigate(ROUTE.login));
extract that shared logic into a small helper (e.g., handleAuthSuccess) and call
it from both mutation onSuccess handlers. Locate the useUnregister and useLogout
hooks, move the sequence queryClient.clear() + navigate(ROUTE.login) into the
helper and have each hook invoke handleAuthSuccess(queryClient, navigate) or
similar so the onSuccess bodies become a single call.
In `@src/pages/my-edit/ui/TermsLink/index.styles.ts`:
- Around line 4-7: The Link styled component (export const Link = styled.a) uses
default inline display so vertical padding may not take effect; update the Link
styles to include display: inline-block (or display: block if full-width
behavior is desired) so padding-top/padding-bottom are applied consistently like
the LogoutButton/UnregisterButton; keep the existing padding and
TextVariant('body1R') but add the display rule to the component.
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Contributor
Author
|
@coderabbitai approve |
✅ Actions performedComments resolved and changes approved. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#23 에 이어서 작업해서 base branch를 그쪽에 맞춰두었습니다!
머지 전에 develop으로 바꿔둘게요!!
💻 작업 내용
정보 수정 페이지를 구현했습니다
이용 약관 페이지 버튼은 구현했는데, 저희가 아직 이용 약관 페이지 관련해서는 얘기를 해 본적이 없는 것 같아서 우선은 구현만 해두고 주석처리 해두었습니다!
📸 스크린샷
👻 리뷰 요구사항
개발하다보니 예전에 나름대로 정리했던 디렉토리 구조나 코드 구조가 여전히 좀 뒤죽박죽인 느낌이 들었습니다... (함수/변수 네이밍이나 api 함수 구조 등등...)
readme에 정리해뒀던 디렉토리 구조도 수정할 겸 다시 코드나 디렉토리 구조 일관성 부분을 확인해보면 좋겠다는 생각이 들어서,
남은 기능 개발하면서 좀 고민해보다가 나중에 여유 생기면 한번 살펴봐도 좋을 것 같습니다!
Summary by CodeRabbit
릴리스 노트
New Features
UI Improvements
Chores