Conversation
… feat/pre-opinion-new-62
… feat/pre-opinion-new-62
… feat/pre-opinion-new-62
… feat/pre-opinion-new-62
Closed
7 tasks
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughPre-opinion 작성 기능이 새로 추가되었습니다: API 엔드포인트·타입·목데이터·훅·페이지·컴포넌트가 포함되며, 키워드 API 리팩토링과 BookReviewForm 도입으로 BookReviewModal이 통합되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User as 사용자
participant Page as PreOpinionWritePage
participant Comp as BookReviewSection/TopicItem
participant Form as BookReviewForm
participant Hook as usePreOpinion / useSavePreOpinion / useSubmitPreOpinion
participant Query as QueryClient
participant API as API
User->>Page: 진입 (gatheringId, meetingId)
Page->>Hook: usePreOpinion(params)
Hook->>API: GET pre-opinion (DETAIL)
API-->>Hook: preOpinion 데이터 반환
Hook-->>Page: 데이터 제공
Page->>Comp: initial props 전달
Comp->>Form: initialRating, initialKeywordIds
Form->>API: 키워드 로드 (KEYWORDS_ENDPOINTS.LIST)
API-->>Form: 키워드 반환
User->>Form: 별점/키워드 선택
Form-->>Page: onChange(rating, keywordIds)
User->>Page: 저장 클릭
Page->>Hook: useSavePreOpinion.mutate(body)
Hook->>API: POST/PATCH CREATE/UPDATE
API-->>Hook: 저장 완료
Hook->>Query: invalidate preOpinionQueryKeys.all
User->>Page: 공유 클릭
Page->>Hook: useSubmitPreOpinion.mutate(body)
Hook->>API: PATCH SUBMIT
API-->>Hook: 제출 완료
Hook->>Query: invalidate preOpinionQueryKeys.all & topicQueryKeys.confirmed()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 분 Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (12)
src/shared/components/SubPageHeader.tsx (1)
46-46: 매직 넘버59px에 대한 의도가 불분명합니다.고정 높이
h-[59px]가 특정 디자인 스펙에 맞춘 값이라면, 다른 곳에서도 참조할 수 있도록 Tailwind 커스텀 토큰이나 주석으로 근거를 남겨두면 유지보수에 도움됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/SubPageHeader.tsx` at line 46, The fixed height h-[59px] in the SubPageHeader component is a magic number; replace it with a named design token or Tailwind utility and/or document its origin: update the Tailwind config to add a custom height like h-layout-header or use a CSS variable (e.g., var(--header-height)) and apply that class instead of h-[59px], and add a short inline comment in SubPageHeader explaining the design spec or ticket that dictates the height so other components can reuse it and maintainers understand the source.src/routes/index.tsx (2)
24-24:PreOpinionWritePagebarrel import 미사용다른 페이지들은
@/pagesbarrel에서 import하는데,PreOpinionWritePage만 직접 경로로 import하고 있습니다. barrel에 추가하면 일관성이 유지됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/index.tsx` at line 24, 현재 파일에서 PreOpinionWritePage를 직접 경로로 import 하고 있어 다른 페이지들과 달리 barrel('@/pages')를 사용하지 않습니다; PreOpinionWritePage를 pages barrel에 export(예: 추가 export { default as PreOpinionWritePage } from './PreOpinions/PreOpinionWritePage')하고 이 파일의 import를 기존 페이지들과 동일하게 import { PreOpinionWritePage } from '@/pages'로 변경해 일관성을 유지하세요.
133-136: 라우트 정의에ROUTES.*()헬퍼 대신 인라인 패턴 사용 권장
ROUTES.PRE_OPINION_WRITE()는 네비게이션 시 실제 URL을 생성하는 용도입니다. 라우트path정의에서는 인라인 패턴을 사용하는 것이 관례입니다.♻️ 인라인 패턴으로 변경
{ - path: ROUTES.PRE_OPINION_WRITE(':gatheringId', ':meetingId'), + path: `${ROUTES.GATHERINGS}/:gatheringId/meetings/:meetingId/pre-opinions/new`, element: <PreOpinionWritePage />, },Based on learnings: "In React Router route definitions, define path patterns inline instead of using ROUTES.() helper functions. ROUTES.() are intended for generating actual URLs during navigation."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/routes/index.tsx` around lines 133 - 136, The route is using the URL-generator helper ROUTES.PRE_OPINION_WRITE(':gatheringId', ':meetingId') for the route path; replace that with an inline path pattern (use the same param names :gatheringId and :meetingId) in the route definition that renders PreOpinionWritePage so the route path is a pattern string instead of calling ROUTES.*(). Locate the route entry that includes ROUTES.PRE_OPINION_WRITE and change its path to the equivalent inline pattern (keeping param names) so navigation still uses ROUTES.PRE_OPINION_WRITE(...) but route matching uses the inline pattern.src/features/pre-opinion/components/PreOpinionQuestionSection.tsx (1)
25-26:sortedTopics메모이제이션 고려매 렌더마다
[...topics].sort()가 재실행됩니다.topics배열이 변경되지 않는 한 불필요한 정렬입니다.♻️ useMemo 적용
+import { useMemo } from 'react' + const PreOpinionQuestionSection = ({ topics, onChange }: PreOpinionQuestionSectionProps) => { - const sortedTopics = [...topics].sort((a, b) => a.confirmOrder - b.confirmOrder) + const sortedTopics = useMemo( + () => [...topics].sort((a, b) => a.confirmOrder - b.confirmOrder), + [topics] + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/components/PreOpinionQuestionSection.tsx` around lines 25 - 26, The component PreOpinionQuestionSection recreates sortedTopics on every render; wrap the sort in a useMemo to avoid unnecessary work: replace the direct assignment to sortedTopics with a useMemo call that returns [...topics].sort((a,b)=>a.confirmOrder - b.confirmOrder) and use topics as the dependency; ensure you import useMemo from React and keep the same variable name (sortedTopics) so the rest of the component is unchanged.src/features/pre-opinion/components/PreOpinionWriteHeader.tsx (1)
62-66:top-[123px]매직 넘버
SubPageHeader높이에 의존하는 하드코딩된 값입니다. 헤더 높이가 변경되면 sticky 위치가 어긋납니다. CSS 변수나 공유 상수로 관리하면 유지보수가 쉬워집니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/components/PreOpinionWriteHeader.tsx` around lines 62 - 66, The hardcoded "top-[123px]" in the className of PreOpinionWriteHeader (the sticky container using isStuck) relies on SubPageHeader's height and should be replaced with a shared value; define a CSS variable (e.g. --subpage-header-height) on SubPageHeader (or export a shared JS/TS constant like SUBPAGE_HEADER_HEIGHT) and use that variable instead of the magic number in PreOpinionWriteHeader's className so the sticky top updates automatically when header height changes; update SubPageHeader to set the CSS variable (or update the shared constant source) and replace "top-[123px]" in PreOpinionWriteHeader with "top-[var(--subpage-header-height)]" (or build the class using the shared constant) and ensure any other locations using the same magic number are migrated.src/features/keywords/index.ts (1)
4-4: mock 모듈의 barrel export 재고려
keywords.mock을 public barrel에서 re-export하면 프로덕션 코드에서도 import가 가능해져 번들에 포함될 수 있습니다. MSW 핸들러 등 개발 전용 코드에서만 사용된다면, barrel에서 제외하고 직접 경로로 import하는 것이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/keywords/index.ts` at line 4, The public barrel currently re-exports the development-only mock (export * from './keywords.mock'), which can leak mocks into production bundles; remove that export from the barrel (the export line in src/features/keywords/index.ts) so the mock is not re-exported, and update any tests/MSW handlers to import the mock directly from its module path ('./keywords.mock') instead of from the barrel; alternatively, if you need type-only exports, convert to a type-only export to avoid runtime inclusion.src/features/pre-opinion/hooks/usePreOpinion.ts (1)
37-42:staleTime미설정 — 작성 중 불필요한 refetch 발생 가능
staleTime이 기본값(0)이므로 윈도우 포커스 시마다 refetch가 발생합니다. 사전 의견 작성 페이지에서는 사용자가 폼을 채우는 동안 불필요한 네트워크 요청이 일어날 수 있습니다.
gcTime과 유사하게staleTime도 설정하면 작성 중 불필요한 요청을 방지할 수 있습니다.💡 제안
return useQuery<GetPreOpinionResponse>({ queryKey: preOpinionQueryKeys.detail(params), queryFn: () => getPreOpinion(params), enabled: isValidParams, + staleTime: 10 * 60 * 1000, gcTime: 10 * 60 * 1000, })As per coding guidelines: "서버 상태는 TanStack Query 사용을 기본으로 가정하고, queryKey 안정성, enabled 조건, select 비용, invalidate/refetch 타이밍을 중점적으로 봐줘."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/hooks/usePreOpinion.ts` around lines 37 - 42, In usePreOpinion where useQuery is called (queryKey: preOpinionQueryKeys.detail(params), queryFn: () => getPreOpinion(params)), add a staleTime option to prevent unnecessary refetches during form editing; set staleTime to a suitable value (for example matching gcTime: 10 * 60 * 1000) alongside the existing gcTime and enabled settings so the query won’t be treated as stale on window focus while the user is writing.src/features/pre-opinion/preOpinion.mock.ts (1)
58-60: 동일 객체 참조 반환 — mock 데이터 오염 가능성
mockPreOpinionDetail을 그대로 반환하면, 호출부에서 객체를 직접 변경할 경우 이후 호출 결과도 오염됩니다. 실 API는 매번 새 객체를 주므로 동작 차이가 생길 수 있습니다.
structuredClone또는 스프레드로 복사해 반환하면 안전합니다.💡 제안
export const getMockPreOpinionDetail = (): GetPreOpinionResponse => { - return mockPreOpinionDetail + return structuredClone(mockPreOpinionDetail) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/preOpinion.mock.ts` around lines 58 - 60, getMockPreOpinionDetail currently returns the shared mockPreOpinionDetail object by reference which can lead to test/data pollution; change getMockPreOpinionDetail to return a fresh deep copy (e.g., using structuredClone(mockPreOpinionDetail) or an equivalent deep-copy) so each call returns an independent object and mutations won't affect future calls or tests.src/features/pre-opinion/preOpinion.api.ts (2)
52-60: savePreOpinion vs submitPreOpinion 파라미터 스타일 불일치
savePreOpinion은 객체 구조분해({ gatheringId, meetingId, isFirstSave })를 사용하고,submitPreOpinion은 개별 인자(gatheringId: number, meetingId: number)를 사용합니다. 호출하는 쪽(hooks)에서도 패턴이 달라져 혼란을 줄 수 있습니다.가능하다면 동일한 패턴으로 통일하는 것을 권장합니다.
예시: submitPreOpinion도 객체 파라미터로 통일
export const submitPreOpinion = async ( - gatheringId: number, - meetingId: number, + { gatheringId, meetingId }: GetPreOpinionParams, body: SubmitPreOpinionBody ): Promise<void> => { return api.patch(PRE_OPINION_ENDPOINTS.SUBMIT(gatheringId, meetingId), body) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/preOpinion.api.ts` around lines 52 - 60, savePreOpinion uses an object destructured param ({ gatheringId, meetingId, isFirstSave }) while submitPreOpinion uses individual args (gatheringId: number, meetingId: number), causing inconsistent call patterns; make them consistent by updating submitPreOpinion to accept a single params object (e.g., { gatheringId, meetingId }) and adjust its implementation and all callers (hooks) to destructure that object, or alternatively change savePreOpinion to accept positional args—ensure you update function signatures and all imports/usages for submitPreOpinion/savePreOpinion and any calling hooks to match the chosen style.
31-41: mock 분기가 getPreOpinion에만 존재
savePreOpinion과submitPreOpinion에는 mock 분기가 없어서,USE_MOCK=true환경에서 저장/제출 시 실제 API 호출이 발생합니다. 의도된 것이라면 무시해도 됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/preOpinion.api.ts` around lines 31 - 41, The mock-only branch currently exists for getPreOpinion but not for savePreOpinion or submitPreOpinion, so when USE_MOCK=true those two still call the real API; add symmetrical mock branches in the savePreOpinion and submitPreOpinion functions that check USE_MOCK, await a short timeout (like getPreOpinion does), and return appropriate mock responses (or create/get corresponding helpers similar to getMockPreOpinionDetail) instead of calling api.post/api.put; reference the functions savePreOpinion and submitPreOpinion and the global flag USE_MOCK to locate where to add these branches.src/pages/PreOpinions/PreOpinionWritePage.tsx (2)
12-15: URL 파라미터 유효성 방어
useParams에서gatheringId나meetingId가undefined일 경우Number(undefined)→NaN이 됩니다.usePreOpinion의enabled조건에서NaN을 걸러주고 있지만, 페이지 레벨에서 잘못된 경로 접근 시 early return이나 리다이렉트를 두면 더 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/PreOpinions/PreOpinionWritePage.tsx` around lines 12 - 15, PreOpinionWritePage currently converts useParams values to numbers which yields NaN when gatheringId or meetingId are undefined; add defensive checks in PreOpinionWritePage before creating numGatheringId/numMeetingId: verify gatheringId and meetingId are non-empty strings and that Number(gatheringId)/Number(meetingId) are valid (not NaN), and if invalid perform an early return or redirect (e.g., render a fallback/error UI or navigate away) so downstream hooks like usePreOpinion (enabled condition) are never invoked with invalid IDs; update any references to numGatheringId/numMeetingId accordingly.
94-98:handleSave에도 성공/실패 피드백 부재저장 성공/실패 시 사용자에게 아무런 피드백(토스트, 알림 등)이 없습니다. UX 측면에서 저장 완료 확인이 필요합니다.
useSavePreOpinion훅의onSuccess/onError콜백이나 페이지 레벨에서 처리를 추가하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/PreOpinions/PreOpinionWritePage.tsx` around lines 94 - 98, handleSave currently calls save(body) but provides no user feedback on success or failure; wire up success/error to show a toast/notification by either passing onSuccess/onError into the useSavePreOpinion hook or handling the returned mutation object here. Specifically, locate useSavePreOpinion (or the save function returned by it), add an onSuccess callback to show a success toast and optionally navigate, and add an onError callback to show an error toast with the error message; ensure handleSave still builds the body via buildSaveBody and calls save(body) but that save triggers those callbacks for user feedback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/book/components/BookReviewForm.tsx`:
- Around line 50-51: The component BookReviewForm currently initializes local
state with useState(initialRating) and useState<number[]>(initialKeywordIds) but
never syncs when those props change; add a useEffect inside BookReviewForm that
watches initialRating and initialKeywordIds and calls setRating(initialRating)
and setSelectedKeywordIds(initialKeywordIds) to keep the form in sync after
parent refetches, or alternatively ensure the parent (e.g., PreOpinionWritePage)
forces a remount by changing the key prop when new review data is loaded.
- Line 174: In BookReviewForm.tsx update the paragraph element that renders
rating.toFixed(1) to use the consistent typography class name: replace
className="subtitle5" with className="typo-subtitle5" (the same prefix pattern
used elsewhere like the nearby "typo-body4"); modify the className on the <p>
that contains rating.toFixed(1) so it references "typo-subtitle5".
In `@src/features/book/components/BookReviewModal.tsx`:
- Around line 37-42: The modal keeps the child form state between opens because
Radix Dialog doesn't unmount children; update BookReviewModal to reset or
remount BookReviewForm when closed: either (A) pass a changing key derived from
the open prop to BookReviewForm (e.g., key based on open) so BookReviewForm
remounts and clears its internal state, or (B) wrap onOpenChange in a handler
(e.g., handleOpenChange) that calls setFormValues({ rating: 0, keywordIds: [],
isValid: false }) when newOpen is false before delegating to the original
onOpenChange; apply the change in the BookReviewModal component where
BookReviewForm is rendered.
In `@src/features/pre-opinion/components/PreOpinionQuestionSection.tsx`:
- Around line 41-44: The Textarea uses defaultValue so updates from refetched
topics aren't reflected; change it to a controlled component by replacing
defaultValue with value={topic.content ?? ''} and keep onChange calling
onChange?.(topic.topicId, e.target.value) (referencing the Textarea component
and onChange handler and topic.topicId/topic.content), or alternatively make the
wrapper key include topic.content (e.g.,
key={`${topic.topicId}-${topic.content}`}) to force remounts if you prefer
remounting behavior.
In `@src/features/pre-opinion/lib/date.tsx`:
- Around line 1-9: formatUpdatedAt currently constructs a Date from dateStr and
will render "NaN..." for invalid input and leaves hours unpadded; update
formatUpdatedAt to validate the Date (e.g., isNaN(date.getTime())) and return a
safe fallback (empty string or a localized "저장되지 않음") when invalid, and pad
month, day and hours with padStart(2,'0') so times like "09:05" render
uniformly; also rename the file to .ts since it contains no JSX. Reference
formatUpdatedAt and the local variables date, year, month, day, hours, minutes
when making the changes.
In `@src/pages/PreOpinions/PreOpinionWritePage.tsx`:
- Around line 34-44: The isFirstSave logic evaluates to false when preOpinion is
not yet loaded because undefined === null is false and the ?? true fallback
doesn't apply; change the computation so that a missing preOpinion yields true
explicitly (e.g. compute isFirstSave by first checking if preOpinion is
null/undefined and returning true, otherwise checking
preOpinion.preOpinion.updatedAt === null) and pass that boolean into
useSavePreOpinion (referencing isFirstSave, preOpinion?.preOpinion.updatedAt,
and useSavePreOpinion).
- Around line 100-111: handleSubmit currently calls submit as a fire-and-forget
mutate which prevents error handling; change it to await the async variant
(submitAsync) and wrap the call in a try-catch so failures are handled (e.g.,
show an error toast or set an error state) after ensuring
buildSubmitBody/buildSaveBody and saveAsync flow remain awaited; reference
handleSubmit, submit -> replace with submitAsync, saveAsync, buildSubmitBody,
buildSaveBody, and isFirstSave when making the change.
In `@src/shared/assets/icon/FilledInfo.tsx`:
- Around line 17-19: In the FilledInfo React component (FilledInfo.tsx) replace
the invalid HTML SVG attributes on the <svg> element—stroke-width,
stroke-linecap, stroke-linejoin—with their JSX camelCase equivalents
strokeWidth, strokeLinecap, and strokeLinejoin so they match the <path> usage
and avoid React console warnings and broken styling.
---
Nitpick comments:
In `@src/features/keywords/index.ts`:
- Line 4: The public barrel currently re-exports the development-only mock
(export * from './keywords.mock'), which can leak mocks into production bundles;
remove that export from the barrel (the export line in
src/features/keywords/index.ts) so the mock is not re-exported, and update any
tests/MSW handlers to import the mock directly from its module path
('./keywords.mock') instead of from the barrel; alternatively, if you need
type-only exports, convert to a type-only export to avoid runtime inclusion.
In `@src/features/pre-opinion/components/PreOpinionQuestionSection.tsx`:
- Around line 25-26: The component PreOpinionQuestionSection recreates
sortedTopics on every render; wrap the sort in a useMemo to avoid unnecessary
work: replace the direct assignment to sortedTopics with a useMemo call that
returns [...topics].sort((a,b)=>a.confirmOrder - b.confirmOrder) and use topics
as the dependency; ensure you import useMemo from React and keep the same
variable name (sortedTopics) so the rest of the component is unchanged.
In `@src/features/pre-opinion/components/PreOpinionWriteHeader.tsx`:
- Around line 62-66: The hardcoded "top-[123px]" in the className of
PreOpinionWriteHeader (the sticky container using isStuck) relies on
SubPageHeader's height and should be replaced with a shared value; define a CSS
variable (e.g. --subpage-header-height) on SubPageHeader (or export a shared
JS/TS constant like SUBPAGE_HEADER_HEIGHT) and use that variable instead of the
magic number in PreOpinionWriteHeader's className so the sticky top updates
automatically when header height changes; update SubPageHeader to set the CSS
variable (or update the shared constant source) and replace "top-[123px]" in
PreOpinionWriteHeader with "top-[var(--subpage-header-height)]" (or build the
class using the shared constant) and ensure any other locations using the same
magic number are migrated.
In `@src/features/pre-opinion/hooks/usePreOpinion.ts`:
- Around line 37-42: In usePreOpinion where useQuery is called (queryKey:
preOpinionQueryKeys.detail(params), queryFn: () => getPreOpinion(params)), add a
staleTime option to prevent unnecessary refetches during form editing; set
staleTime to a suitable value (for example matching gcTime: 10 * 60 * 1000)
alongside the existing gcTime and enabled settings so the query won’t be treated
as stale on window focus while the user is writing.
In `@src/features/pre-opinion/preOpinion.api.ts`:
- Around line 52-60: savePreOpinion uses an object destructured param ({
gatheringId, meetingId, isFirstSave }) while submitPreOpinion uses individual
args (gatheringId: number, meetingId: number), causing inconsistent call
patterns; make them consistent by updating submitPreOpinion to accept a single
params object (e.g., { gatheringId, meetingId }) and adjust its implementation
and all callers (hooks) to destructure that object, or alternatively change
savePreOpinion to accept positional args—ensure you update function signatures
and all imports/usages for submitPreOpinion/savePreOpinion and any calling hooks
to match the chosen style.
- Around line 31-41: The mock-only branch currently exists for getPreOpinion but
not for savePreOpinion or submitPreOpinion, so when USE_MOCK=true those two
still call the real API; add symmetrical mock branches in the savePreOpinion and
submitPreOpinion functions that check USE_MOCK, await a short timeout (like
getPreOpinion does), and return appropriate mock responses (or create/get
corresponding helpers similar to getMockPreOpinionDetail) instead of calling
api.post/api.put; reference the functions savePreOpinion and submitPreOpinion
and the global flag USE_MOCK to locate where to add these branches.
In `@src/features/pre-opinion/preOpinion.mock.ts`:
- Around line 58-60: getMockPreOpinionDetail currently returns the shared
mockPreOpinionDetail object by reference which can lead to test/data pollution;
change getMockPreOpinionDetail to return a fresh deep copy (e.g., using
structuredClone(mockPreOpinionDetail) or an equivalent deep-copy) so each call
returns an independent object and mutations won't affect future calls or tests.
In `@src/pages/PreOpinions/PreOpinionWritePage.tsx`:
- Around line 12-15: PreOpinionWritePage currently converts useParams values to
numbers which yields NaN when gatheringId or meetingId are undefined; add
defensive checks in PreOpinionWritePage before creating
numGatheringId/numMeetingId: verify gatheringId and meetingId are non-empty
strings and that Number(gatheringId)/Number(meetingId) are valid (not NaN), and
if invalid perform an early return or redirect (e.g., render a fallback/error UI
or navigate away) so downstream hooks like usePreOpinion (enabled condition) are
never invoked with invalid IDs; update any references to
numGatheringId/numMeetingId accordingly.
- Around line 94-98: handleSave currently calls save(body) but provides no user
feedback on success or failure; wire up success/error to show a
toast/notification by either passing onSuccess/onError into the
useSavePreOpinion hook or handling the returned mutation object here.
Specifically, locate useSavePreOpinion (or the save function returned by it),
add an onSuccess callback to show a success toast and optionally navigate, and
add an onError callback to show an error toast with the error message; ensure
handleSave still builds the body via buildSaveBody and calls save(body) but that
save triggers those callbacks for user feedback.
In `@src/routes/index.tsx`:
- Line 24: 현재 파일에서 PreOpinionWritePage를 직접 경로로 import 하고 있어 다른 페이지들과 달리
barrel('@/pages')를 사용하지 않습니다; PreOpinionWritePage를 pages barrel에 export(예: 추가
export { default as PreOpinionWritePage } from
'./PreOpinions/PreOpinionWritePage')하고 이 파일의 import를 기존 페이지들과 동일하게 import {
PreOpinionWritePage } from '@/pages'로 변경해 일관성을 유지하세요.
- Around line 133-136: The route is using the URL-generator helper
ROUTES.PRE_OPINION_WRITE(':gatheringId', ':meetingId') for the route path;
replace that with an inline path pattern (use the same param names :gatheringId
and :meetingId) in the route definition that renders PreOpinionWritePage so the
route path is a pattern string instead of calling ROUTES.*(). Locate the route
entry that includes ROUTES.PRE_OPINION_WRITE and change its path to the
equivalent inline pattern (keeping param names) so navigation still uses
ROUTES.PRE_OPINION_WRITE(...) but route matching uses the inline pattern.
In `@src/shared/components/SubPageHeader.tsx`:
- Line 46: The fixed height h-[59px] in the SubPageHeader component is a magic
number; replace it with a named design token or Tailwind utility and/or document
its origin: update the Tailwind config to add a custom height like
h-layout-header or use a CSS variable (e.g., var(--header-height)) and apply
that class instead of h-[59px], and add a short inline comment in SubPageHeader
explaining the design spec or ticket that dictates the height so other
components can reuse it and maintainers understand the source.
src/features/pre-opinion/components/PreOpinionQuestionSection.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
src/shared/assets/icon/FilledInfo.tsx (1)
23-24:stroke="white"하드코딩 — 테마/배경 변경 시 시각적 깨짐 위험흰색 stroke를 하드코딩하면, 아이콘이 밝은 배경에 렌더링되거나 다크모드 등 테마 전환 시 획이 보이지 않게 됩니다.
white대신currentColor의 반전색을 CSS 변수로 관리하거나, Tailwind의text-white클래스를 활용하는 방식을 고려해 주세요.또한
strokeWidth="2"는 SVG 루트에서 이미 상속되므로 각<path>에서 중복 선언하지 않아도 됩니다.♻️ 개선 예시
- <path d="M12 16v-4" stroke="white" strokeWidth="2" /> - <path d="M12 8h.01" stroke="white" strokeWidth="2" /> + <path d="M12 16v-4" stroke="white" /> + <path d="M12 8h.01" stroke="white" />배경이 항상 진한 색상으로 보장된다면 현재
stroke="white"유지도 가능하나, 그 전제가 깨질 경우 CSS 변수(var(--icon-inner-color)등)로 교체를 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/assets/icon/FilledInfo.tsx` around lines 23 - 24, Replace the hardcoded stroke and duplicated strokeWidth on the <path> elements in the FilledInfo component: remove stroke="white" and per-path strokeWidth="2", and instead expose/control the stroke color and width on the root <svg> (or via a passed className/prop) so the icon inherits color (use "currentColor" or a CSS variable like --icon-inner-color) and a single strokeWidth at the SVG root; ensure the component accepts className/props so callers can apply Tailwind classes (e.g., text-white) or theme CSS to control the stroke color.src/features/pre-opinion/components/PreOpinionQuestionSection.tsx (1)
67-68:sortedTopics—useMemo로 감싸면 불필요한 재정렬 방지 가능.매 렌더마다 배열 복사 + 정렬이 실행됩니다. topic 수가 적어 실질적 영향은 없지만,
topics참조가 바뀔 때만 정렬하도록useMemo를 적용하면 더 명확합니다.♻️ 제안
+import { useMemo } from 'react' + const PreOpinionQuestionSection = ({ topics, onChange }: PreOpinionQuestionSectionProps) => { - const sortedTopics = [...topics].sort((a, b) => a.confirmOrder - b.confirmOrder) + const sortedTopics = useMemo( + () => [...topics].sort((a, b) => a.confirmOrder - b.confirmOrder), + [topics], + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/components/PreOpinionQuestionSection.tsx` around lines 67 - 68, Wrap the sortedTopics computation in React's useMemo inside the PreOpinionQuestionSection component so the array copy + sort only runs when the topics prop changes; replace const sortedTopics = [...topics].sort(...) with a useMemo that returns the sorted array and lists topics as its dependency, ensuring stable identity and avoiding unnecessary re-sorts across renders.src/pages/PreOpinions/PreOpinionWritePage.tsx (2)
94-98:handleSave도 fire-and-forget — 저장 실패 시 사용자 피드백 부재.
save(mutate)는 에러를 반환하지 않으므로, 네트워크 오류 등으로 저장 실패 시 사용자가 저장된 줄 알고 페이지를 떠날 수 있습니다.mutateAsync+ try-catch 또는onError콜백으로 최소한의 실패 피드백을 제공하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/PreOpinions/PreOpinionWritePage.tsx` around lines 94 - 98, handleSave currently calls save (a fire-and-forget mutate) so failures give no user feedback; change handleSave to call the async variant (e.g., save.mutateAsync or the provided mutateAsync API) and wrap it in try-catch (or use save.mutate with an onError callback) to surface errors to the user (toast/alert/setError state) and prevent navigation on failure; update references to buildSaveBody and save in the useCallback and ensure any loading state is handled while awaiting the mutation.
25-32:useEffect로reviewRef동기화 — 의도는 명확하나 한 가지 참고.서버 데이터가 바뀔 때마다(refetch 후)
reviewRef를 덮어씁니다.onSuccess에서만invalidateQueries가 실행되므로 저장 실패 시 refetch가 발생하지 않아 사용자 입력이 유실되지는 않습니다.다만
preOpinion?.review는 객체 참조이므로, refetch마다 새 참조가 생성되어 effect가 매번 실행됩니다. 사용자가 폼을 수정한 직후 다른 원인으로 refetch가 발생하면 로컬 수정이 서버 값으로 리셋될 수 있습니다. 현재 흐름에서는 문제가 되지 않지만, 추후 주의가 필요한 부분입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/PreOpinions/PreOpinionWritePage.tsx` around lines 25 - 32, 현재 useEffect가 preOpinion?.review의 새 참조마다 reviewRef를 덮어써서 사용자가 편집한 로컬 변경이 refetch에 의해 덮어씌워질 가능성이 있습니다; 이를 고치려면 useEffect(및 관련 로직)에서 항상 덮어쓰지 않고 로컬 편집 여부를 확인해 초기 동기화만 수행하도록 변경하세요 — 예: reviewRef 또는 별도 플래그(hasLocalChanges/isPristine)를 활용해 reviewRef.current.isValid가 아직 설정되지 않았을 때(또는 로컬 변경이 없다면)만 reviewRef를 preOpinion.review의 rating과 keywordIds로 설정하고, onSuccess/invalidateQueries 흐름과 함께 작동하도록 조정하세요 (참조 대상: useEffect, reviewRef, preOpinion?.review, onSuccess, invalidateQueries).src/features/book/components/BookReviewForm.tsx (1)
61-61:initialKeywordKey를useMemo로 감싸는 것을 고려현재 렌더마다
.slice().sort().join()연산이 수행됩니다. 문자열 원시값이므로useEffect의존성 비교는 올바르게 동작하지만, 어떤 값이 훅의 의존성으로 사용된다면useMemo로 감싸는 것이 권장됩니다.♻️ 리팩토링 제안
- const initialKeywordKey = initialKeywordIds.slice().sort().join(',') + const initialKeywordKey = useMemo( + () => initialKeywordIds.slice().sort().join(','), + [initialKeywordIds] + ) useEffect(() => { const ids = initialKeywordKey ? initialKeywordKey.split(',').map(Number) : [] setSelectedKeywordIds(ids) }, [initialKeywordKey])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/book/components/BookReviewForm.tsx` at line 61, Wrap the computation of initialKeywordKey in a useMemo to avoid recalculating initialKeywordIds.slice().sort().join(',') on every render; specifically, replace the direct assignment of initialKeywordKey with a useMemo that returns initialKeywordIds.slice().sort().join(',') and uses [initialKeywordIds] as the dependency array so the memo updates when the source array changes (referencing initialKeywordKey and initialKeywordIds in the change).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/book/components/BookReviewForm.tsx`:
- Around line 57-65: The component updates local state in the useEffect blocks
(setRating and setSelectedKeywordIds derived from initialRating and
initialKeywordIds/initialKeywordKey) but never notifies the parent, causing
parent/child state mismatch; fix by invoking the component's onChange callback
after you call setRating or setSelectedKeywordIds inside those useEffect hooks
(pass the new rating/keyword id array in the same shape the parent expects), or
if you prefer to avoid syncing logic here, document and implement the
alternative approach: require callers to force a remount by changing the
BookReviewForm key when initial props change so initial useState values are
authoritative; reference useEffect, setRating, initialRating, initialKeywordIds,
initialKeywordKey, setSelectedKeywordIds, and onChange to locate the code to
modify.
In `@src/features/book/components/BookReviewModal.tsx`:
- Around line 84-88: The submit button enabled state is inconsistent because
BookReviewForm.isValid only checks keywords and not the star rating, causing
formValues.isValid to be true while rating === 0 triggers an error in
handleSubmit; update the logic so the button is disabled when rating is 0 by
either extending BookReviewForm.isValid to include rating > 0 or by changing the
Button disabled prop to disabled={!formValues.isValid || rating === 0 ||
isPending}; modify the BookReviewModal component (where formValues.isValid,
isPending, handleSubmit and rating are used) accordingly so the UI accurately
reflects actual submitability.
- Around line 63-66: onSuccess currently calls onOpenChange(false) directly
which bypasses handleOpenChange and prevents setFormValues(INITIAL_FORM_VALUES)
from resetting formValues; change the onSuccess handler to call
handleOpenChange(false) so handleOpenChange will run
setFormValues(INITIAL_FORM_VALUES) and reset form state before the modal closes
(affecting onSuccess, handleOpenChange, onOpenChange, setFormValues,
INITIAL_FORM_VALUES, formValues, BookReviewForm).
In `@src/pages/PreOpinions/PreOpinionWritePage.tsx`:
- Around line 100-115: handleSubmit currently swallows errors from
saveAsync/submitAsync in the catch block while the hooks
useSavePreOpinion/useSubmitPreOpinion do not provide onError handlers; fix by
either (A) updating useSavePreOpinion and useSubmitPreOpinion to accept and call
an onError callback (pass that callback in where saveAsync/submitAsync are
created) or (B) change handleSubmit's catch to accept the error parameter and
surface feedback (e.g., call the app's toast/notification utility and
processLogger.error) so users see a failure message when saveAsync or
submitAsync fail; reference handleSubmit, saveAsync, submitAsync,
useSavePreOpinion and useSubmitPreOpinion when making the change.
- Around line 117-126: The current render in PreOpinionWritePage shows a spinner
when !preOpinion which causes an infinite spinner on query errors; update the
component to also check the usePreOpinion hook's isError (and error) alongside
isLoading and preOpinion, and render an error state (e.g., SubPageHeader plus a
centered error message and optional retry control) when isError is true; locate
the hook usage (usePreOpinion) and the early return block that checks isLoading
|| !preOpinion and modify it to handle isError explicitly and display the error
UI instead of the spinner.
---
Duplicate comments:
In `@src/features/book/components/BookReviewModal.tsx`:
- Around line 48-53: The modal previously persisted form state across reopen,
but the combination of resetting form values in handleOpenChange
(setFormValues(INITIAL_FORM_VALUES)) when newOpen is false and the remount
strategy using key={open ? 'open' : 'closed'} correctly resolves it; no code
change required—keep handleOpenChange and the key prop as implemented to ensure
the form is reset on manual close and remounted fresh on open.
In `@src/shared/assets/icon/FilledInfo.tsx`:
- Around line 17-19: The SVG attribute naming has already been corrected from
kebab-case to camelCase (e.g., strokeWidth, strokeLinecap, strokeLinejoin) in
the FilledInfo component; no code change needed — confirm the component/function
FilledInfo (or the JSX element in src/shared/assets/icon/FilledInfo.tsx) uses
strokeWidth, strokeLinecap, and strokeLinejoin consistently and approve the
change.
---
Nitpick comments:
In `@src/features/book/components/BookReviewForm.tsx`:
- Line 61: Wrap the computation of initialKeywordKey in a useMemo to avoid
recalculating initialKeywordIds.slice().sort().join(',') on every render;
specifically, replace the direct assignment of initialKeywordKey with a useMemo
that returns initialKeywordIds.slice().sort().join(',') and uses
[initialKeywordIds] as the dependency array so the memo updates when the source
array changes (referencing initialKeywordKey and initialKeywordIds in the
change).
In `@src/features/pre-opinion/components/PreOpinionQuestionSection.tsx`:
- Around line 67-68: Wrap the sortedTopics computation in React's useMemo inside
the PreOpinionQuestionSection component so the array copy + sort only runs when
the topics prop changes; replace const sortedTopics = [...topics].sort(...) with
a useMemo that returns the sorted array and lists topics as its dependency,
ensuring stable identity and avoiding unnecessary re-sorts across renders.
In `@src/pages/PreOpinions/PreOpinionWritePage.tsx`:
- Around line 94-98: handleSave currently calls save (a fire-and-forget mutate)
so failures give no user feedback; change handleSave to call the async variant
(e.g., save.mutateAsync or the provided mutateAsync API) and wrap it in
try-catch (or use save.mutate with an onError callback) to surface errors to the
user (toast/alert/setError state) and prevent navigation on failure; update
references to buildSaveBody and save in the useCallback and ensure any loading
state is handled while awaiting the mutation.
- Around line 25-32: 현재 useEffect가 preOpinion?.review의 새 참조마다 reviewRef를 덮어써서
사용자가 편집한 로컬 변경이 refetch에 의해 덮어씌워질 가능성이 있습니다; 이를 고치려면 useEffect(및 관련 로직)에서 항상
덮어쓰지 않고 로컬 편집 여부를 확인해 초기 동기화만 수행하도록 변경하세요 — 예: reviewRef 또는 별도
플래그(hasLocalChanges/isPristine)를 활용해 reviewRef.current.isValid가 아직 설정되지 않았을 때(또는
로컬 변경이 없다면)만 reviewRef를 preOpinion.review의 rating과 keywordIds로 설정하고,
onSuccess/invalidateQueries 흐름과 함께 작동하도록 조정하세요 (참조 대상: useEffect, reviewRef,
preOpinion?.review, onSuccess, invalidateQueries).
In `@src/shared/assets/icon/FilledInfo.tsx`:
- Around line 23-24: Replace the hardcoded stroke and duplicated strokeWidth on
the <path> elements in the FilledInfo component: remove stroke="white" and
per-path strokeWidth="2", and instead expose/control the stroke color and width
on the root <svg> (or via a passed className/prop) so the icon inherits color
(use "currentColor" or a CSS variable like --icon-inner-color) and a single
strokeWidth at the SVG root; ensure the component accepts className/props so
callers can apply Tailwind classes (e.g., text-white) or theme CSS to control
the stroke color.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/features/pre-opinion/components/BookReviewSection.tsx (1)
31-35:infoMessage에 빈 문자열 대신undefined사용 권장
hasReview가false일 때''을 넘기면Container.Title이infoMessage !== undefined로 체크할 경우 빈 툴팁 아이콘이 렌더링될 수 있습니다.undefined를 전달하면 의도가 더 명확합니다.♻️ 제안
<Container.Title infoMessage={ hasReview ? '내 책장의 기록을 자동으로 불러왔어요. 여기서 수정하는 내용은 내 책장에도 똑같이 반영돼요!' - : '' + : undefined }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/components/BookReviewSection.tsx` around lines 31 - 35, The infoMessage prop in BookReviewSection.tsx currently passes an empty string when hasReview is false, which can trigger Container.Title's check (infoMessage !== undefined) and render an empty tooltip icon; change the ternary to return undefined instead of '' so that when hasReview is false no infoMessage prop is supplied (or explicitly set to undefined) — update the expression that sets infoMessage in the BookReviewSection component to return undefined for the false branch (reference the infoMessage prop and hasReview variable to locate the code).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/PreOpinions/PreOpinionWritePage.tsx`:
- Around line 117-121: handleSave currently fires save(body) without handling
errors so failures give no user feedback; change it to use the same
error-handling flow as handleSubmit by either calling the async saveAsync (or
awaiting save if it returns a promise) inside a try-catch and invoking openError
on failure, or pass an onError callback into useSavePreOpinion/save that
triggers openError; update the handleSave function (referencing handleSave,
buildSaveBody, save/saveAsync, useSavePreOpinion, openError) to ensure any save
rejection is caught and an error UI/notification is shown to the user.
---
Nitpick comments:
In `@src/features/pre-opinion/components/BookReviewSection.tsx`:
- Around line 31-35: The infoMessage prop in BookReviewSection.tsx currently
passes an empty string when hasReview is false, which can trigger
Container.Title's check (infoMessage !== undefined) and render an empty tooltip
icon; change the ternary to return undefined instead of '' so that when
hasReview is false no infoMessage prop is supplied (or explicitly set to
undefined) — update the expression that sets infoMessage in the
BookReviewSection component to return undefined for the false branch (reference
the infoMessage prop and hasReview variable to locate the code).
haruyam15
reviewed
Feb 19, 2026
src/features/pre-opinion/components/PreOpinionQuestionSection.tsx
Outdated
Show resolved
Hide resolved
… feat/pre-opinion-new-62
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
src/shared/constants/routes.ts (1)
27-29:PRE_OPINION_WRITE상수 추가 자체는 깔끔합니다.다만
PRE_OPINIONS(Line 38~39)가// Meetings섹션 안에 남아 있어서 그룹 일관성이 깨집니다. 두 pre-opinion 관련 상수를 같은// Pre-opinions섹션으로 묶으면 좋겠습니다.♻️ 수정 제안
// Pre-opinions PRE_OPINION_WRITE: (gatheringId: number | string, meetingId: number | string) => `/gatherings/${gatheringId}/meetings/${meetingId}/pre-opinions/new`, + PRE_OPINIONS: (gatheringId: number | string, meetingId: number | string) => + `/gatherings/${gatheringId}/meetings/${meetingId}/pre-opinions`, // Meetings MEETING_DETAIL: ... ... - PRE_OPINIONS: (gatheringId: number | string, meetingId: number | string) => - `/gatherings/${gatheringId}/meetings/${meetingId}/pre-opinions`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/constants/routes.ts` around lines 27 - 29, PRE_OPINION_WRITE was added under the "Pre-opinions" section but PRE_OPINIONS remains under the "// Meetings" section, breaking grouping consistency; move the PRE_OPINIONS constant (reference PRE_OPINIONS) out of the "// Meetings" block and place it alongside PRE_OPINION_WRITE under the "// Pre-opinions" comment so both pre-opinion routes are grouped together and ordered logically.src/features/pre-opinion/components/PreOpinionWriteHeader.tsx (1)
66-66:top-[123px]매직 넘버 → 상수화 권장64px 네비게이션 + 59px SubPageHeader 합산으로 추정되는 값인데, 둘 중 하나의 높이가 변경되면 이 값이 조용히 어긋납니다. CSS 변수나 Tailwind 커스텀 토큰으로 분리하면 일관성을 유지하기 쉽습니다.
♻️ 개선 예시
- 'sticky top-[123px] z-30 bg-white w-screen ml-[calc(-50vw+50%)] transition-shadow duration-200', + 'sticky top-[var(--header-offset)] z-30 bg-white w-screen ml-[calc(-50vw+50%)] transition-shadow duration-200',또는 공유 상수로 관리:
// src/shared/lib/layout.ts export const STICKY_OFFSET = 123 // 64px nav + 59px SubPageHeader- 'sticky top-[123px] z-30 ...' + `sticky top-[${STICKY_OFFSET}px] z-30 ...`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/components/PreOpinionWriteHeader.tsx` at line 66, The hardcoded "top-[123px]" magic number in PreOpinionWriteHeader.tsx should be extracted to a shared constant or CSS variable so it stays in sync with the nav/SubPageHeader heights; create a shared value (e.g., export const STICKY_OFFSET = 123 or '123px' in a new/existing module like shared/lib/layout.ts) or define a CSS variable (--sticky-offset) and then replace the literal "top-[123px]" in the component with a reference to that token (e.g., use Tailwind arbitrary value with the CSS var like "top-[var(--sticky-offset)]" and set the variable on a parent or inline style, or interpolate the shared constant into the component className if you're generating the class string), keeping the unique symbol PreOpinionWriteHeader and the original "top-[123px]" location to find and update the code.src/features/pre-opinion/preOpinion.types.ts (1)
93-128:review서브타입 중복 및SavePreOpinionParams필드 중복
SavePreOpinionBody(Line 95-98)와SubmitPreOpinionBody(Line 123-126)의review객체가 동일한 구조입니다. 평점/키워드 스펙이 바뀌면 두 곳을 동시에 수정해야 합니다.또한
SavePreOpinionParams(Line 111-113)의gatheringId/meetingId는GetPreOpinionParams와 중복됩니다.♻️ 리팩터 제안
+/** + * 사전 의견 리뷰(평가) 입력 타입 + */ +type PreOpinionReviewInput = { + rating: number + keywordIds: number[] +} export type SavePreOpinionBody = { - review: { - rating: number - keywordIds: number[] - } + review: PreOpinionReviewInput answers: { topicId: number content: string | null }[] } +export type SavePreOpinionParams = GetPreOpinionParams & { + /** 최초 저장 여부 (updatedAt이 null이면 true) */ + isFirstSave: boolean +} export type SubmitPreOpinionBody = { - review: { - rating: number - keywordIds: number[] - } + review: PreOpinionReviewInput topicIds: number[] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/preOpinion.types.ts` around lines 93 - 128, Extract the duplicated review object into a shared type (e.g., PreOpinionReview) and replace the inline review definitions in SavePreOpinionBody.review and SubmitPreOpinionBody.review with that new type; likewise, consolidate the duplicated gatheringId/meetingId into a shared params type (e.g., BasePreOpinionParams) and have SavePreOpinionParams reuse or extend that same type (or make GetPreOpinionParams extend BasePreOpinionParams) so both use the single source of truth for those fields; update the type names referenced above (PreOpinionReview, BasePreOpinionParams) wherever GetPreOpinionParams, SavePreOpinionParams, SavePreOpinionBody, and SubmitPreOpinionBody currently declare those fields.src/features/pre-opinion/components/TopicItem.tsx (1)
12-18: 렌더 중 setState는 필요하지만, 더 명확한 패턴으로 개선 권장부모에서 이미
key={topic.topicId}를 사용하고 있으므로 ID 변경 시 컴포넌트는 재마운트됩니다. 하지만topicId는 같고topic.content만 외부에서 변경되는 시나리오(초안 불러오기, 리페치 등)가 있다면 현재 패턴이 필요합니다.다만 렌더 중 setState는 추가 렌더 사이클을 발생시킵니다.
useEffect를 사용하면 더 예측 가능한 흐름을 만들 수 있습니다 (약간의 렌더 lag는 있지만, 대부분의 경우 체감상 무시할 수 있습니다).useEffect(() => { setValue(topic.content ?? '') setPrevContent(topic.content) }, [topic.content])외부
topic.content변경이 없다면 이 로직 자체를 제거할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/components/TopicItem.tsx` around lines 12 - 18, The component currently calls setPrevContent and setValue during render when topic.content differs, causing extra renders; move this synchronization into a useEffect that runs when topic.content changes: inside an effect with dependency [topic.content] call setValue(topic.content ?? '') and setPrevContent(topic.content) (or remove prevContent state entirely and rely on the effect if you only need to respond to changes). Update imports to include useEffect and remove the inline conditional setState in the render body (keep references to value, setValue, prevContent, setPrevContent, and topic.content to locate the code).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/pre-opinion/components/PreOpinionWriteHeader.tsx`:
- Line 16: The prop isReviewValid on the PreOpinionWriteHeader component is
optional but currently treated with a negation (!isReviewValid) which treats
undefined as false and unintentionally disables buttons when the parent omits
the prop; update the component to declare an explicit default (e.g.,
isReviewValid = false) in the props destructuring or provide a sane default via
defaultProps, and update any checks around the Save/Share button logic (the
usage around lines referenced 87-91) to rely on that default or use an explicit
comparison like isReviewValid === true so the intent is clear and parents
omitting the prop won’t accidentally disable the UI.
- Around line 12-13: The buttons become clickable when isReviewValid is true
even if onSave/onSubmit are omitted, so make the intent explicit by providing
safe no-op fallbacks for the handlers: in PreOpinionWriteHeader, either make
onSave/onSubmit required in the props type or (preferably) keep them optional
and pass a default no-op to the Button onClick (e.g., use onClick={onSave ?? (()
=> {})} and onClick={onSubmit ?? (() => {})}) wherever the save/submit buttons
are rendered (also update the same pattern around the block referenced at lines
82-93) so clicks do not silently do nothing.
In `@src/features/pre-opinion/hooks/useSubmitPreOpinion.ts`:
- Around line 34-37: Change the onSuccess handler in useSubmitPreOpinion to be
async and await the queryClient.invalidateQueries calls so the mutation waits
for cache invalidation; specifically, update the onSuccess used inside
useSubmitPreOpinion to await queryClient.invalidateQueries({ queryKey:
preOpinionQueryKeys.all }) and await the more specific invalidate for topics
(replace topicQueryKeys.confirmed() with the targeted key
topicQueryKeys.confirmedList({ gatheringId, meetingId, pageSize }) or, if broad
invalidation is intentional, add a clear comment explaining why the broader
topicQueryKeys.confirmed() is used).
In `@src/routes/index.tsx`:
- Around line 135-138: Replace the route path helper with an inline path string:
instead of using ROUTES.PRE_OPINION_WRITE(':gatheringId', ':meetingId') in the
route definition that renders <PreOpinionWritePage />, define the path as the
literal route pattern (use the same pattern style as the neighboring routes,
e.g. include :gatheringId and :meetingId segments in the string) so the route
pattern is stable even if ROUTES helpers change.
- Line 26: Add PreOpinionWritePage to the pages barrel export and switch the
route import to use that barrel: in the PreOpinions module index, export the
PreOpinionWritePage component (named export from the PreOpinions folder), then
in the routes file replace the direct path import of PreOpinionWritePage with an
import from the pages barrel (i.e., import { PreOpinionWritePage } from
'@/pages'). This preserves consistent import style across pages.
---
Nitpick comments:
In `@src/features/pre-opinion/components/PreOpinionWriteHeader.tsx`:
- Line 66: The hardcoded "top-[123px]" magic number in PreOpinionWriteHeader.tsx
should be extracted to a shared constant or CSS variable so it stays in sync
with the nav/SubPageHeader heights; create a shared value (e.g., export const
STICKY_OFFSET = 123 or '123px' in a new/existing module like
shared/lib/layout.ts) or define a CSS variable (--sticky-offset) and then
replace the literal "top-[123px]" in the component with a reference to that
token (e.g., use Tailwind arbitrary value with the CSS var like
"top-[var(--sticky-offset)]" and set the variable on a parent or inline style,
or interpolate the shared constant into the component className if you're
generating the class string), keeping the unique symbol PreOpinionWriteHeader
and the original "top-[123px]" location to find and update the code.
In `@src/features/pre-opinion/components/TopicItem.tsx`:
- Around line 12-18: The component currently calls setPrevContent and setValue
during render when topic.content differs, causing extra renders; move this
synchronization into a useEffect that runs when topic.content changes: inside an
effect with dependency [topic.content] call setValue(topic.content ?? '') and
setPrevContent(topic.content) (or remove prevContent state entirely and rely on
the effect if you only need to respond to changes). Update imports to include
useEffect and remove the inline conditional setState in the render body (keep
references to value, setValue, prevContent, setPrevContent, and topic.content to
locate the code).
In `@src/features/pre-opinion/preOpinion.types.ts`:
- Around line 93-128: Extract the duplicated review object into a shared type
(e.g., PreOpinionReview) and replace the inline review definitions in
SavePreOpinionBody.review and SubmitPreOpinionBody.review with that new type;
likewise, consolidate the duplicated gatheringId/meetingId into a shared params
type (e.g., BasePreOpinionParams) and have SavePreOpinionParams reuse or extend
that same type (or make GetPreOpinionParams extend BasePreOpinionParams) so both
use the single source of truth for those fields; update the type names
referenced above (PreOpinionReview, BasePreOpinionParams) wherever
GetPreOpinionParams, SavePreOpinionParams, SavePreOpinionBody, and
SubmitPreOpinionBody currently declare those fields.
In `@src/shared/constants/routes.ts`:
- Around line 27-29: PRE_OPINION_WRITE was added under the "Pre-opinions"
section but PRE_OPINIONS remains under the "// Meetings" section, breaking
grouping consistency; move the PRE_OPINIONS constant (reference PRE_OPINIONS)
out of the "// Meetings" block and place it alongside PRE_OPINION_WRITE under
the "// Pre-opinions" comment so both pre-opinion routes are grouped together
and ordered logically.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/features/pre-opinion/components/PreOpinionWriteHeader.tsx (1)
52-57: IntersectionObserverrootMargin누락으로 그림자 타이밍 미스매치sentinel이 뷰포트 상단(0px)을 벗어날 때
isStuck=true가 되지만, 헤더는top-[123px]에서 이미 고정됩니다. 두 시점 사이에 헤더는 sticky이지만 그림자는 없습니다.
rootMargin: '-123px 0px 0px 0px'을 추가하면 sticky 시작점과 그림자 시작점이 일치합니다.top-[123px]값이 바뀌면 rootMargin도 같이 수정해야 하므로 상수로 관리하면 좋습니다.♻️ rootMargin 동기화 제안
+ const STICKY_OFFSET = 123 // GNB(~64px) + SubPageHeader(59px) const observer = new IntersectionObserver( ([entry]) => { setIsStuck(!entry.isIntersecting) }, - { threshold: 0 } + { threshold: 0, rootMargin: `-${STICKY_OFFSET}px 0px 0px 0px` } )그리고 sticky top도 같은 상수로:
- 'sticky top-[123px] z-30 ...' + // className 내 top 값도 STICKY_OFFSET과 동기화 유지Also applies to: 68-70
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/pre-opinion/components/PreOpinionWriteHeader.tsx` around lines 52 - 57, The IntersectionObserver currently watches sentinel with threshold:0 causing the shadow toggle (setIsStuck in the observer callback) to be out of sync with the sticky header which uses top-[123px]; update the observer configuration to include a rootMargin that matches the header offset (e.g. rootMargin: `-123px 0px 0px 0px`) and factor the offset into a shared constant used both for the header's top style and the observer rootMargin (update usages around the observer creation in PreOpinionWriteHeader and the sticky header top value) so changes to the header offset remain synchronized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/features/pre-opinion/components/PreOpinionWriteHeader.tsx`:
- Around line 9-17: The same prop changes (making onSave and onSubmit required
and defaulting isReviewValid to false) were applied to the
PreOpinionWriteHeaderProps interface but duplicated elsewhere; locate the other
props/interface/usage for the PreOpinionWriteHeader component (the duplicate
around the second declaration/usage) and update it to match: require onSave and
onSubmit (remove optional ?), and ensure the component's props destructuring or
default props set isReviewValid = false so the default is consistent.
In `@src/routes/index.tsx`:
- Line 23: There are duplicate review/comment blocks that assert LGTM for
PreOpinionWritePage; remove the redundant comment so only one approval remains.
Locate the import/usage of PreOpinionWritePage (the import from the pages barrel
and the route definitions where PreOpinionWritePage is mounted) and delete the
duplicate review text that appears a second time (the duplicate approval
referencing the same checks), leaving a single, clear comment. Ensure no other
functional code is changed—only remove the repeated comment block to avoid
duplicate PR chatter.
---
Nitpick comments:
In `@src/features/pre-opinion/components/PreOpinionWriteHeader.tsx`:
- Around line 52-57: The IntersectionObserver currently watches sentinel with
threshold:0 causing the shadow toggle (setIsStuck in the observer callback) to
be out of sync with the sticky header which uses top-[123px]; update the
observer configuration to include a rootMargin that matches the header offset
(e.g. rootMargin: `-123px 0px 0px 0px`) and factor the offset into a shared
constant used both for the header's top style and the observer rootMargin
(update usages around the observer creation in PreOpinionWriteHeader and the
sticky header top value) so changes to the header offset remain synchronized.
haruyam15
approved these changes
Feb 19, 2026
mgYang53
approved these changes
Feb 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚀 풀 리퀘스트 제안
📋 작업 내용
사전 의견 작성 페이지의 전체 기능을 구현했습니다. 변경된 백엔드 API 스펙에 맞춰 조회 API를 반영하고, 주제별 의견 작성/책 평가/저장/공유 기능을 연결했습니다.
🔧 변경 사항
API 및 타입
Hooks
컴포넌트
기타
📸 스크린샷 (선택 사항)
📄 기타
추가적으로 전달하고 싶은 내용이나 특별한 요구 사항이 있으면 작성해 주세요.
Summary by CodeRabbit
릴리스 노트
새로운 기능
UI 개선