Conversation
📝 WalkthroughWalkthrough인증 플로우 전체를 재구조화하고 공통 컴포넌트(Input, Button, Modal)를 신규 추가하거나 재배치했습니다. 단계별 인증 페이지를 모듈화하고, 스키마와 컴포넌트 이름을 더 명확하게 정리했으며, 모달 관리 방식을 객체 기반에서 인자 기반으로 개선했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Page as FindPw Page
participant EmailVerification as EmailVerificationStep
participant Hook as useEmailVerification
participant AuthStore as Auth Store
participant API as API Server
User->>Page: 비밀번호 찾기 진입
Page->>EmailVerification: onNext 핸들러 전달
User->>EmailVerification: 이메일 입력
EmailVerification->>Hook: register('email'), form 상태 관리
User->>EmailVerification: 인증번호 받기 클릭
EmailVerification->>API: postSendCode(email)
API-->>EmailVerification: 인증번호 전송 완료
EmailVerification->>EmailVerification: sendCode=true, 타이머 시작
User->>EmailVerification: 인증번호 입력
EmailVerification->>Hook: register('code'), 입력값 유효성 검증
User->>EmailVerification: 다음 클릭
EmailVerification->>API: postVerifyCode(email, code)
API-->>EmailVerification: 검증 성공
EmailVerification->>Page: onNext() 콜백 실행
Page->>Page: step = 2로 변경
Page->>NewPasswordStep: 비밀번호 리셋 폼 렌더링
sequenceDiagram
participant User
participant Page as Signup Page
participant EnterEmailStep as EnterEmailStep
participant PasswordSetupStep as PasswordSetupStep
participant ProfileSetupStep as ProfileSetupStep
participant Hook as useStepNavigation
participant AuthStore as Auth Store
User->>Page: 회원가입 진입 (step=1)
Page->>EnterEmailStep: step 1 렌더링
User->>EnterEmailStep: 이메일 입력 및 확인
EnterEmailStep->>EnterEmailStep: 이메일 검증
User->>EnterEmailStep: 다음 클릭
EnterEmailStep->>Hook: handleNext() 호출
Hook->>Page: step=2로 업데이트
Page->>PasswordSetupStep: step 2 렌더링
User->>PasswordSetupStep: 비밀번호 입력
User->>PasswordSetupStep: 다음 클릭
PasswordSetupStep->>AuthStore: setPassword(password) 호출
PasswordSetupStep->>Hook: handleNext() 호출
Hook->>Page: step=3으로 업데이트
Page->>ProfileSetupStep: step 3 렌더링
User->>ProfileSetupStep: 프로필 정보 입력
User->>ProfileSetupStep: 가입 완료 클릭
ProfileSetupStep->>AuthStore: 프로필 정보 저장
ProfileSetupStep->>Page: 가입 완료
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분 리뷰 의견구조 개선 - 긍정적 평가컴포넌트 모듈화가 잘 진행되었습니다. CommonAuthInput을 Input 기반으로 리팩토링하고, InputActions로 버튼/타이머/검증 상태를 분리한 점이 좋습니다. PasswordForm으로 재사용성을 높인 것도 명확합니다. 스텝 네비게이션 훅 추가(useStepNavigation)로 상태 관리가 단순화되었습니다. 페이지별로 로컬 useState를 제거하고 일관된 인터페이스를 사용하니 로직이 읽기 쉬워집니다. 주의할 부분1. Modal openModal 시그니처 변경의 영향도 확인 필요 // 변경 전: openModal({ modalType: 'PRIVACY', modalProps: {...} })
// 변경 후: openModal('PRIVACY', {...})ProfileSetupStep에서 이 변경이 적용되었는데, 다른 곳에서 Modal을 사용하는 부분이 있다면 모두 마이그레이션되었는지 확인하세요. 특히 레거시 코드에서 놓친 부분이 있을 수 있습니다. 2. PasswordForm의 의존성 확인 3. useEmailVerification 훅의 타입 안정성 4. 에러 핸들링 강화 - 좋은 방향 5. CSS 토큰 확장 시 일관성 소소하지만 중요한 점
최종 체크리스트
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (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: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/pages/auth/RedirectPage.tsx (1)
30-31:⚠️ Potential issue | 🟠 Major
login()호출에 하드코딩된 이메일이 사용되고 있어요.
useTokenRefresh.ts의 Line 15와 동일하게"social@user.com"이라는 placeholder 이메일이 사용되고 있습니다. 소셜 로그인 리다이렉트 시 실제 사용자 이메일을 서버 응답이나 쿠키에서 받아 사용해야 할 것 같습니다. TODO 주석이 있지만, 이 상태로 배포되면 모든 소셜 로그인 사용자의 이메일이 동일하게 저장됩니다.src/components/auth/flows/signup/ProfileSetupStep.tsx (2)
106-110:⚠️ Potential issue | 🟡 Minor
watch("terms")가undefined를 반환할 수 있습니다.
checked속성에undefined가 전달되면 React에서 uncontrolled → controlled 전환 경고가 발생할 수 있습니다.watch("terms") || false또는!!watch("terms")로 방어 처리를 추가하면 안전합니다.🛡️ 수정 제안
<input type="checkbox" className="checkbox pointer-events-none" - checked={watch("terms")} + checked={watch("terms") ?? false} readOnly />
86-123: 🛠️ Refactor suggestion | 🟠 Major클릭 가능한
<div>에 키보드 접근성이 누락되어 있습니다.개인정보 동의 영역이
<div onClick>으로 구현되어 있어 키보드 사용자는 접근할 수 없습니다.role="button",tabIndex={0},onKeyDown(Enter/Space) 처리를 추가하거나<button>으로 감싸는 것을 권장합니다. 코딩 가이드라인에서도 시맨틱 HTML과 ARIA 속성 사용을 요구하고 있습니다.♻️ 수정 제안
<div - className="flex items-center mt-3 pl-1 w-full justify-between cursor-pointer" + role="button" + tabIndex={0} + className="flex items-center mt-3 pl-1 w-full justify-between cursor-pointer" onClick={() => { openModal(MODAL_TYPES.PRIVACY, { ... }); }} + onKeyDown={(e) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + openModal(MODAL_TYPES.PRIVACY, { ... }); + } + }} >As per coding guidelines, "접근성: 시맨틱 HTML, ARIA 속성 사용 확인."
src/components/auth/flows/find-email/EnterPhoneStep.tsx (2)
52-56:⚠️ Potential issue | 🟠 Major하드코딩된 이메일 주소 — 테스트 데이터가 코드에 남아 있습니다
"smu2021@naver.com"은 실제 사용자의 이메일로 보이며, 프로덕션 코드에 하드코딩되어 있습니다. 개인정보 노출 위험이 있고, 실제 API 연동 시 반드시 제거해야 합니다. 최소한 TODO 또는 FIXME 주석으로 명시적으로 표시해 주세요. As per coding guidelines, "compliance/privacy risks (PII retention, logging sensitive data)".♻️ 수정 제안
const onSubmit: SubmitHandler<TStep01FormValues> = async () => { - // 임시 이메일 - setEmail("smu2021@naver.com"); + // TODO: API 연동 후 실제 이메일로 교체 필요 + setEmail("test@example.com"); onNext(); };
111-111:⚠️ Potential issue | 🔴 Critical타이머가 하드코딩된 정적 문자열입니다
timer={sendCode ? "03:00" : undefined}은 실제로 카운트다운하지 않습니다.EnterEmailStep에서는useEmailVerification훅의useTimer를 통해formattedTime을 동적으로 관리하고 있는데, 이 컴포넌트에는 타이머 로직이 전혀 없습니다. 정적 문자열로 표시되면 사용자는 실제로 시간이 남아있다고 착각할 수 있으며, 인증번호의 유효성 상태를 올바르게 전달할 수 없습니다.
useEmailVerification처럼 타이머 관리 로직이 필요합니다.useTimer훅을 활용하거나usePhoneVerification훅을 새로 만들어 동적인 타이머를 구현해주세요.
🤖 Fix all issues with AI agents
In `@src/components/auth/common/InputActions.tsx`:
- Around line 35-45: The rendered Button can appear clickable when `button` is
true but `onButtonClick` is undefined; update the rendering in InputActions.tsx
so the Button's interactivity reflects that: include `onButtonClick` existence
in the disabled condition (e.g., treat missing `onButtonClick` as disabled) and
only pass `onClick={onButtonClick}` when `onButtonClick` is defined (or
vice‑versa require/throw if a button is requested without a handler). Adjust the
disabled expression that currently uses `type`, `error`, and `validation` to
also check `!onButtonClick` and ensure the Button receives no `onClick` prop
when the handler is absent.
In `@src/components/auth/common/PasswordForm.tsx`:
- Around line 42-43: The title prop of PasswordForm is rendered inside an <h1>,
so callers must not pass block-level elements (like <p>); update the
PasswordForm component documentation and API and fix callers: add a JSDoc
comment above the PasswordForm component and its prop (title) stating it must be
plain text or inline content for use inside an h1, or rename the prop to
titleText to make intent explicit, and then update PasswordSetupStep and
NewPasswordStep to pass a plain string or inline JSX (remove surrounding <p>
tags). Ensure the prop type reflects this expectation (e.g., string or
React.ReactNode with the JSDoc constraint) and adjust any imports/usages
accordingly.
In `@src/components/auth/flows/reset-password/EmailVerificationStep.tsx`:
- Around line 34-37: The heading in EmailVerificationStep.tsx uses an <h1
className="text-start font-heading2 text-text-main mb-10"> that incorrectly
nests two <p> elements; replace those nested <p> tags with either <span
className="block"> elements (preserving the visual block layout and existing
classes) or use inline text with <br /> between lines so the <h1> remains
semantically valid, and ensure the change retains the same className and
accessibility/ARIA considerations for the EmailVerificationStep component.
In `@src/components/auth/flows/reset-password/NewPasswordStep.tsx`:
- Around line 22-27: The title prop in NewPasswordStep currently renders two <p>
elements which will be nested inside an <h1> in PasswordForm; replace those <p>
tags with a single fragment that uses a <br /> between the two lines (i.e.,
render the same text but using line breaks rather than <p> elements) so the
output inside PasswordForm's <h1> is valid HTML; update the JSX in
NewPasswordStep where the title prop is defined accordingly.
- Around line 12-18: handleSubmit currently only calls setPassword and shows a
success toast without calling the backend; add a password-reset API function
(e.g., export async function resetPassword(email: string, password: string) in
the auth API module) that sends { email, password } to the server, then update
handleSubmit to: retrieve the verified email from your reset-password store (the
same store that holds setPassword/verifiedEmail), call await
resetPassword(email, password), handle success by calling setPassword(password),
showing the toast and navigate("/login"), and handle errors by showing an error
toast; also add basic request-state handling (disable submit while pending) and
proper try/catch around the API call in handleSubmit.
In `@src/components/auth/flows/signup/EnterEmailStep.tsx`:
- Around line 88-93: The code input field and the "Next" button have
inconsistent disabled conditions. The error message condition correctly checks
both isExpired and sendCode, but the disabled attributes on the input field and
button only check isExpired. Update the disabled property on the code input
field to include !sendCode in addition to isExpired (line 87), and similarly
update the disabled property on the next/submit button to include !sendCode in
addition to the existing conditions like !isValid, isPending, and isExpired
(line 103). This ensures that both the input field and button are properly
disabled when no authentication code has been sent or when the code has expired.
In `@src/components/auth/flows/signup/PasswordSetupStep.tsx`:
- Around line 17-28: The title prop passed to PasswordForm in PasswordSetupStep
renders inside an <h1>, so replace the current <p> elements with inline/valid
alternatives (e.g., use <br /> where line breaks are needed or use <span
className="block"> for block-like behavior) to produce valid HTML; update the
JSX passed to the title prop in the PasswordSetupStep component (the
PasswordForm title prop) accordingly while keeping buttonText and onSubmit
(handleSubmit) unchanged.
In `@src/components/common/button/Button.tsx`:
- Around line 35-36: Update the Tailwind gradient utility in the Button
component: inside the gradient string (in
src/components/common/button/Button.tsx, symbol name "gradient") replace the v3
utility "bg-gradient-to-r" with the Tailwind v4 syntax "bg-linear-to-r" so the
class becomes bg-linear-to-r while keeping the rest of the string (from-logo-1
to-logo-2 text-white ... disabled:opacity-100) unchanged.
In `@src/components/common/input/Input.tsx`:
- Around line 56-64: The focus ring color defined by
"focus-within:ring-logo-1/30" conflicts with the error ring ("ring-status-red")
so focusing an errored Input loses the red ring; update the className
composition in the Input component to conditionally apply focus-within styles
based on the error flag (either omit "focus-within:ring-logo-1/30" when error is
true or replace it with "focus-within:ring-status-red"), i.e. adjust the ternary
that uses error and success (references: error, success, className composition,
containerClassName, and the "focus-within:ring-logo-1/30" / "ring-status-red"
tokens) so error state retains the error ring on focus.
In `@src/components/common/modal/Modal.tsx`:
- Around line 106-109: The modal currently sets aria-labelledby="modal-title"
but never renders an element with that id and may cause collisions; update the
Modal component to generate a unique id via React's useId (e.g., modalTitleId =
useId()), render the title element (h2 or appropriate heading) inside the modal
when the title prop is provided with id={modalTitleId} placed above children,
and set aria-labelledby={title ? modalTitleId : undefined} so screen readers
reference a real, unique element and avoid id conflicts when multiple modals are
open.
In `@src/pages/auth/RedirectPage.tsx`:
- Line 9: RedirectPage.tsx is setting a placeholder email and not populating the
user's socialId when calling useAuthStore().login, leaving socialId as -1;
update the social login flow to extract the real email and socialId from the
social provider response and pass them into the auth store (either call login
with the actual email and socialId or call setSocialId(...) after login),
replacing the hardcoded "social@user.com" and ensuring accessToken, email, and
socialId (symbols: RedirectPage.tsx, useAuthStore, login, setSocialId, socialId,
accessToken, "social@user.com") are all properly set from the provider response.
In `@src/pages/auth/Signup.tsx`:
- Around line 7-9: The imports are using old numeric aliases (Step01Email,
Step02Password, Step03Profile) instead of the new action-based component names;
replace those aliased imports with the actual component identifiers
EnterEmailStep, PasswordSetupStep, ProfileSetupStep and update any local
references/usages in this file (e.g., where
Step01Email/Step02Password/Step03Profile are rendered or passed around) to use
EnterEmailStep, PasswordSetupStep, and ProfileSetupStep so names are consistent
with the refactor.
In `@src/store/useModalStore.ts`:
- Around line 19-24: TModalPayload 타입은 프로젝트에서 사용되지 않으므로
src/store/useModalStore.ts에서 TModalPayload의 export 및 정의를 제거하세요; openModal (제네릭
<T extends TModalType>)과 TModalState/TModalType/TGlobalModalProps 구조가 타입 안전성을
제공하므로 TModalPayload를 삭제하면 됩니다, 삭제 후에 참조되는 곳이 없는지 전역 검색하고 타입체크(tsc)를 실행해 빌드 에러가
없는지 확인하세요.
🧹 Nitpick comments (27)
src/hooks/auth/useSocialLogin.ts (1)
5-8: 환경 변수가 모두 누락된 경우에 대한 방어 처리가 필요해요.
VITE_API_TARGET_URL과VITE_API_BASE_URL둘 다 설정되지 않은 경우,API_TARGET_URL이undefined가 되어"undefined/oauth2/authorization/..."같은 잘못된 URL로 리다이렉트될 수 있습니다. 개발 환경에서.env설정 누락 시 디버깅이 어려울 수 있으니, 간단한 가드를 추가하는 걸 권장드려요.또한, 환경 변수 값 끝에 trailing slash(
/)가 포함되어 있으면//oauth2/...같은 이중 슬래시 URL이 만들어질 수 있어요.♻️ 방어 코드 제안
const handleSocialLogin = (platform: TSocialLoginPlatform) => { const API_TARGET_URL = import.meta.env.VITE_API_TARGET_URL || import.meta.env.VITE_API_BASE_URL; + if (!API_TARGET_URL) { + console.error("OAuth API URL이 설정되지 않았습니다. 환경 변수를 확인해주세요."); + return; + } + - window.location.href = `${API_TARGET_URL}/oauth2/authorization/${platform}`; + const baseUrl = API_TARGET_URL.replace(/\/+$/, ""); + window.location.href = `${baseUrl}/oauth2/authorization/${platform}`; };src/components/common/skeleton/Skeleton.tsx (1)
8-24: 접근성 개선 제안: 스켈레톤 컴포넌트에aria속성 고려스켈레톤 로더는 시각적 플레이스홀더이므로, 스크린 리더 사용자에게는 불필요한 정보가 될 수 있어요. 기본적으로
aria-hidden="true"를 적용하고, 필요 시 소비자 쪽에서 오버라이드할 수 있도록 하면 접근성이 좋아집니다....props스프레드가 뒤에 있으므로 소비자가 원하면 덮어쓸 수 있어요.♿ 접근성 개선 제안
export function Skeleton({ className, ...props }: ISkeletonProps) { return ( <div + aria-hidden="true" className={twMerge("animate-shimmer rounded bg-gray-200", className)} {...props} /> ); } export function SkeletonCircle({ className, ...props }: ISkeletonProps) { return ( <div + aria-hidden="true" className={twMerge("animate-shimmer rounded-full bg-gray-200", className)} {...props} /> ); }As per coding guidelines, "접근성: 시맨틱 HTML, ARIA 속성 사용 확인."
src/hooks/auth/useTokenRefresh.ts (2)
17-19: 토큰 재발급 실패 시 에러 로깅이 완전히 제거되었어요.
catch블록에서 에러 파라미터와console.error가 모두 제거되면서, 토큰 재발급 실패 원인을 추적할 수 없게 되었습니다. 네트워크 오류, 서버 오류, 토큰 만료 등 다양한 실패 원인을 디버깅하려면 최소한의 로깅은 유지하는 게 좋겠습니다.🔧 에러 로깅 추가 제안
- } catch { + } catch (error) { + console.error("토큰 재발급 실패:", error); logout(); }
14-15: 하드코딩된 이메일 값이 TODO로 남아있어요.
login("user@example.com", data.accessToken)— 토큰 재발급 응답에서 실제 사용자 이메일을 받아 사용해야 할 것 같은데, 현재 placeholder 값이 그대로 유지되고 있습니다. 이 부분은 별도 이슈로 추적하고 계신가요?이 TODO 항목을 별도 이슈로 생성해드릴까요?
src/components/auth/intro/IntroAdManagement.tsx (1)
1-1: 에셋 파일명에 오타가 있어요:logo_goole.svg기존부터 있던 이슈이긴 하지만,
logo_goole.svg→logo_google.svg로 수정이 필요해 보입니다. 리팩토링 PR이니 함께 정리하면 좋을 것 같아요.src/components/auth/intro/IntroAIAnalytics.tsx (1)
22-29:bars배열이 매 렌더마다 재생성되고 있어요.
bars는 정적 데이터이므로 컴포넌트 외부 상수로 추출하면 불필요한 객체 생성을 방지할 수 있습니다. 기존 코드에서부터의 이슈이지만, 리팩토링 PR이니 참고 차원에서 남깁니다.♻️ 상수 추출 제안
+const BARS = [ + { height: "25%", delay: "0ms", isBlue: false }, + { height: "45%", delay: "100ms", isBlue: false }, + { height: "35%", delay: "200ms", isBlue: false }, + { height: "90%", delay: "300ms", isBlue: true }, + { height: "50%", delay: "400ms", isBlue: false }, + { height: "80%", delay: "550ms", isBlue: false }, +]; + export default function IntroAIAnalytics({ isActive }: { isActive: boolean }) { const [showBubble, setShowBubble] = useState(false); // ... - const bars = [ - { height: "25%", delay: "0ms", isBlue: false }, - // ... - ];src/components/auth/intro/OnboardingIntro.tsx (1)
3-5: import alias가 새로운 컴포넌트 이름과 일치하지 않아요.파일명은
IntroAdManagement,IntroAIAnalytics,IntroLogo로 의미 있게 리네이밍했는데, import에서 여전히IntroSlide1/2/3으로 alias하고 있어서 리팩토링 효과가 반감됩니다. 새 이름을 그대로 사용하면 코드 가독성이 훨씬 좋아질 것 같아요.♻️ 제안
-import IntroSlide2 from "@/components/auth/intro/IntroAdManagement"; -import IntroSlide3 from "@/components/auth/intro/IntroAIAnalytics"; -import IntroSlide1 from "@/components/auth/intro/IntroLogo"; +import IntroAdManagement from "@/components/auth/intro/IntroAdManagement"; +import IntroAIAnalytics from "@/components/auth/intro/IntroAIAnalytics"; +import IntroLogo from "@/components/auth/intro/IntroLogo";JSX 부분도 함께 수정:
- <IntroSlide1 isActive={currentSlide === 0} /> - <IntroSlide2 isActive={currentSlide === 1} /> - <IntroSlide3 isActive={currentSlide === 2} /> + <IntroLogo isActive={currentSlide === 0} /> + <IntroAdManagement isActive={currentSlide === 1} /> + <IntroAIAnalytics isActive={currentSlide === 2} />src/components/auth/common/InputActions.tsx (1)
5-14: 타입 안정성 개선이 필요해요.몇 가지 타입 관련 피드백입니다:
type?: string—"code" | "email" | ...같은 union type으로 좁히는 게 좋겠어요. Line 38에서type === "code"매직 스트링으로 분기하고 있어서, 오타나 잘못된 값이 들어오면 의도치 않은 동작이 발생할 수 있어요.button?: boolean—showButton같은 이름이 역할을 더 명확하게 전달합니다.button이true인데buttonText나onButtonClick이undefined일 수 있는 상태입니다. 버튼을 보여줄 때 필수인 props를 discriminated union이나 조건부 타입으로 묶어주면 실수를 방지할 수 있어요.♻️ 타입 개선 예시
+type InputActionType = "email" | "code" | "phone"; + interface IInputActionsProps { - button?: boolean; - buttonText?: string; - onButtonClick?: () => void; + showButton?: boolean; + buttonText?: string; + onButtonClick?: () => void; validationState?: string; timer?: string; validation?: boolean; error?: boolean; - type?: string; + type?: InputActionType; }src/components/common/button/Button.tsx (2)
23-38:sizeClasses와variantClasses객체를 컴포넌트 외부로 추출하면 불필요한 재생성을 피할 수 있어요.매 렌더마다 동일한 객체가 새로 생성됩니다. 성능에 큰 영향은 없지만, 상수이므로 컴포넌트 바깥에 선언하는 것이 관용적입니다.
40-49:sizeClasses와variantClasses객체를 컴포넌트 외부로 이동해주세요.
transition-smooth와active-scale커스텀 유틸리티는src/index.css에 정의되어 있어 문제없습니다.다만 현재 23-38번 라인의
sizeClasses와variantClasses객체가 매번 렌더링될 때마다 재생성되고 있습니다. 이는 불필요한 메모리 할당과 참조 비교 비용을 발생시킵니다. 컴포넌트 외부로 호이스팅하면 성능을 개선할 수 있습니다:const sizeClasses = { big: "h-button-big px-6 rounded-component-md font-heading3", small: "h-button-small px-4 rounded-component-md font-body1", }; const variantClasses = { primary: "bg-brand-800 text-white hover:bg-brand-700 disabled:bg-bg-disabled disabled:text-text-disabled disabled:hover:bg-bg-disabled", // ... 나머지 variants }; export default function Button({ ... }) { // ... }src/components/common/input/Input.tsx (1)
20-103: React 19에서는forwardRef없이ref를 일반 prop으로 받을 수 있어요.React 19.2.0을 사용하고 계시니 참고로 알려드려요.
forwardRef가 deprecated은 아니지만, React 19부터는ref를 일반 prop으로 전달할 수 있어서forwardRef래핑 없이도 동일하게 동작합니다. 지금 구현도 정상 동작하니 급하지 않게 전환하셔도 됩니다.src/components/common/modal/Modal.tsx (3)
50-59: ESC 이벤트 리스너가 모달이 닫혀 있을 때에도 항상 등록돼요.
isOpen이false여도keydown리스너가 document에 등록됩니다. 핸들러 내부에서isOpen체크를 하고 있지만, 불필요한 리스너 등록을 피하는 것이 더 깔끔해요.♻️ isOpen일 때만 리스너 등록
useEffect(() => { + if (!isOpen) return; + const handleEscape = (e: KeyboardEvent) => { - if (e.key === "Escape" && isOpen) { + if (e.key === "Escape") { onClose(); } }; document.addEventListener("keydown", handleEscape); return () => document.removeEventListener("keydown", handleEscape); }, [isOpen, onClose]);
35-47: 포커스 복원 로직에서isOpen이false로 바뀔 때의 cleanup이 아닌 else 분기에서 복원하고 있어요.현재 구조에서는
isOpen이false로 전환될 때 else 분기가 실행되어 포커스를 복원하는데, Line 79에서!isOpen이면null을 반환하므로 컴포넌트가 언마운트됩니다. 언마운트 시에는 effect cleanup에서 포커스를 복원하는 것이 더 안정적이에요.♻️ cleanup 함수에서 포커스 복원
useEffect(() => { if (isOpen) { previousActiveElement.current = document.activeElement as HTMLElement; setTimeout(() => { modalRef.current?.focus(); }, 0); - } else { - if (previousActiveElement.current) { - previousActiveElement.current.focus(); - } } + + return () => { + if (previousActiveElement.current) { + previousActiveElement.current.focus(); + previousActiveElement.current = null; + } + }; }, [isOpen]);
101-135: 포커스 트랩이 없어서 Tab 키로 모달 바깥 요소에 포커스가 이동할 수 있어요.
aria-modal="true"와role="dialog"를 잘 적용했지만, 실제 포커스 트랩이 구현되지 않아 키보드 사용자가 Tab으로 모달 밖 요소에 접근할 수 있습니다. WAI-ARIA 모달 패턴에서는 포커스가 모달 내에서 순환해야 합니다.
focus-trap-react같은 라이브러리를 활용하거나, 직접 첫/마지막 focusable 요소에서 Tab 순환을 구현하는 것을 고려해주세요. 당장은 아니더라도 향후 접근성 개선 시 반영하면 좋겠습니다.src/components/modal/privacyModal/PrivacyModal.tsx (1)
66-73:isOpen={true}하드코딩 — 부모에서 마운트/언마운트로 제어하는 패턴이죠?
isOpen이 항상true이면 Modal 내부의isOpen기반 로직(ESC 리스너, 포커스 관리 등)은 정상 동작하지만, 닫을 때 부모에서 컴포넌트 자체를 언마운트하는 방식에 의존하게 됩니다. 현재 구조에서 의도된 동작이라면 괜찮지만, Modal의isOpen상태를 부모에서 관리하는 방식도 고려해볼 수 있어요.src/index.css (1)
7-22:html, body와body블록 간 중복 속성이 있어요.
overflow-x: hidden,margin: 0,padding: 0이 두 블록에서 반복되고,height설정도100dvh→min-height: 100dvh+height: 100%로 혼재되어 있어요. 하나로 통합하면 유지보수가 편해질 것 같습니다.♻️ 통합 예시
html, body { height: 100dvh; overflow-x: hidden; - overflow-y: auto; margin: 0; padding: 0; } body { min-height: 100dvh; - height: 100%; - overflow-x: hidden; - margin: 0; - padding: 0; + overflow-y: auto; }src/components/auth/flows/reset-password/EmailVerificationStep.tsx (2)
80-96: 인증번호 발송 전에도 코드 입력이 활성화되어 있어요.
sendCode가false일 때 코드 입력 필드가disabled되지 않아서, 인증번호를 받기 전에 코드를 입력할 수 있습니다. 사용자 혼란을 줄이려면 발송 전에는 비활성화하는 게 좋을 것 같아요.♻️ 수정 제안
{...register("code")} - disabled={isExpired} + disabled={!sendCode || isExpired} error={!!errors.code || !!codeError || (isExpired && sendCode)}
39-109:<form>요소가 없어서 Enter 키로 제출이 불가능해요.현재 구조에서 "다음으로" 버튼이
onClick으로만 동작하고<form>래핑이 없어서, 키보드 사용자가 Enter 키로 제출할 수 없습니다. 접근성 개선을 위해<form onSubmit={handleSubmit}>으로 감싸는 걸 고려해 주세요.As per coding guidelines, "시맨틱 HTML, ARIA 속성 사용 확인".
src/components/auth/common/CommonAuthInput.tsx (1)
108-115:e.target.value직접 변경은 React에서 주의가 필요한 패턴이에요.
formatPhoneNumber적용 시 synthetic event의e.target.value를 직접 변경하고 있는데, react-hook-form의register와 함께 사용할 때는 동작하지만, React의 controlled input 패턴과는 다소 다릅니다. 현재 동작에 문제는 없지만, 향후 React 버전에서 synthetic event 풀링이나 동작이 바뀔 경우를 대비해 알아두면 좋겠어요.src/pages/auth/Login.tsx (1)
38-49:mutate호출 시 inlineonSuccess/onError대신useMutation옵션으로 분리하는 것도 고려해 보세요.현재
mutate호출 시 inline으로 콜백을 전달하고 있는데, 이 패턴은 컴포넌트가 언마운트되면 콜백이 실행되지 않을 수 있어요.useMutation선언 시 옵션으로 두면 항상 실행이 보장됩니다. 다만 현재 로그인 플로우에서는 큰 문제가 되진 않을 것 같습니다.src/components/auth/flows/signup/ProfileSetupStep.tsx (1)
64-67:<h1>안에<p>태그 사용은 유효하지 않은 HTML입니다.
<p>는 블록 레벨 요소이므로<h1>내부에 중첩할 수 없습니다. 줄바꿈이 필요하다면<br />또는<span>+ CSS를 사용하는 게 적절합니다. 이 패턴이 다른 step 컴포넌트에도 동일하게 적용되어 있으니 일괄 수정을 고려해 주세요.♻️ 수정 제안
<h1 className="text-start font-heading2 text-text-main mb-10"> - <p>사용자의</p> - <p>기본 정보를 입력해 주세요</p> + <span className="block">사용자의</span> + <span className="block">기본 정보를 입력해 주세요</span> </h1>src/components/modal/ModalProvider.tsx (1)
23-27: 모달 타입이 추가될 때 확장성을 고려해 주세요.현재는
PRIVACY하나뿐이라 switch-case가 간결하지만, 모달 타입이 늘어나면 이 파일을 매번 수정해야 합니다. 지금 당장은 문제가 아니지만, 향후 모달이 3~4개 이상이 되면 컴포넌트 맵 기반 레지스트리로 전환하는 것도 고려해 볼 만합니다.src/pages/auth/FindEmail.tsx (1)
3-4: import alias가 실제 컴포넌트 이름과 불일치합니다.파일명이
EnterPhoneStep과ShowEmailResultStep으로 변경되었는데, import alias는 여전히Step01Phone,Step02Email을 사용하고 있습니다. 이번 리팩토링의 목적이 액션 기반 네이밍 전환인 만큼, alias도 맞춰주면 일관성이 높아집니다.♻️ 수정 제안
-import Step01Phone from "@/components/auth/flows/find-email/EnterPhoneStep"; -import Step02Email from "@/components/auth/flows/find-email/ShowEmailResultStep"; +import EnterPhoneStep from "@/components/auth/flows/find-email/EnterPhoneStep"; +import ShowEmailResultStep from "@/components/auth/flows/find-email/ShowEmailResultStep";JSX도 함께 업데이트:
- {step === 1 && <Step01Phone onNext={handleNext} />} - {step === 2 && <Step02Email />} + {step === 1 && <EnterPhoneStep onNext={handleNext} />} + {step === 2 && <ShowEmailResultStep />}src/pages/auth/FindPw.tsx (1)
3-4:FindEmail.tsx와 동일하게 import alias가 실제 컴포넌트 이름과 불일치합니다.
EmailVerificationStep과NewPasswordStep으로 변경된 파일에서 여전히Step01VerifyEmail,Step02ResetPassword로 alias하고 있습니다. 일관성을 위해 맞춰주시면 좋겠습니다.♻️ 수정 제안
-import Step01VerifyEmail from "@/components/auth/flows/reset-password/EmailVerificationStep"; -import Step02ResetPassword from "@/components/auth/flows/reset-password/NewPasswordStep"; +import EmailVerificationStep from "@/components/auth/flows/reset-password/EmailVerificationStep"; +import NewPasswordStep from "@/components/auth/flows/reset-password/NewPasswordStep";src/components/auth/flows/signup/EnterEmailStep.tsx (1)
32-35:<h1>내부에<p>태그 사용은 시맨틱 HTML 위반입니다
<p>는 블록 레벨 요소로,<h1>안에 중첩하면 HTML 명세에 맞지 않습니다.<span>+<br />또는<span className="block">으로 변경하는 것이 적절합니다. 이 패턴이 다른 Step 컴포넌트에서도 반복되므로 일괄 수정을 권장합니다. As per coding guidelines, "시맨틱 HTML, ARIA 속성 사용 확인".♻️ 수정 제안
<h1 className="text-start font-heading2 text-text-main mb-10"> - <p>회원가입을 위해</p> - <p>이메일 인증을 진행할게요</p> + <span className="block">회원가입을 위해</span> + <span className="block">이메일 인증을 진행할게요</span> </h1>src/components/auth/flows/find-email/EnterPhoneStep.tsx (1)
42-50:postSendCode에서 실제 API 호출 없이 상태만 변경됩니다현재
postSendCode는 유효성 검증 후setSendCode(true)와 toast만 표시합니다.EnterEmailStep의 경우useEmailVerification훅으로 로직이 분리되어 있는데, 이 컴포넌트는 로컬 상태로 직접 관리하고 있어 일관성이 떨어집니다. 추후 API 연동 시useEmailVerification과 유사한 커스텀 훅(usePhoneVerification등)으로 추출하는 것을 권장합니다. As per coding guidelines, "커스텀 훅으로의 분리 여부 검토".src/pages/auth/Signup.tsx (1)
33-41: step 렌더링 분기에서 잘못된 step 값에 대한 방어가 없습니다현재
step이 1, 2, 3이 아닌 다른 양수(예: 4 이상)가 되면 기본 회원가입 화면(소셜 로그인 버튼)이 다시 노출됩니다.useStepNavigation의handleNext는prev + 1로 무한 증가할 수 있으므로, Step03Profile에서handleNext가 호출되지 않더라도 방어적으로 상한을 두는 것이 좋습니다.
Seojegyeong
force-pushed
the
refactor/#35
branch
from
6c05591 to
df948cc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/auth/flows/signup/ProfileSetupStep.tsx (1)
64-67:⚠️ Potential issue | 🟡 Minor
<h1>내부에<p>태그가 있어 유효하지 않은 HTML입니다.
NewPasswordStep에서는<br />로 수정되었는데, 여기서는 아직<p>태그가 남아있습니다. 동일한 패턴으로 수정해주세요. As per coding guidelines, "시맨틱 HTML, ARIA 속성 사용 확인".🐛 수정 제안
<h1 className="text-start font-heading2 text-text-main mb-10"> - <p>사용자의</p> - <p>기본 정보를 입력해 주세요</p> + 사용자의 + <br /> + 기본 정보를 입력해 주세요 </h1>src/components/auth/flows/find-email/EnterPhoneStep.tsx (2)
69-72:⚠️ Potential issue | 🟡 Minor
<h1>내부에<p>태그 — 동일한 HTML 유효성 문제입니다.다른 스텝 컴포넌트들과 동일하게
<br />로 변경해주세요.🐛 수정 제안
<h1 className="text-start font-heading2 text-text-main mb-10"> - <p>이메일 찾기를 위해</p> - <p>휴대폰 인증을 진행할게요</p> + 이메일 찾기를 위해 + <br /> + 휴대폰 인증을 진행할게요 </h1>
104-115:⚠️ Potential issue | 🟠 Major타이머가 정적 문자열
"03:00"으로 고정되어 있어요.
useEmailVerification훅에서는useTimer를 활용하여 실제 카운트다운을 구현하고 있는데, 여기서는timer="03:00"이라는 고정 문자열만 전달합니다. 실제 만료 시간이 반영되지 않아 사용자에게 잘못된 정보를 줄 수 있습니다.
useEmailVerification훅의 패턴처럼useTimer를 도입하거나, API 응답의 만료 시간을 기반으로 동적 타이머를 적용하는 것이 필요합니다.
🤖 Fix all issues with AI agents
In `@src/components/auth/flows/find-email/EnterPhoneStep.tsx`:
- Around line 52-56: The onSubmit handler in EnterPhoneStep (function onSubmit:
SubmitHandler<TFindEmailFormValues>) currently sets a hardcoded email via
setEmail("smu2021@naver.com") then calls onNext(); remove the hardcoded value
and instead call the real API to fetch the user email (or await the service that
returns it) and pass that result into setEmail(...), and if the API integration
isn’t implemented yet replace the hardcoded string with a clear TODO comment
explaining that setEmail must be fed the API response and leave a lightweight
fallback (or disable progression) to avoid shipping a constant email; ensure the
code path still calls onNext() only after a successful response or appropriate
error handling.
🧹 Nitpick comments (9)
src/components/common/input/Input.tsx (1)
85-96: 성공 상태일 때 helperText 색상이 구분되지 않아요.현재
helperText색상은 에러일 때text-status-red, 그 외에는text-text-sub로 처리됩니다.success상태에서도 동일하게text-text-sub가 적용되어 시각적으로 성공 피드백이 전달되지 않습니다.♻️ success 상태 helperText 색상 추가
className={twMerge( "font-caption pl-1", - error ? "text-status-red" : "text-text-sub", + error ? "text-status-red" : success ? "text-logo-1" : "text-text-sub", )}src/components/common/modal/Modal.tsx (3)
42-54: 모달이 닫혀 있을 때도 focus restore 로직이 실행될 수 있어요.
isOpen이false로 바뀔 때previousActiveElement.current?.focus()가 호출되는데, 컴포넌트가 최초 마운트 시(isOpen=false)previousActiveElement가null이므로 현재는 문제가 없습니다. 다만isOpen이 여러 번 토글되는 시나리오에서 의도치 않은 포커스 이동이 발생할 수 있으니, cleanup 함수로 처리하는 것이 더 안전합니다.♻️ cleanup 기반 포커스 복귀
useEffect(() => { if (isOpen) { previousActiveElement.current = document.activeElement as HTMLElement; - setTimeout(() => { modalRef.current?.focus(); }, 0); - } else { - if (previousActiveElement.current) { - previousActiveElement.current.focus(); - } + + return () => { + previousActiveElement.current?.focus(); + }; } }, [isOpen]);
108-147: 포커스 트랩(focus trap)이 없어서 Tab 키로 모달 밖으로 이동할 수 있어요.현재
aria-modal="true"와 ESC 닫기, 오버레이 클릭 닫기는 잘 구현되어 있지만, 키보드 사용자가 Tab 키로 모달 외부 요소에 도달할 수 있습니다. 스크린 리더 사용자나 키보드 네비게이션 사용자에게 접근성 문제가 됩니다.
react-focus-lock같은 라이브러리를react-remove-scroll과 함께 사용하면 간단히 해결됩니다. As per coding guidelines, "시맨틱 HTML, ARIA 속성 사용 확인".
88-100:sizeClasses,paddingClasses매핑을 컴포넌트 외부로 추출하면 좋겠어요.이 객체들은 매 렌더마다 새로 생성됩니다. 상수이므로 컴포넌트 밖에 선언하면 불필요한 할당을 줄일 수 있습니다.
♻️ 상수 외부 추출
+const SIZE_CLASSES = { + sm: "max-w-modal-sm", + md: "max-w-modal-md", + lg: "max-w-modal-lg", + xl: "max-w-modal-xl", +} as const; + +const PADDING_CLASSES = { + none: "p-modal-none", + sm: "p-modal-sm", + md: "p-modal-md", + lg: "p-modal-lg", +} as const; + function Modal({그리고 내부에서
SIZE_CLASSES[size],PADDING_CLASSES[padding]으로 참조합니다.src/components/auth/flows/signup/ProfileSetupStep.tsx (1)
86-103:onClick으로 모달을 여는div에 키보드 접근성이 없어요.이
div는 클릭으로 모달을 열지만, 키보드 사용자는 이 영역에 포커스하거나 활성화할 수 없습니다.role="button",tabIndex={0},onKeyDown핸들러를 추가하거나, 시맨틱<button>으로 감싸는 것이 좋겠습니다. As per coding guidelines, "시맨틱 HTML, ARIA 속성 사용 확인".♻️ 접근성 개선 제안
<div className="flex items-center mt-3 pl-1 w-full justify-between cursor-pointer" + role="button" + tabIndex={0} onClick={() => { openModal(MODAL_TYPES.PRIVACY, { // ... }); }} + onKeyDown={(e) => { + if (e.key === "Enter" || e.key === " ") { + e.preventDefault(); + openModal(MODAL_TYPES.PRIVACY, { + // ...same props + }); + } + }} >또는 전체 영역을
<button type="button">으로 변경하는 것이 더 깔끔합니다.src/utils/validation.ts (1)
15-21:nameSchema에서.min(1)뒤에.min(2)가 있어 의도가 명확한지 확인해주세요.
.min(1, "이름은 필수입니다.")와.min(2, "이름은 2자 이상 입력해주세요.")가 동시에 존재합니다. Zod는 체인 순서대로 검증하므로, 빈 문자열 입력 시.min(1)메시지가, 1글자 입력 시.min(2)메시지가 나옵니다. 의도된 동작이라면 괜찮지만,.min(2)하나로 통합하고 빈 문자열은 별도 처리하는 것이 더 직관적일 수 있습니다.src/components/auth/common/PasswordForm.tsx (3)
6-11:signupPasswordSchema를 비밀번호 재설정 플로우에서도 공유한다면, 네이밍이 혼란을 줄 수 있어요.현재
PasswordForm은 회원가입뿐 아니라 비밀번호 재설정(NewPasswordStep)에서도 사용되는 공용 컴포넌트인데, 스키마와 타입 이름에signup이 붙어 있어서 의도가 불분명해질 수 있습니다.passwordFormSchema/TPasswordFormValues같은 범용적인 이름을 고려해 보시면 좋을 것 같아요.♻️ 네이밍 개선 제안 (validation.ts 측 변경 포함)
src/utils/validation.ts쪽에서 스키마 이름을 변경하고, 이 파일에서도 맞춰주는 형태입니다:-import { signupPasswordSchema } from "@/utils/validation"; +import { passwordFormSchema } from "@/utils/validation"; -type TSignupPasswordFormValues = z.infer<typeof signupPasswordSchema>; +type TPasswordFormValues = z.infer<typeof passwordFormSchema>;
40-44: 재사용 가능한 폼 컴포넌트에 페이지 레이아웃(min-h-screen)이 포함되어 있어요.
PasswordForm이 공용 컴포넌트로 설계되었지만,min-h-screen과flex items-center justify-center같은 페이지 레벨 레이아웃이 컴포넌트 내부에 있으면, 모달이나 다른 컨텍스트에서 재사용하기 어렵습니다. 레이아웃 책임을 호출하는 쪽(페이지/스텝 컴포넌트)으로 분리하는 것을 검토해 주세요.♻️ 레이아웃 분리 제안
- <div className="w-full min-h-screen bg-white flex items-center justify-center"> - <div className="w-full max-w-130 px-6 pb-12"> + <div className="w-full max-w-130 px-6 pb-12">호출하는 쪽(
PasswordSetupStep,NewPasswordStep)에서 페이지 레이아웃을 감싸주면 됩니다:// 예: PasswordSetupStep.tsx <div className="w-full min-h-screen bg-white flex items-center justify-center"> <PasswordForm title={...} buttonText={...} onSubmit={...} /> </div>
46-48:<form>에noValidate속성을 추가하면 브라우저 기본 유효성 검사와의 충돌을 방지할 수 있어요.현재
react-hook-form+zod로 커스텀 유효성 검사를 하고 있는데, 브라우저 기본 유효성 검사 팝업이 동시에 뜨면 UX가 어색해질 수 있습니다.noValidate를 추가해서 커스텀 검증만 사용하도록 하면 좋겠어요.♻️ noValidate 추가
<form onSubmit={handleSubmit(handleFormSubmit)} className="flex flex-col gap-7" + noValidate >
Seojegyeong
changed the title
jjjsun
left a comment
jjjsun
left a comment
There was a problem hiding this comment.
P4: 잘 진행해주셔서 코드리뷰는 간단한 부분에 대해서 달아놓았습니다! 고생하셨어요:)
| fullWidth | ||
| onClick={handleSubmit} | ||
| variant="gradient" | ||
| disabled={!isValid || isPending || isExpired} |
There was a problem hiding this comment.
P3: 여기에 type=button 넣어주면 좋을것같아요!
| .string() | ||
| .min(1, "비밀번호는 필수입니다.") | ||
| .regex(PasswordPattern, "영문, 숫자, 특수문자가 모두 들어간 8 -16글자"); | ||
| .regex(passwordPattern, "영문, 숫자, 특수문자가 모두 들어간 8 -16글자"); |
There was a problem hiding this comment.
P3: 사소한건데,,, 16 앞에 띄어쓰기 하나 있으면 더 잘보일것같아요!
2 participants
#35
🚨 관련 이슈
✨ 변경사항
✏️ 작업 내용
공용 컴포넌트 추출
common/button/Button.tsx)common/input/Input.tsx)common/modal/Modal.tsx)auth/common/InputActions.tsx)auth/common/PasswordForm.tsx)hooks/common/useStepNavigation.ts)폴더 구조 및 네이밍 변경
flows/signup/flows/find-email/flows/reset-password/auth/intro/로 이동폴더구조
src/ ├── components/ │ ├── auth/ │ │ ├── common/ │ │ │ ├── CommonAuthInput.tsx │ │ │ ├── InputActions.tsx │ │ │ └── PasswordForm.tsx │ │ ├── flows/ │ │ │ ├── find-email/ │ │ │ │ ├── EnterPhoneStep.tsx │ │ │ │ └── ShowEmailResultStep.tsx │ │ │ ├── reset-password/ │ │ │ │ ├── EmailVerificationStep.tsx │ │ │ │ └── NewPasswordStep.tsx │ │ │ └── signup/ │ │ │ ├── EnterEmailStep.tsx │ │ │ ├── PasswordSetupStep.tsx │ │ │ └── ProfileSetupStep.tsx │ │ ├── intro/ │ │ │ ├── IntroAIAnalytics.tsx │ │ │ ├── IntroAdManagement.tsx │ │ │ ├── IntroLogo.tsx │ │ │ └── OnboardingIntro.tsx │ │ └── skeleton/ │ │ ├── LoginPageSkeleton.tsx │ │ ├── SignupEmailStepSkeleton.tsx │ │ └── SignupPageSkeleton.tsx │ ├── common/ │ │ ├── button/ │ │ │ └── Button.tsx │ │ ├── input/ │ │ │ └── Input.tsx │ │ └── modal/ │ │ └── Modal.tsx │ └── modal/ │ └── ModalProvider.tsx │ ├── hooks/ │ └── common/ │ └── useStepNavigation.ts │ ├── store/ │ └── useModalStore.ts │ └── utils/ ├── formatPhoneNumber.ts └── validation.ts😅 미완성 작업
N/A
📢 논의 사항 및 참고 사항
N/A
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선 사항