🚚 Merge #140
🚚 Merge #140
Conversation
* feat: 검색 부분 미비한 로직 추가 * fix: 이미지 수정 * Feat/#79 (#96) * fix: 히어로 이미지 수정 * feat: 프리미엄 후기 구현 * feat: 세부페이지 구현 * fix: 에러해결 --------- Co-authored-by: yura <[email protected]> * feat: 좋아요 API 명세 추가 --------- Co-authored-by: oaoong <[email protected]> Co-authored-by: yura <[email protected]>
* style: 디자인토큰, 타이포그래피 수정 * fix: onsuccess 콜백에 mutaiton 인자 추가 --------- Co-authored-by: yura <[email protected]>
* style: 디자인토큰, 타이포그래피 수정 * fix: onsuccess 콜백에 mutaiton 인자 추가 * fix: mutation파일에 onmutationresult 추가 --------- Co-authored-by: yura <[email protected]>
style: 배경 기본 색상 변경
Co-authored-by: yura <[email protected]>
* style: 탐색하기 사이드바, 히어로 이미지 수정 * style: 탐색하기 레이아웃 수정 --------- Co-authored-by: yura <[email protected]>
* feat: 리뷰 작성 연결 페이지 재정의 * feat: 리뷰 UI 개발사항 반영 * fix: formatting * feat: QnA 컴포넌트 추가
* refactor: card 컴포넌트 수정 * feat: 북마크 추가 * refactor: card 컴포넌트 수정 * feat: 모바일 카드 구현
* feat: drawer 구현 * feat: 탐색 반응형 구현
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR refactors the review form creation flow from a type-discriminated structure to a step-based approach, consolidates routing from dual kind/type parameters to single kind, and redesigns card and exploration UI components with responsive variants, subscription handling, and new drawer-based filters. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Areas requiring extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ 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 |
Summary of ChangesHello @yura0302, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant overhaul to the review creation workflow, transforming it into a more structured, multi-step process for various review types (paper, interview, activity). It also enhances the user experience on mobile devices by implementing new filtering and sorting components for the club exploration page, alongside updated club cards that now support bookmarking. Underlying technical improvements include critical dependency upgrades and the addition of a Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
전반적으로 대규모 리팩토링이 이루어진 PR로 보입니다. 특히 후기 작성 플로우를 여러 단계의 폼으로 개선하고, 탐색 페이지의 반응형 UI를 개선한 점이 인상적입니다. 새로운 공용 컴포넌트(Drawer, DynamicQASection 등)를 도입하여 코드 재사용성을 높인 것도 좋은 방향입니다. 다만, 몇 가지 중요한 수정이 필요해 보입니다. 탐색 페이지에서 데이터 로딩 및 에러 상태 처리가 누락되어 사용자 경험에 문제가 발생할 수 있습니다. 또한, 카드 컴포넌트의 스타일링에 버그가 있고, 일부 컴포넌트에서 라이브러리의 비공개 API를 사용하는 등 유지보수성을 저해할 수 있는 부분이 확인되었습니다. 자세한 내용은 각 파일에 남긴 리뷰 코멘트를 참고해주세요.
| const { data: clubsData } = useExploreClubs(queryParams) | ||
|
|
||
| const { data: subscribesData } = useUserSubscribes() |
There was a problem hiding this comment.
useExploreClubs와 useUserSubscribes 훅을 사용할 때 isLoading과 error 상태를 처리하지 않고 있습니다. 데이터 로딩이 지연되거나 API 호출에 실패할 경우, 사용자에게 로딩 상태나 오류 메시지가 표시되지 않아 빈 화면이 보이거나 앱이 멈춘 것처럼 보일 수 있습니다. 이전 코드에서는 이 상태들을 처리하고 있었는데, 리팩토링 과정에서 누락된 것으로 보입니다. 사용자 경험에 치명적인 영향을 줄 수 있으므로 로딩 및 오류 상태에 대한 UI 처리를 다시 추가하는 것이 중요합니다.
// 예시
const { data: clubsData, isLoading, error } = useExploreClubs(queryParams);
if (isLoading) {
return <LoadingSpinner />;
}
if (error) {
return <ErrorMessage message="데이터를 불러오는 데 실패했습니다." />;
}
// ... 데이터 렌더링| className={cn( | ||
| base, | ||
| 'gap-[var(--card-gap)] p-[var(--card-pad)] rounded-[12px]', | ||
| 'gap-(--card-gap) p-(--card-pad) rounded-[12px]', |
There was a problem hiding this comment.
Tailwind CSS의 임의 값(arbitrary value) 구문에 오타가 있습니다. gap-(--card-gap) p-(--card-pad)는 유효하지 않은 클래스명이며, 이로 인해 gap과 padding이 적용되지 않습니다. var() 함수를 사용하려면 대괄호([])로 감싸야 합니다. 이로 인해 카드의 레이아웃이 의도와 다르게 표시될 수 있으므로 수정이 필요합니다.
'gap-[var(--card-gap)] p-[var(--card-pad)] rounded-[12px]',
| const questionValue = control._formValues[name]?.[index]?.question || '' | ||
| const answerValue = control._formValues[name]?.[index]?.answer || '' |
There was a problem hiding this comment.
react-hook-form의 내부 속성인 control._formValues에 직접 접근하고 있습니다. 밑줄(_)로 시작하는 속성은 공개 API가 아니므로, 라이브러리 업데이트 시 예고 없이 변경되거나 제거될 수 있어 잠재적인 버그를 유발할 수 있습니다.
더 안정적인 방법은 useWatch를 사용하는 것입니다. 배열의 각 항목을 별도의 컴포넌트로 분리하고 그 안에서 useWatch를 사용하면 성능 저하 없이 안전하게 값의 변화를 감지할 수 있습니다.
| value={field.value?.toString() || ''} | ||
| > | ||
| <SelectTrigger> | ||
| <SelectValue placeholder="-" /> |
There was a problem hiding this comment.
SelectValue 컴포넌트의 placeholder가 "-"로 설정되어 있습니다. 사용자에게 어떤 동작을 해야 하는지 명확하게 알려주지 못해 UX를 저해할 수 있습니다. "선택하기"와 같이 좀 더 설명적인 텍스트로 변경하는 것을 권장합니다. 이 파일의 다른 Select 컴포넌트들에도 동일하게 적용하면 좋겠습니다.
| <SelectValue placeholder="-" /> | |
| <SelectValue placeholder="선택하기" /> |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/molecules/multiDropDown/MultiDropDown.tsx (1)
169-175: Remove empty setTimeout callback - dead code.The
setTimeout(() => {}, 100)on line 173 does nothing and appears to be leftover debug or placeholder code.const handleOpenChange = React.useCallback((newOpen: boolean) => { setOpen(newOpen) - - if (!newOpen) { - setTimeout(() => {}, 100) - } }, [])
🧹 Nitpick comments (35)
src/components/atoms/sideBar/Sidebar.tsx (1)
47-47: Remove the trailing space in the class string.The design token
bg-light-color-2is properly defined insrc/styles/design-tokens.cssand is already used throughout the codebase. However, line 47 contains an unnecessary trailing space after the class name.- 'peer-data-[state=checked]:bg-light-color-2 ', + 'peer-data-[state=checked]:bg-light-color-2',src/shared/constants/filters.ts (1)
1-54: Centralized filter options look good; just watch module boundaries and Tab typing.This shared constants file is a nice improvement over duplicating option arrays. Two follow‑ups to consider:
src/shared/constantsimportingGroup/TabOptionfrom components slightly inverts the usualshared → componentsdependency direction. Long‑term, it may be cleaner to move these types intoshared(e.g. a shared types module) and have components import from there instead of the other way around.- Ensure the
Tabtype behindTabOption['value']actually includes'마감순' | '이름순' | '인기순'; otherwise you’ll hit TS errors or have to widen the type.Not urgent, but worth aligning when you next touch these types.
src/styles/typography.css (1)
31-41: Duplicatetypo-body-3-2-mutility; consider keeping only one definition.
@utility typo-body-3-2-mis declared once in the main body section and again under/* 이전버전 */with the same font-size/line-height/font-weight. It will work, but dropping one definition (likely the one in the legacy block) will avoid confusion for future readers.-@utility typo-body-3-2-m { - font-size: var(--text-body-3); - line-height: 1.5; - font-weight: 500; -}Also applies to: 145-178
src/components/atoms/Button/button.tsx (1)
2-2:asChild+ Radix Slot integration is solid; note minor typing/disabled nuances.The
Comp = asChild ? Slot : 'button'pattern and suppressingtypewhenasChildis true look good and should avoid invalid props on non‑button children.Two small things to be aware of:
Buttonis still typed asforwardRef<HTMLButtonElement, ButtonProps>, so whenasChildis true, refs are typed asHTMLButtonElementeven if the underlying element is, say, a link. This is the common shadcn pattern, but if you ever need stricter typing you may want to switch to a generic/ref helper pattern.- When
asChildis true,disabledis still forwarded. Depending on how Tailwind’sdisabled:variant is configured, non‑button children (e.g. links) might not pick up disabled styles unless you also set something likearia-disabled/data-disabledand adjust the CSS.Nothing blocking, just details to keep in mind as you start using
asChildmore broadly.Also applies to: 62-86
src/components/pages/review/new/forms/activity/ActivityStep1.tsx (1)
26-39: Q3 multi-select handling is correct; consider triggering validation on changeThe Q3 selection logic (toggle + max 4) and rendering via
watch('q3Satisfaction')are sound. To keepformState.errors.q3Satisfactionin sync as users click options (especially after a failed “다음” attempt), you might pass validation flags intosetValue, e.g.:form.setValue('q3Satisfaction', next, { shouldValidate: true, shouldDirty: true, })This keeps error messages and dirty state accurate without waiting for an explicit
trigger.Also applies to: 158-183
src/components/pages/review/new/forms/activity/useActivityForm.ts (1)
133-195: API mapping looks consistent; confirm activityresultTypesemanticsThe transformation of Q1–Q3, 한줄평, dynamic QA items, tip/freeReview into
AnswerRequest[]and the rest ofBasicReviewCreateRequestlooks consistent with the schema. For activity reviews you always sendresultType: ResultType.Pass(comment notes “활동 후기는 resultType이 없으므로 Pass로 설정”). If the backend truly ignoresresultTypeforReviewCategory.Activity, that’s fine; otherwise you may want to omit it or make this convention explicit in the shared type/docs so “Pass” isn’t misinterpreted as an actual outcome.src/components/pages/review/new/forms/interview/useInterviewForm.ts (1)
18-25: TightenresultTypetyping and confirm NOT_PARTICIPATED → Fail mappingThe overall form wiring and payload transformation look good. Two small points:
resultTypeis validated asz.string().min(1, ...), but the UI only ever supplies{ ResultType.Pass, ResultType.Fail, 'NOT_PARTICIPATED', ResultType.Ready }. Using a union (e.g.z.enum([...])orz.union([z.nativeEnum(ResultType), z.literal('NOT_PARTICIPATED')])) would catch unexpected values earlier.- In
transformToApiRequest,'NOT_PARTICIPATED'is normalized toResultType.Fail. Please confirm this matches the backend and product semantics for outcome stats/filters.Also, since you reserve
questionId: 100 + indexfor dynamic QA items, it may be worth documenting that this ID range is synthetic/reserved so it doesn’t clash with server-defined question IDs later.Also applies to: 68-88, 144-220
src/components/pages/review/new/forms/FormFactory.tsx (1)
14-37: Kind-based FormFactory looks good; a couple of small type-guard cleanups are possibleThe switch-based
FormFactoryand the per-kind title/description helpers read cleanly. If you want to simplify further:
isValidFormKindcan avoid the cast by treating the array asreadonlystrings and using it both for the type and runtime check:const formKinds = ['paper', 'interview', 'activity'] as const export type FormKind = (typeof formKinds)[number] export const isValidFormKind = (kind: string): kind is FormKind => formKinds.includes(kind as FormKind)- Since
kindis alreadyFormKind, the|| ''/|| '후기 작성'fallbacks are unreachable; you can safely returndescriptions[kind]/titles[kind]directly if you don’t need the defensive default.Purely optional; current implementation is functionally fine.
Also applies to: 39-59
src/components/pages/review/new/forms/interview/index.tsx (1)
3-3: Unused React import.With modern JSX transform (React 17+), explicit
import React from 'react'is no longer required unless you use React namespace directly.-import React from 'react'src/components/pages/review/new/forms/activity/index.tsx (1)
3-3: Unused React import.Same as in
InterviewForm— modern JSX transform doesn't require explicit React import.-import React from 'react'src/components/molecules/tab/TabOverlay.tsx (1)
8-17: Type naming may cause confusion.Using
Tabas both a component name and a type for the value creates ambiguity. Consider using a more explicit type name likeTabValuefor clarity.-import { Tab, type TabOption } from './Tab' +import { Tab, type TabOption, type TabValue } from './Tab' export type TabOverlayProps = { options?: TabOption[] - value?: Tab - defaultValue?: Tab - onChange?: (value: Tab) => void + value?: TabValue + defaultValue?: TabValue + onChange?: (value: TabValue) => void className?: string // Mobile only trigger?: React.ReactNode onReset?: () => void }src/components/molecules/filterBar/MobileFilterBar.tsx (2)
33-47: Consider type-safe option access.Line 39 uses an inline type assertion
(option: { value: string; label: string })without runtime validation. If the options don't match this shape, this could fail at runtime.Consider adding a type guard or using a more type-safe approach:
- ;(tab.options as Group[]).forEach((group) => { - group.options.forEach((option: { value: string; label: string }) => { + ;(tab.options as Group[]).forEach((group) => { + group.options.forEach((option) => { + if (!option.value || !option.label) return map.set(option.value, option.label) }) })Alternatively, define a proper type for the options structure if not already defined.
49-75: Consider extracting hardcoded locale string.Line 66 contains a hardcoded '전체' string. For consistency and potential i18n support, consider extracting this to a constant.
+const ALL_LABEL = '전체' + const buttonLabels = React.useMemo(() => { const labels: Record<string, string> = {} tabs.forEach((tab) => { // ... } else if (valueArray.includes('all')) { - labels[tab.id] = '전체' + labels[tab.id] = ALL_LABEL } else {src/components/pages/review/new/forms/paper/PaperStep2.tsx (1)
3-3: Unused import.The
Reactimport is not needed in modern React/Next.js with the automatic JSX transform.-import React from 'react'src/components/pages/review/new/forms/activity/ActivityStep2.tsx (2)
3-3: Unused import.The
Reactimport is not needed in modern React/Next.js with the automatic JSX transform.-import React from 'react'
20-109: Consider extracting a shared Step2 component.This component is nearly identical to
PaperStep2.tsx, differing only in form type, labels, and placeholder texts. Consider creating a genericReviewStep2component that accepts configuration props for titles and placeholders to reduce duplication.This is optional and can be deferred if the forms are expected to diverge significantly in the future.
src/app/(root)/(routes)/review/new/[kind]/page.tsx (1)
41-63: Duplicate loading UI can be consolidated.The loading state UI is duplicated between the auth loading check (lines 41-51) and the formKind loading check (lines 53-63).
- if (isLoading || !user) { - return ( - <main className=""> - <div className="max-w-[800px] mx-auto pt-20"> - <div className="text-center"> - <p className="typo-body-2-r text-grey-color-4">로딩 중...</p> - </div> - </div> - </main> - ) - } - - if (!formKind) { + if (isLoading || !user || !formKind) { return ( <main className=""> <div className="max-w-[800px] mx-auto pt-20"> <div className="text-center"> <p className="typo-body-2-r text-grey-color-4">로딩 중...</p> </div> </div> </main> ) }src/components/molecules/card/MobileCard.tsx (1)
37-37: Handle edge case whenfallbackSrcis explicitlynull.If a consumer passes
fallbackSrc={null}explicitly, the non-null assertion will still result innull, causing theImagecomponent to fail. Consider a safer fallback:- const src = !logoUrl ? fallbackSrc! : logoUrl! + const src = logoUrl || fallbackSrc || '/images/default.svg'src/components/pages/review/new/forms/interview/InterviewStep1.tsx (2)
30-40: Trigger validation when updating multi-select value.The
setValuecalls don't trigger form validation, which may leave the field in an invalid state without showing/clearing error messages promptly. Add{ shouldValidate: true }to ensure validation runs after each toggle.if (current.includes(optionId)) { form.setValue( 'q1QuestionType', current.filter((id) => id !== optionId), + { shouldValidate: true }, ) } else { - form.setValue('q1QuestionType', [...current, optionId]) + form.setValue('q1QuestionType', [...current, optionId], { shouldValidate: true }) }
102-127: Consider wrapping Q1 inFormFieldfor consistency.Q1 uses manual watch/setValue handling while Q2-Q4 use
FormField. While this is necessary for multi-select, wrapping Q1 inFormField(even with custom render logic) would provide consistent error handling viaFormMessageinstead of manually accessingform.formState.errors.q1QuestionType.src/components/pages/review/new/shared/ReviewFormHeader.tsx (4)
30-30: Consider pagination or dynamic sizing for clubs list.The hardcoded
size: 100may not fetch all clubs if the list grows beyond 100. Consider implementing pagination, increasing the limit, or fetching all available clubs dynamically.
43-47: Use Next.js Image component for optimized loading.Using the native
<img>tag bypasses Next.js image optimization (lazy loading, format conversion, responsive sizing). Consider using thenext/imagecomponent for better performance.+import Image from 'next/image' ... - <img - src={selectedClub.logoUrl} - alt={selectedClub.clubName} - className="w-16 h-16 rounded-full object-cover flex-shrink-0" - /> + <Image + src={selectedClub.logoUrl} + alt={selectedClub.clubName} + width={64} + height={64} + className="rounded-full object-cover flex-shrink-0" + />
106-112: Consider extracting hardcoded job options to a shared constant.The job options (PM/PO, 프로덕트 디자이너, etc.) with their IDs are hardcoded here. If these values are used elsewhere or fetched from an API, consider extracting them to a shared constant file for consistency and maintainability.
54-85: Type casting withas Path<T>reduces type safety.The use of
'clubId' as Path<T>,'jobId' as Path<T>, and'generation' as Path<T>assumes these field names exist in all form typesT. This could lead to runtime errors if a form type doesn't include these fields. Consider constrainingTto require these fields:-interface ReviewFormHeaderProps<T extends FieldValues> { +interface ReviewFormHeaderFields { + clubId: number + jobId: number + generation: number +} + +interface ReviewFormHeaderProps<T extends ReviewFormHeaderFields> { control: Control<T> selectedClubId?: number }src/components/pages/review/new/forms/paper/PaperStep1.tsx (1)
101-126: Q1 section lacks FormField/FormControl wrapper, breaking consistency.Unlike Q2 and Q3, the Q1 multi-select section doesn't use the
FormField/FormControlpattern. This inconsistency may affect accessibility (missing aria attributes from FormControl) and breaks the uniform form composition pattern.Consider wrapping Q1 with
FormFieldfor consistency:- {/* Q1 */} - <FormItem> - <FormLabel className="typo-body-2-sb text-black-color mb-4"> + {/* Q1 */} + <FormField + control={form.control} + name="q1ImportantAppeal" + render={({ field }) => ( + <FormItem> + <FormLabel className="typo-body-2-sb text-black-color mb-4"> Q1. 지원서 작성에 있어 가장 중요하게 어필한 것은 무엇이었나요? <span className="typo-caption-m text-grey-color-3 ml-2"> (최대 4개) </span> </FormLabel> - <div className="flex flex-col desktop:flex-row gap-3 desktop:flex-wrap"> + <FormControl> + <div className="flex flex-col desktop:flex-row gap-3 desktop:flex-wrap"> {Q1_IMPORTANT_APPEAL_OPTIONS.map((option) => ( <OptionButton key={option.id} - selected={watchedQ1.includes(option.id)} + selected={field.value?.includes(option.id)} onClick={() => handleQ1Change(option.id)} className="w-full desktop:w-auto" > {option.label} </OptionButton> ))} - </div> - {form.formState.errors.q1ImportantAppeal && ( - <p className="text-failure-color typo-caption-m mt-1"> - {form.formState.errors.q1ImportantAppeal.message} - </p> - )} - </FormItem> + </div> + </FormControl> + <FormMessage /> + </FormItem> + )} + />src/components/molecules/tab/MobileTab.tsx (3)
46-48: Hardcoded default value could be fragile.The default value
'마감순' as Tabis hardcoded twice (lines 47 and 68). If theTabtype changes or this value becomes invalid, it could cause issues. Consider using the first option fromoptionsas the default or accepting a requireddefaultValueprop.
135-135: Memoize reversed options array.
[...options].reverse()creates a new array on every render. Consider memoizing this computation.+ const reversedOptions = React.useMemo( + () => [...options].reverse(), + [options], + ) + ... - {[...options].reverse().map((option) => { + {reversedOptions.map((option) => {
89-96: handleOptionChange ignores unchecked state.The
handleOptionChangecallback only processes whencheckedistrue. While this may be intentional for radio-button-like behavior, consider adding a comment to clarify this is by design.const handleOptionChange = React.useCallback( (optionValue: Tab, checked: boolean) => { + // Only process checked state - unchecking is handled by selecting another option if (checked) { setTempSelected(optionValue) } }, [], )src/components/(pages)/club/explore/Explore.tsx (3)
36-68: Consider extracting duplicate array parsing logic.The
partArray,wayArray, andtargetArrayuseMemo hooks have identical logic. Consider extracting to a shared helper function to reduce duplication.+const parseArrayParam = (value: string | null | undefined): string[] => { + if (value === 'all') return ['all'] + if (value === null || value === undefined) return [] + return value ? value.split(',').filter(Boolean) : [] +} + -const partArray = React.useMemo(() => { - if (part === 'all') { - // "전체" 선택 시 ['all'] 반환 - return ['all'] - } - if (part === null || part === undefined) { - // 초기 상태 또는 선택 없음 시 빈 배열 반환 - return [] - } - return part ? part.split(',').filter(Boolean) : [] -}, [part]) +const partArray = React.useMemo(() => parseArrayParam(part), [part]) +const wayArray = React.useMemo(() => parseArrayParam(way), [way]) +const targetArray = React.useMemo(() => parseArrayParam(target), [target])
110-112: Remove or implement identity function.
mapCategoryreturns its input unchanged. Either implement the intended mapping or remove this function and usecurrentFielddirectly inqueryParams.-const mapCategory = (category: string): string => { - return category -} const queryParams = { page: 0, size: 14, - field: currentField !== 'all' ? mapCategory(currentField) : undefined, + field: currentField !== 'all' ? currentField : undefined,
129-139: MemoizesubscribedClubIdsto avoid creating a new Set on every render.The
Setis recreated on every render even whensubscribeshasn't changed. This affects referential equality checks downstream and incurs unnecessary allocations.const clubs = clubsData?.content || [] const subscribes = subscribesData?.data?.content || [] -const subscribedClubIds = new Set(subscribes.map((s) => s.clubId)) +const subscribedClubIds = React.useMemo( + () => new Set(subscribes.map((s) => s.clubId)), + [subscribes], +)src/components/pages/review/new/forms/paper/usePaperForm.ts (1)
141-171: Extract magic question IDs to named constants.Hard-coded question IDs (1, 2, 3, 19, 20, 21, 100+) scattered throughout the transformation logic reduce maintainability. Extract these to named constants for clarity and to prevent ID collision issues.
+// Question IDs for Paper form +const QUESTION_IDS = { + IMPORTANT_APPEAL: 1, + REFERENCE_INFO: 2, + TECH_DESCRIPTION: 3, + ONE_LINE_COMMENT: 19, + TIP: 20, + FREE_REVIEW: 21, + DYNAMIC_QA_BASE: 100, +} as const + const transformToApiRequest = ( data: PaperFormType, ): BasicReviewCreateRequest => { const questions: AnswerRequest[] = [ { - questionId: 1, // Q1: 중요 어필 + questionId: QUESTION_IDS.IMPORTANT_APPEAL, questionType: QuestionType.MultipleChoice, value: data.q1ImportantAppeal, }, // ... apply similar changes to other question IDssrc/components/molecules/multiDropDown/MobileMultiDropdown.tsx (1)
225-229: Avoid hardcoding tab ID check.The check
tab.id === 'part'couples this reusable component to a specific use case. Consider adding ashowGroupTitleproperty to theGrouptype orMobileMultiDropdownTabinstead.export type MobileMultiDropdownTab = { id: string label: string type: 'sort' | 'multi' options: TabOption[] | Group[] value: string | string[] defaultValue?: string | string[] onChange: (value: string | string[]) => void onReset?: () => void + showGroupTitles?: boolean } // Then in render: -{group.title && tab.id === 'part' && ( +{group.title && tab.showGroupTitles && (src/components/molecules/drawer/drawer.tsx (1)
67-67: Minor: Extra whitespace in className.There's a double space in the className that should be cleaned up.
-<div className="bg-muted mx-auto hidden h-2 w-[100px] shrink-0 rounded-full group-data-[vaul-drawer-direction=bottom]/drawer-content:block" /> +<div className="bg-muted mx-auto hidden h-2 w-[100px] shrink-0 rounded-full group-data-[vaul-drawer-direction=bottom]/drawer-content:block" />src/components/molecules/card/Card.tsx (1)
170-170: Hardcoded aspect ratio class may conflict with inline style.
aspectClassonly handles two specific ratios, but the inlineaspectRatiostyle (Line 191) handles any ratio. Consider removing the hardcoded class since the inline style already provides the correct ratio, or use Tailwind's arbitrary value syntax.-const aspectClass = ratio === '113/108' ? 'aspect-[113/108]' : 'aspect-[3/2]' +// Let inline style handle aspect ratio, or use arbitrary value: +const aspectStyle = ratio ? { aspectRatio: ratio.replace('/', ' / ') } : undefined
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
package-lock.jsonis excluded by!**/package-lock.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yamlpublic/images/heroAll.svgis excluded by!**/*.svgsrc/assets/icons/bookmark-empty-icon.svgis excluded by!**/*.svgsrc/assets/icons/bookmark-filled-icon.svgis excluded by!**/*.svgsrc/assets/icons/bookmark-mobile-empty.svgis excluded by!**/*.svgsrc/assets/icons/bookmark-mobile-filled.svgis excluded by!**/*.svgsrc/assets/icons/document-file-icon.svgis excluded by!**/*.svg
📒 Files selected for processing (82)
.example.env(0 hunks)CLAUDE.md(1 hunks)package.json(3 hunks)public/mockServiceWorker.js(5 hunks)src/app/(root)/(routes)/review/new/[kind]/[type]/page.tsx(0 hunks)src/app/(root)/(routes)/review/new/[kind]/page.tsx(1 hunks)src/app/(root)/(routes)/review/new/page.tsx(1 hunks)src/assets/icons/index.ts(1 hunks)src/components/(pages)/club/explore/Explore.tsx(2 hunks)src/components/atoms/Button/button.tsx(2 hunks)src/components/atoms/Textarea/Textarea.tsx(1 hunks)src/components/atoms/UnderLineTab/UnderLineTab.tsx(1 hunks)src/components/atoms/sideBar/Sidebar.tsx(1 hunks)src/components/molecules/card/Card.stories.tsx(2 hunks)src/components/molecules/card/Card.tsx(4 hunks)src/components/molecules/card/CardContent.tsx(0 hunks)src/components/molecules/card/CardDescription.tsx(0 hunks)src/components/molecules/card/CardFooter.tsx(0 hunks)src/components/molecules/card/CardHeader.tsx(0 hunks)src/components/molecules/card/CardImage.tsx(0 hunks)src/components/molecules/card/CardMeta.tsx(0 hunks)src/components/molecules/card/CardOverlay.tsx(1 hunks)src/components/molecules/card/CardStats.tsx(0 hunks)src/components/molecules/card/CardTitle.tsx(0 hunks)src/components/molecules/card/MobileCard.tsx(1 hunks)src/components/molecules/card/index.ts(4 hunks)src/components/molecules/card/presets.ts(2 hunks)src/components/molecules/drawer/drawer.tsx(1 hunks)src/components/molecules/drawer/index.ts(1 hunks)src/components/molecules/dynamicQaSection/DynamicQASection.stories.tsx(1 hunks)src/components/molecules/dynamicQaSection/DynamicQASection.tsx(1 hunks)src/components/molecules/dynamicQaSection/index.ts(1 hunks)src/components/molecules/filterBar/MobileFilterBar.tsx(1 hunks)src/components/molecules/layout/Header.tsx(1 hunks)src/components/molecules/layout/MobileHeader.tsx(1 hunks)src/components/molecules/layout/sections/ReviewWriteDropdown.tsx(1 hunks)src/components/molecules/multiDropDown/MobileMultiDropdown.tsx(1 hunks)src/components/molecules/multiDropDown/MultiDropDown.tsx(4 hunks)src/components/molecules/tab/MobileTab.tsx(1 hunks)src/components/molecules/tab/Tab.tsx(1 hunks)src/components/molecules/tab/TabOverlay.tsx(1 hunks)src/components/pages/review/new/ReviewCardTemplate.tsx(0 hunks)src/components/pages/review/new/forms/ActivityNormalForm.tsx(0 hunks)src/components/pages/review/new/forms/ActivityPremiumForm.tsx(0 hunks)src/components/pages/review/new/forms/FormFactory.tsx(1 hunks)src/components/pages/review/new/forms/InterviewNormalForm.tsx(0 hunks)src/components/pages/review/new/forms/InterviewPremiumForm.tsx(0 hunks)src/components/pages/review/new/forms/PaperNormalForm.tsx(0 hunks)src/components/pages/review/new/forms/PaperPremiumForm.tsx(0 hunks)src/components/pages/review/new/forms/activity/ActivityStep1.tsx(1 hunks)src/components/pages/review/new/forms/activity/ActivityStep2.tsx(1 hunks)src/components/pages/review/new/forms/activity/index.tsx(1 hunks)src/components/pages/review/new/forms/activity/useActivityForm.ts(1 hunks)src/components/pages/review/new/forms/hooks/index.ts(0 hunks)src/components/pages/review/new/forms/hooks/useActivityNormalForm.ts(0 hunks)src/components/pages/review/new/forms/hooks/useActivityPremiumForm.ts(0 hunks)src/components/pages/review/new/forms/hooks/useInterviewNormalForm.ts(0 hunks)src/components/pages/review/new/forms/hooks/useInterviewPremiumForm.ts(0 hunks)src/components/pages/review/new/forms/hooks/usePaperNormalForm.ts(0 hunks)src/components/pages/review/new/forms/hooks/usePaperPremiumForm.ts(0 hunks)src/components/pages/review/new/forms/index.ts(1 hunks)src/components/pages/review/new/forms/interview/InterviewStep1.tsx(1 hunks)src/components/pages/review/new/forms/interview/InterviewStep2.tsx(1 hunks)src/components/pages/review/new/forms/interview/index.tsx(1 hunks)src/components/pages/review/new/forms/interview/useInterviewForm.ts(1 hunks)src/components/pages/review/new/forms/paper/PaperStep1.tsx(1 hunks)src/components/pages/review/new/forms/paper/PaperStep2.tsx(1 hunks)src/components/pages/review/new/forms/paper/index.tsx(1 hunks)src/components/pages/review/new/forms/paper/usePaperForm.ts(1 hunks)src/components/pages/review/new/shared/ReviewFormHeader.tsx(1 hunks)src/components/pages/review/new/shared/StepNavigation.tsx(1 hunks)src/components/pages/review/new/shared/index.ts(1 hunks)src/features/clubs/mutations.ts(3 hunks)src/features/like/mutations.ts(2 hunks)src/features/review/mutations.ts(2 hunks)src/features/subscribe/mutations.ts(1 hunks)src/shared/configs/appPath.ts(1 hunks)src/shared/constants/category.ts(1 hunks)src/shared/constants/filters.ts(1 hunks)src/styles/design-tokens.css(1 hunks)src/styles/globals.css(1 hunks)src/styles/typography.css(1 hunks)
💤 Files with no reviewable changes (24)
- src/components/pages/review/new/ReviewCardTemplate.tsx
- src/components/molecules/card/CardHeader.tsx
- src/components/molecules/card/CardDescription.tsx
- src/components/molecules/card/CardMeta.tsx
- src/components/pages/review/new/forms/hooks/usePaperPremiumForm.ts
- src/components/molecules/card/CardTitle.tsx
- src/components/pages/review/new/forms/PaperPremiumForm.tsx
- src/components/pages/review/new/forms/InterviewPremiumForm.tsx
- src/app/(root)/(routes)/review/new/[kind]/[type]/page.tsx
- src/components/pages/review/new/forms/PaperNormalForm.tsx
- .example.env
- src/components/pages/review/new/forms/ActivityNormalForm.tsx
- src/components/molecules/card/CardContent.tsx
- src/components/pages/review/new/forms/hooks/useActivityPremiumForm.ts
- src/components/pages/review/new/forms/hooks/useActivityNormalForm.ts
- src/components/molecules/card/CardStats.tsx
- src/components/pages/review/new/forms/hooks/index.ts
- src/components/pages/review/new/forms/hooks/useInterviewPremiumForm.ts
- src/components/molecules/card/CardFooter.tsx
- src/components/pages/review/new/forms/hooks/usePaperNormalForm.ts
- src/components/pages/review/new/forms/hooks/useInterviewNormalForm.ts
- src/components/pages/review/new/forms/InterviewNormalForm.tsx
- src/components/pages/review/new/forms/ActivityPremiumForm.tsx
- src/components/molecules/card/CardImage.tsx
🧰 Additional context used
🧬 Code graph analysis (28)
src/components/pages/review/new/forms/paper/index.tsx (5)
src/components/pages/review/new/forms/paper/usePaperForm.ts (1)
usePaperForm(87-230)src/components/molecules/Form/Form.tsx (1)
Form(160-160)src/components/pages/review/new/forms/paper/PaperStep1.tsx (1)
PaperStep1(26-187)src/components/pages/review/new/forms/paper/PaperStep2.tsx (1)
PaperStep2(20-111)src/components/pages/review/new/shared/StepNavigation.tsx (1)
StepNavigation(14-44)
src/components/molecules/filterBar/MobileFilterBar.tsx (3)
src/components/molecules/multiDropDown/MobileMultiDropdown.tsx (2)
MobileMultiDropdownTab(16-25)MobileMultiDropdown(36-298)src/components/molecules/multiDropDown/MultiDropDown.tsx (1)
Group(16-16)src/components/molecules/tab/Tab.tsx (1)
TabOption(16-16)
src/components/pages/review/new/forms/interview/InterviewStep1.tsx (1)
src/components/pages/review/new/forms/interview/useInterviewForm.ts (6)
InterviewFormType(90-90)INTERVIEW_RESULT_OPTIONS(19-24)Q1_QUESTION_TYPE_OPTIONS(27-36)Q2_INTERVIEWER_ATTITUDE_OPTIONS(39-44)Q3_MAIN_TOPIC_OPTIONS(47-52)Q4_EMPHASIZED_SKILL_OPTIONS(55-60)
src/app/(root)/(routes)/review/new/page.tsx (1)
src/app/(root)/(routes)/review/new/[kind]/page.tsx (1)
Page(17-74)
src/components/molecules/card/MobileCard.tsx (1)
src/components/molecules/card/index.ts (1)
MobileCard(50-50)
src/components/molecules/dynamicQaSection/DynamicQASection.tsx (2)
src/components/molecules/Form/Form.tsx (4)
FormField(166-166)FormItem(161-161)FormControl(163-163)FormMessage(165-165)src/components/atoms/Textarea/Textarea.tsx (1)
Textarea(23-23)
src/components/pages/review/new/forms/interview/index.tsx (5)
src/components/pages/review/new/forms/interview/useInterviewForm.ts (1)
useInterviewForm(92-240)src/components/molecules/Form/Form.tsx (1)
Form(160-160)src/components/pages/review/new/shared/ReviewFormHeader.tsx (1)
ReviewFormHeader(26-152)src/components/pages/review/new/forms/interview/InterviewStep1.tsx (1)
InterviewStep1(27-216)src/components/pages/review/new/forms/interview/InterviewStep2.tsx (1)
InterviewStep2(20-111)
src/components/pages/review/new/forms/activity/useActivityForm.ts (3)
src/shared/configs/appValidation.ts (1)
appValidation(3-24)src/features/review/mutations.ts (1)
usePostBasicReview(16-32)src/features/review/types.ts (2)
BasicReviewCreateRequest(158-191)AnswerRequest(28-41)
src/shared/constants/category.ts (1)
src/components/atoms/sideBar/Sidebar.tsx (1)
SideOption(7-7)
src/app/(root)/(routes)/review/new/[kind]/page.tsx (4)
src/app/(root)/(routes)/review/new/page.tsx (1)
Page(22-96)src/shared/providers/auth-provider.tsx (1)
useAuth(138-144)src/components/pages/review/new/forms/FormFactory.tsx (3)
FormKind(8-8)isValidFormKind(34-37)FormFactory(14-31)src/components/pages/review/new/forms/index.ts (2)
FormKind(3-3)isValidFormKind(5-5)
src/components/pages/review/new/forms/activity/ActivityStep2.tsx (4)
src/components/pages/review/new/forms/activity/useActivityForm.ts (1)
ActivityFormType(81-81)src/components/molecules/Form/Form.tsx (5)
FormField(166-166)FormItem(161-161)FormLabel(162-162)FormControl(163-163)FormMessage(165-165)src/components/atoms/Textarea/Textarea.tsx (1)
Textarea(23-23)src/components/molecules/dynamicQaSection/DynamicQASection.tsx (1)
DynamicQASection(28-155)
src/components/pages/review/new/forms/paper/usePaperForm.ts (3)
src/shared/configs/appValidation.ts (1)
appValidation(3-24)src/features/review/mutations.ts (1)
usePostBasicReview(16-32)src/features/review/types.ts (2)
BasicReviewCreateRequest(158-191)AnswerRequest(28-41)
src/components/molecules/multiDropDown/MobileMultiDropdown.tsx (4)
src/components/molecules/tab/Tab.tsx (1)
TabOption(16-16)src/components/molecules/multiDropDown/MultiDropDown.tsx (1)
Group(16-16)src/components/molecules/drawer/drawer.tsx (4)
Drawer(124-124)DrawerContent(129-129)DrawerHeader(130-130)DrawerFooter(131-131)src/components/atoms/UnderLineTab/UnderLineTab.tsx (1)
UnderLineTab(66-145)
src/components/molecules/card/index.ts (1)
src/components/molecules/card/Card.tsx (2)
CardImage(151-209)CardBookmark(298-330)
src/components/molecules/card/CardOverlay.tsx (5)
src/features/explore/types.ts (1)
ClubItem(14-21)src/shared/hooks/useMediaQuery.ts (1)
useMediaQuery(6-23)src/components/molecules/card/Card.tsx (1)
Card(41-85)src/components/molecules/card/index.ts (2)
Card(26-36)MobileCard(50-50)src/components/molecules/card/MobileCard.tsx (1)
MobileCard(24-127)
src/shared/constants/filters.ts (2)
src/components/molecules/tab/Tab.tsx (1)
TabOption(16-16)src/components/molecules/multiDropDown/MultiDropDown.tsx (1)
Group(16-16)
src/features/clubs/mutations.ts (1)
src/features/clubs/keys.ts (1)
clubKeys(1-26)
src/components/pages/review/new/forms/paper/PaperStep2.tsx (3)
src/components/pages/review/new/forms/paper/usePaperForm.ts (1)
PaperFormType(85-85)src/components/molecules/Form/Form.tsx (5)
FormField(166-166)FormItem(161-161)FormLabel(162-162)FormControl(163-163)FormMessage(165-165)src/components/molecules/dynamicQaSection/DynamicQASection.tsx (1)
DynamicQASection(28-155)
src/components/molecules/layout/sections/ReviewWriteDropdown.tsx (1)
src/components/atoms/Button/button.tsx (1)
Button(95-95)
src/components/pages/review/new/forms/interview/InterviewStep2.tsx (4)
src/components/pages/review/new/forms/interview/useInterviewForm.ts (1)
InterviewFormType(90-90)src/components/molecules/Form/Form.tsx (5)
FormField(166-166)FormItem(161-161)FormLabel(162-162)FormControl(163-163)FormMessage(165-165)src/components/atoms/Textarea/Textarea.tsx (1)
Textarea(23-23)src/components/molecules/dynamicQaSection/DynamicQASection.tsx (1)
DynamicQASection(28-155)
src/components/molecules/card/Card.stories.tsx (2)
src/components/molecules/card/Card.tsx (1)
Card(41-85)src/components/molecules/card/index.ts (1)
Card(26-36)
src/features/subscribe/mutations.ts (1)
src/features/subscribe/keys.ts (1)
subscribeKeys(1-6)
src/components/molecules/dynamicQaSection/DynamicQASection.stories.tsx (1)
src/components/molecules/dynamicQaSection/DynamicQASection.tsx (1)
DynamicQASection(28-155)
src/components/pages/review/new/forms/interview/useInterviewForm.ts (3)
src/shared/configs/appValidation.ts (1)
appValidation(3-24)src/features/review/mutations.ts (1)
usePostBasicReview(16-32)src/features/review/types.ts (2)
BasicReviewCreateRequest(158-191)AnswerRequest(28-41)
src/components/molecules/card/Card.tsx (3)
src/components/molecules/card/presets.ts (1)
PRESET(3-36)src/assets/icons/GreyThumbsUp.tsx (1)
GreyThumbsUp(2-18)src/assets/icons/GreyMessage.tsx (1)
GreyMessage(2-11)
src/components/pages/review/new/forms/paper/PaperStep1.tsx (2)
src/components/pages/review/new/forms/paper/usePaperForm.ts (5)
PaperFormType(85-85)PAPER_RESULT_OPTIONS(19-24)Q1_IMPORTANT_APPEAL_OPTIONS(27-36)Q2_REFERENCE_INFO_OPTIONS(39-44)Q3_TECH_DESCRIPTION_OPTIONS(47-52)src/components/molecules/Form/Form.tsx (5)
FormField(166-166)FormItem(161-161)FormLabel(162-162)FormControl(163-163)FormMessage(165-165)
src/components/molecules/multiDropDown/MultiDropDown.tsx (1)
src/shared/utils/index.ts (1)
cn(2-2)
src/features/review/mutations.ts (1)
src/features/review/keys.ts (1)
reviewKeys(3-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test-and-build
| useEffect(() => { | ||
| async function loadParams() { | ||
| const { kind } = await params | ||
|
|
||
| const kindName = getKindDisplayName(kind) | ||
| if (!isValidFormKind(kind)) { | ||
| notFound() | ||
| } | ||
|
|
||
| return { | ||
| title: `${kindName} 후기 작성`, | ||
| description: `IT 동아리 ${kindName} 후기를 작성해보세요. 일반 후기와 프리미엄 후기 중 선택하여 경험을 공유하고 다른 분들에게 도움을 주세요.`, | ||
| keywords: [`IT 동아리 ${kindName} 후기`, '동아리 후기 작성', '경험 공유'], | ||
| openGraph: { | ||
| title: `${kindName} 후기 작성 | 모여잇`, | ||
| description: `IT 동아리 ${kindName} 후기를 작성해보세요. 일반 후기와 프리미엄 후기 중 선택하여 경험을 공유하고 다른 분들에게 도움을 주세요.`, | ||
| }, | ||
| } | ||
| } | ||
| setFormKind(kind as FormKind) | ||
| } | ||
|
|
||
| export default async function Page({ | ||
| params, | ||
| }: { | ||
| params: Promise<{ kind: string }> | ||
| }) { | ||
| const { kind } = await params | ||
| const isValidKind = (validKinds as readonly string[]).includes(kind) | ||
| if (!isValidKind) { | ||
| notFound() | ||
| } | ||
| loadParams() | ||
| }, [params]) |
There was a problem hiding this comment.
notFound() does not work correctly inside useEffect.
notFound() from next/navigation throws a special error that Next.js catches during server-side rendering or during the render phase. Calling it inside an async function within useEffect will result in an unhandled error instead of displaying the 404 page.
Consider validating the kind during the render phase or using client-side navigation:
+'use client'
+
+import { Suspense, useEffect, useState } from 'react'
+import { notFound, useRouter } from 'next/navigation'
import {
FormFactory,
isValidFormKind,
type FormKind,
} from '@/components/pages/review/new/forms'
import AppPath from '@/shared/configs/appPath'
import { useAuth } from '@/shared/providers/auth-provider'
interface PageProps {
params: Promise<{ kind: string }>
}
export default function Page({ params }: PageProps) {
const { user, isLoading } = useAuth()
+ const router = useRouter()
const [formKind, setFormKind] = useState<FormKind | null>(null)
+ const [isInvalidKind, setIsInvalidKind] = useState(false)
useEffect(() => {
async function loadParams() {
const { kind } = await params
if (!isValidFormKind(kind)) {
- notFound()
+ setIsInvalidKind(true)
+ return
}
setFormKind(kind as FormKind)
}
loadParams()
}, [params])
+
+ if (isInvalidKind) {
+ notFound()
+ }Alternatively, consider converting this to a server component to validate kind before rendering.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/app/(root)/(routes)/review/new/[kind]/page.tsx around lines 21-33,
calling next/navigation's notFound() inside an async useEffect causes an
unhandled thrown error; instead validate params synchronously during render
(before hooks) and call notFound() there, or if you need client-side handling,
replace the useEffect path with a client-side navigation (e.g.,
useRouter().replace('/404') or push a fallback route) so you don't throw inside
an async effect; alternatively move this component to a server component and
perform the kind validation server-side before rendering.
| useEffect(() => { | ||
| if (!isLoading && !user) { | ||
| redirect(AppPath.login()) | ||
| } | ||
| } | ||
| }, [user, isLoading]) |
There was a problem hiding this comment.
redirect() does not work correctly inside useEffect.
Similar to notFound(), redirect() from next/navigation is designed for server components or the render phase. Use useRouter for client-side navigation.
+import { useRouter } from 'next/navigation'
+
export default function Page({ params }: PageProps) {
const { user, isLoading } = useAuth()
+ const router = useRouter()
// ...
useEffect(() => {
if (!isLoading && !user) {
- redirect(AppPath.login())
+ router.replace(AppPath.login())
}
- }, [user, isLoading])
+ }, [user, isLoading, router])Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/app/(root)/(routes)/review/new/[kind]/page.tsx around lines 35 to 39, the
code calls redirect() inside useEffect which is for server/render-phase only;
change to client-side navigation by making this a client component (add "use
client" at the top if not present), import useRouter from next/navigation, get
const router = useRouter(), and replace the redirect(...) call with
router.replace(AppPath.login()) (or router.push if you want history) inside the
useEffect; keep the same dependency array and ensure imports are updated.
| const [failed, setFailed] = React.useState(false) | ||
| const src = failed || !logoUrl ? fallbackSrc! : logoUrl! | ||
|
|
||
| const ratio = ratioOverride || p.ratio | ||
|
|
||
| const aspectClass = ratio === '113/108' ? 'aspect-[113/108]' : 'aspect-[3/2]' |
There was a problem hiding this comment.
Non-null assertion on potentially null fallbackSrc.
fallbackSrc has type string | null, but Line 166 uses fallbackSrc! which assumes it's non-null. If both logoUrl is falsy/failed and fallbackSrc is null, this will pass null to Image.src.
-const src = failed || !logoUrl ? fallbackSrc! : logoUrl!
+const src = failed || !logoUrl ? (fallbackSrc ?? '/images/default.svg') : logoUrlOr update the type to make fallbackSrc non-nullable since there's already a default value in the destructuring.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/molecules/card/Card.tsx around lines 165-170, the code uses
fallbackSrc! which asserts non-null but fallbackSrc is typed string | null;
replace the non-null assertion by either (a) providing a safe default in the
destructuring (e.g. fallbackSrc = '' so the prop becomes non-null at runtime and
update the prop type accordingly), or (b) compute src defensively like const src
= failed || !logoUrl ? (fallbackSrc ?? '') : logoUrl and update downstream Image
usage to handle an empty string (or conditionally render the Image when no src).
Ensure the prop types are adjusted if you choose to make fallbackSrc
non-nullable.
| const questionValue = control._formValues[name]?.[index]?.question || '' | ||
| const answerValue = control._formValues[name]?.[index]?.answer || '' |
There was a problem hiding this comment.
Avoid using internal control._formValues API.
Accessing control._formValues relies on react-hook-form's private implementation details, which can break on library updates. Use useWatch from react-hook-form to reactively get field values instead.
+'use client'
+
+import React, { useState } from 'react'
import {
Control,
useFieldArray,
FieldValues,
ArrayPath,
Path,
+ useWatch,
} from 'react-hook-form'Then within the component, replace the direct access:
export default function DynamicQASection<T extends FieldValues>({
control,
name,
title,
questionPlaceholder,
answerPlaceholder,
}: DynamicQASectionProps<T>) {
const { fields, append } = useFieldArray({
control,
name,
})
+ const watchedValues = useWatch({ control, name })
const [focusedField, setFocusedField] = useState<{
index: number
field: 'question' | 'answer'
} | null>(null)Then in the map callback:
{fields.map((field, index) => {
- const questionValue = control._formValues[name]?.[index]?.question || ''
- const answerValue = control._formValues[name]?.[index]?.answer || ''
+ const questionValue = watchedValues?.[index]?.question || ''
+ const answerValue = watchedValues?.[index]?.answer || ''📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const questionValue = control._formValues[name]?.[index]?.question || '' | |
| const answerValue = control._formValues[name]?.[index]?.answer || '' | |
| 'use client' | |
| import React, { useState } from 'react' | |
| import { | |
| Control, | |
| useFieldArray, | |
| FieldValues, | |
| ArrayPath, | |
| Path, | |
| useWatch, | |
| } from 'react-hook-form' | |
| export default function DynamicQASection<T extends FieldValues>({ | |
| control, | |
| name, | |
| title, | |
| questionPlaceholder, | |
| answerPlaceholder, | |
| }: DynamicQASectionProps<T>) { | |
| const { fields, append } = useFieldArray({ | |
| control, | |
| name, | |
| }) | |
| const watchedValues = useWatch({ control, name }) | |
| const [focusedField, setFocusedField] = useState<{ | |
| index: number | |
| field: 'question' | 'answer' | |
| } | null>(null) | |
| // ... rest of component ... | |
| {fields.map((field, index) => { | |
| const questionValue = watchedValues?.[index]?.question || '' | |
| const answerValue = watchedValues?.[index]?.answer || '' | |
| // ... rest of map callback ... | |
| })} |
🤖 Prompt for AI Agents
In src/components/molecules/dynamicQaSection/DynamicQASection.tsx around lines
56-57, the code reads control._formValues[...] which uses react-hook-form
internal API; replace this with useWatch to subscribe to the fields reactively.
Import useWatch from react-hook-form, call useWatch with the control and name to
get the array of items (or provide a default empty array), then inside the map
read item.question and item.answer (with safe defaults) instead of accessing
control._formValues; ensure the useWatch call is outside the render loop and
handles undefined by falling back to [].
| onFocus={() => | ||
| setFocusedField({ index, field: 'question' }) | ||
| } | ||
| onBlur={() => setFocusedField(null)} |
There was a problem hiding this comment.
Potential conflict between field spread and onBlur.
The {...field} spread includes field.onBlur from react-hook-form. Your custom onBlur handler only calls setFocusedField(null) without invoking the original field.onBlur, which may prevent validation from triggering on blur.
<Textarea
{...field}
placeholder={questionPlaceholder}
className="min-h-6 border-none p-0 resize-none focus-visible:ring-0"
onFocus={() =>
setFocusedField({ index, field: 'question' })
}
- onBlur={() => setFocusedField(null)}
+ onBlur={(e) => {
+ setFocusedField(null)
+ field.onBlur()
+ }}
/>Apply the same pattern to all Textarea onBlur handlers in this component (lines 102, 134).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onFocus={() => | |
| setFocusedField({ index, field: 'question' }) | |
| } | |
| onBlur={() => setFocusedField(null)} | |
| onFocus={() => | |
| setFocusedField({ index, field: 'question' }) | |
| } | |
| onBlur={(e) => { | |
| setFocusedField(null) | |
| field.onBlur() | |
| }} |
🤖 Prompt for AI Agents
In src/components/molecules/dynamicQaSection/DynamicQASection.tsx around lines
99–102 and 134, the Textarea components spread {...field} (which supplies
react-hook-form's field.onBlur) but override onBlur with a handler that only
calls setFocusedField(null); update those onBlur handlers to call the original
field.onBlur (e.g., call field.onBlur(event) or invoke the passed onBlur from
args) before or after setFocusedField(null) so react-hook-form blur validation
still runs; apply the same pattern to any other Textarea onBlur overrides in
this component.
| // <DropdownMenu> | ||
| // <DropdownMenuTrigger asChild> | ||
| // <Button size="small" variant="solid" className="typo-button"> | ||
| // 후기 작성 | ||
| // </Button> | ||
| // </DropdownMenuTrigger> | ||
| // <DropdownMenuContent className="w-40 mt-4 mr-10"> | ||
| // <DropdownMenuItem className="typo-body-3-3-r text-grey-color-5"> | ||
| // <Link href={AppPath.reviewNew('paper')} className="w-full px-4 py-2"> | ||
| // 서류 후기 | ||
| // </Link> | ||
| // </DropdownMenuItem> | ||
| // <DropdownMenuItem className="typo-body-3-3-r text-grey-color-5"> | ||
| // <Link | ||
| // href={AppPath.reviewNew('interview')} | ||
| // className="w-full px-4 py-2" | ||
| // > | ||
| // 인터뷰/면접 후기 | ||
| // </Link> | ||
| // </DropdownMenuItem> | ||
| // <DropdownMenuItem className="typo-body-3-3-r text-grey-color-5"> | ||
| // <Link | ||
| // href={AppPath.reviewNew('activity')} | ||
| // className="w-full px-4 py-2" | ||
| // > | ||
| // 활동 후기 | ||
| // </Link> | ||
| // </DropdownMenuItem> | ||
| // </DropdownMenuContent> | ||
| // </DropdownMenu> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove commented-out code.
The large block of commented-out dropdown menu implementation should be removed rather than kept in the codebase. Dead code makes the file harder to maintain and adds unnecessary clutter. If you need to reference the old implementation, it's available in version control history.
Apply this diff to remove the commented code:
- // <DropdownMenu>
- // <DropdownMenuTrigger asChild>
- // <Button size="small" variant="solid" className="typo-button">
- // 후기 작성
- // </Button>
- // </DropdownMenuTrigger>
- // <DropdownMenuContent className="w-40 mt-4 mr-10">
- // <DropdownMenuItem className="typo-body-3-3-r text-grey-color-5">
- // <Link href={AppPath.reviewNew('paper')} className="w-full px-4 py-2">
- // 서류 후기
- // </Link>
- // </DropdownMenuItem>
- // <DropdownMenuItem className="typo-body-3-3-r text-grey-color-5">
- // <Link
- // href={AppPath.reviewNew('interview')}
- // className="w-full px-4 py-2"
- // >
- // 인터뷰/면접 후기
- // </Link>
- // </DropdownMenuItem>
- // <DropdownMenuItem className="typo-body-3-3-r text-grey-color-5">
- // <Link
- // href={AppPath.reviewNew('activity')}
- // className="w-full px-4 py-2"
- // >
- // 활동 후기
- // </Link>
- // </DropdownMenuItem>
- // </DropdownMenuContent>
- // </DropdownMenu>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // <DropdownMenu> | |
| // <DropdownMenuTrigger asChild> | |
| // <Button size="small" variant="solid" className="typo-button"> | |
| // 후기 작성 | |
| // </Button> | |
| // </DropdownMenuTrigger> | |
| // <DropdownMenuContent className="w-40 mt-4 mr-10"> | |
| // <DropdownMenuItem className="typo-body-3-3-r text-grey-color-5"> | |
| // <Link href={AppPath.reviewNew('paper')} className="w-full px-4 py-2"> | |
| // 서류 후기 | |
| // </Link> | |
| // </DropdownMenuItem> | |
| // <DropdownMenuItem className="typo-body-3-3-r text-grey-color-5"> | |
| // <Link | |
| // href={AppPath.reviewNew('interview')} | |
| // className="w-full px-4 py-2" | |
| // > | |
| // 인터뷰/면접 후기 | |
| // </Link> | |
| // </DropdownMenuItem> | |
| // <DropdownMenuItem className="typo-body-3-3-r text-grey-color-5"> | |
| // <Link | |
| // href={AppPath.reviewNew('activity')} | |
| // className="w-full px-4 py-2" | |
| // > | |
| // 활동 후기 | |
| // </Link> | |
| // </DropdownMenuItem> | |
| // </DropdownMenuContent> | |
| // </DropdownMenu> |
🤖 Prompt for AI Agents
In src/components/molecules/layout/sections/ReviewWriteDropdown.tsx around lines
13 to 42, remove the entire commented-out DropdownMenu block (all lines starting
with // that comprise the old dropdown implementation) so the file contains no
dead/commented UI code; commit the change with a concise message like "chore:
remove commented-out ReviewWriteDropdown markup" and rely on VCS history if the
old implementation needs to be recovered.
| const onSubmit = async (data: PaperFormType) => { | ||
| try { | ||
| const apiData = transformToApiRequest(data) | ||
| await postBasicReviewMutation.mutateAsync(apiData) | ||
| router.push(AppPath.reviewSubmitted()) | ||
| } catch (error) { | ||
| console.error('Form submission error:', error) | ||
| } | ||
| } |
There was a problem hiding this comment.
Add user-facing error handling for form submission failures.
Errors are only logged to the console. Users won't know if their submission failed. Consider propagating the error or integrating with a toast/notification system.
+import { toast } from '@/components/ui/toast' // or your toast library
+
const onSubmit = async (data: PaperFormType) => {
try {
const apiData = transformToApiRequest(data)
await postBasicReviewMutation.mutateAsync(apiData)
router.push(AppPath.reviewSubmitted())
} catch (error) {
console.error('Form submission error:', error)
+ toast.error('제출에 실패했습니다. 다시 시도해주세요.')
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/components/pages/review/new/forms/paper/usePaperForm.ts around lines
212-220, the onSubmit catch block only logs errors to the console so the user
gets no feedback on failures; update the handler to show a user-facing error
(e.g., call the app toast/notification service with a clear message and optional
error details), avoid redirecting on failure, and propagate or rethrow the error
to upstream callers (or return a rejected promise) so calling code/tests can
react; also consider setting a form-level error state (or calling form.setError)
and disabling the submit button while the mutation is running.
| onSuccess: (data, variables, onMutateResult, context) => { | ||
| queryClient.invalidateQueries({ | ||
| queryKey: clubKeys.detail(variables.clubId), | ||
| }) | ||
| options?.onSuccess?.(data, variables, context as never) | ||
| options?.onSuccess?.(data, variables, onMutateResult, context) | ||
| }, |
There was a problem hiding this comment.
Incorrect callback signature: TanStack Query onSuccess has 3 parameters, not 4.
The onSuccess callback signature for useMutation is (data, variables, context). There is no onMutateResult parameter. The third parameter is already the context returned from onMutate.
- onSuccess: (data, variables, onMutateResult, context) => {
+ onSuccess: (data, variables, context) => {
queryClient.invalidateQueries({
queryKey: clubKeys.detail(variables.clubId),
})
- options?.onSuccess?.(data, variables, onMutateResult, context)
+ options?.onSuccess?.(data, variables, context)
},🤖 Prompt for AI Agents
In src/features/clubs/mutations.ts around lines 29 to 34, the onSuccess handler
uses an incorrect 4-argument signature (data, variables, onMutateResult,
context); TanStack Query mutation onSuccess is (data, variables, context).
Change the handler parameters to (data, variables, context) and forward those
three arguments when calling options?.onSuccess?.(data, variables, context);
adjust any local variable names and types accordingly so the third parameter is
the onMutate context.
| onSuccess: (data, variables, onMutateResult, context) => { | ||
| // 베이직 후기 목록 쿼리 무효화 | ||
| queryClient.invalidateQueries({ | ||
| queryKey: reviewKeys.basicLists(), | ||
| }) | ||
| options?.onSuccess?.(data, variables, context as never) | ||
| options?.onSuccess?.(data, variables, onMutateResult, context) |
There was a problem hiding this comment.
Incorrect onSuccess callback signature.
TanStack Query's useMutation onSuccess callback receives only 3 parameters: (data, variables, context) where context is the return value from onMutate. The current 4-parameter signature (data, variables, onMutateResult, context) doesn't match the library's API.
The third parameter you're receiving is already the onMutate result. Forwarding with 4 arguments will cause options?.onSuccess to receive misaligned parameters.
- onSuccess: (data, variables, onMutateResult, context) => {
+ onSuccess: (data, variables, context) => {
// 베이직 후기 목록 쿼리 무효화
queryClient.invalidateQueries({
queryKey: reviewKeys.basicLists(),
})
- options?.onSuccess?.(data, variables, onMutateResult, context)
+ options?.onSuccess?.(data, variables, context)
},TanStack Query v5 useMutation onSuccess callback parameters
🤖 Prompt for AI Agents
In src/features/review/mutations.ts around lines 23 to 28, the onSuccess
callback uses an incorrect 4-parameter signature (data, variables,
onMutateResult, context); TanStack Query v5 expects (data, variables, context)
where the third arg is the onMutate result. Change the callback signature to
(data, variables, context) and forward those three parameters to
options?.onSuccess?.(data, variables, context) after invalidating the query so
the downstream handler receives correctly aligned arguments.
| onSuccess: (data, variables, onMutateResult, context) => { | ||
| // 프리미엄 후기 목록 쿼리 무효화 | ||
| queryClient.invalidateQueries({ | ||
| queryKey: reviewKeys.premiumLists(), | ||
| }) | ||
| options?.onSuccess?.(data, variables, context as never) | ||
| options?.onSuccess?.(data, variables, onMutateResult, context) |
There was a problem hiding this comment.
Same issue: incorrect onSuccess callback signature.
Apply the same fix as noted above for usePostBasicReview.
- onSuccess: (data, variables, onMutateResult, context) => {
+ onSuccess: (data, variables, context) => {
// 프리미엄 후기 목록 쿼리 무효화
queryClient.invalidateQueries({
queryKey: reviewKeys.premiumLists(),
})
- options?.onSuccess?.(data, variables, onMutateResult, context)
+ options?.onSuccess?.(data, variables, context)
},🤖 Prompt for AI Agents
In src/features/review/mutations.ts around lines 50 to 55, the onSuccess
callback uses an incorrect signature (it takes onMutateResult) which doesn't
match React Query's mutation onSuccess signature; change the handler to (data,
variables, context) and call options?.onSuccess?.(data, variables, context)
after invalidating reviewKeys.premiumLists(), and update any TypeScript types if
necessary so the forwarded callback receives the correct parameters.
2 participants
🔗 관련 이슈
Closes #
✨ 변경사항
🎯 리뷰 포인트
📝 추가 정보
Summary by CodeRabbit
New Features
Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.