Feat(client): QA 반영 리마인드 옵션버튼 작동 문제 해결#138
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (2)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Walkthrough리마인드 페이지에서 옵션 메뉴 오픈 시 전달하는 식별자를 카테고리 ID에서 아티클 ID로 변경하여, 메뉴 앵커 대상이 아티클 기준으로 바뀌었습니다. 공개/내보내기 시그니처 변경은 없습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant R as RemindPage
participant M as OptionsMenu
U->>R: 클릭 (옵션 버튼)
Note right of R: 변경 전: openMenu(categoryId)<br/>변경 후: openMenu(articleId)
R->>M: openMenu(articleId)
M-->>U: 아티클에 앵커된 옵션 메뉴 표시
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
|
✅ Storybook chromatic 배포 확인: |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/client/src/pages/remind/Remind.tsx (2)
54-63: 의도치 않은 window.close() 호출 가능성 — 즉시 제거 필요
closeMenu()다음의close()는 전역window.close()실행으로 이어질 수 있어 창 종료/보안 경고를 유발합니다.다음과 같이 제거해 주세요.
setIsDeleteOpen(false); setDeleteTargetId(null); closeMenu(); - close();
115-121: reverse‑tabnabbing 방지: window.open 보안 플래그 추가
_blank로 새 탭을 열 때noopener,noreferrer미지정 시 보안 리스크가 있습니다.아래처럼 보완해 주세요.
- window.open(article.url, '_blank'); + const newTab = window.open( + article.url, + '_blank', + 'noopener,noreferrer' + ); + if (newTab) newTab.opener = null;
🧹 Nitpick comments (2)
apps/client/src/pages/remind/Remind.tsx (2)
147-149: Props 명과 의미 불일치(categoryId → articleId) 정리 제안현재
categoryId={menu.categoryId}/getCategoryName를 유지하면서 상류에서는 아티클 id를 전달하고 있어 명세 혼동 여지가 큽니다. 컴포넌트/훅 전반을targetId또는articleId로 리네이밍하는 정리가 좋습니다. (동작 영향 없으면 후속 PR 권장)예시(이 파일 관점의 사용처만):
- categoryId={menu.categoryId} - getCategoryName={getItemTitle} + articleId={menu.categoryId /* upstream에서 필드/prop 리네이밍 필요 */} + getArticleTitle={getItemTitle}
49-51: 옵션 라벨이 MOCK 데이터에 의존
REMIND_MOCK_DATA로 제목을 찾으면 실제 API 데이터와 불일치할 수 있습니다. 실데이터(data.articles)로 치환을 권장합니다.다음처럼 대체 가능합니다.
- const getItemTitle = (id: number | null) => - id == null ? '' : (REMIND_MOCK_DATA.find((d) => d.id === id)?.title ?? ''); + const getItemTitle = (id: number | null) => { + if (id == null) return ''; + const found = data?.articles?.find((a) => a.articleId === id); + return found?.memo ?? found?.url ?? ''; + };
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/design-system/src/icons/source/tooltip_1.svgis excluded by!**/*.svgpackages/design-system/src/icons/source/tooltip_2.svgis excluded by!**/*.svg
📒 Files selected for processing (1)
apps/client/src/pages/remind/Remind.tsx(1 hunks)
🔇 Additional comments (1)
apps/client/src/pages/remind/Remind.tsx (1)
58-58: 'arcons' queryKey 일치 확인 — 수정 불필요apps/client/src/shared/apis/queries.ts(59)에서 useGetArcons가 queryKey: ['arcons']로 정의되어 있으며, apps/client/src/pages/remind/Remind.tsx(58,124) 및 apps/client/src/pages/myBookmark/MyBookmark.tsx(73,170)에서 동일 키로 invalidateQueries를 호출하므로 무효화가 정상 동작합니다.
| onOptionsClick={(e) => { | ||
| e.stopPropagation(); | ||
| openMenu(article.category.categoryId, e.currentTarget); | ||
| openMenu(article.articleId, e.currentTarget); | ||
| }} |
There was a problem hiding this comment.
🧩 Analysis chain
openMenu에 articleId 전달 변경 OK
옵션 버튼 앵커를 카드(아티클) 기준으로 맞춘 변경은 문제 원인을 정확히 짚었습니다. 다만, 하위에서 여전히 categoryId 명칭을 사용하고 있어(예: menu.categoryId, OptionsMenuPortal props) 의미 혼동 가능성이 있습니다. id 의미(카테고리 vs 아티클)를 일치시키는지 확인 부탁드립니다.
다음 스크립트로 훅/포털의 시그니처와 prop 명을 확인해 주세요.
🏁 Script executed:
#!/bin/bash
# useAnchoredMenu 정의 및 state/open 시그니처 확인
fd -t f --hidden --exclude node_modules 'useAnchoredMenu.*' | xargs -I{} rg -nC3 -H 'useAnchoredMenu|open\s*\(|state\s*:'
# OptionsMenuPortal props에서 categoryId/getCategoryName 의미 확인
fd -t f --hidden --exclude node_modules 'OptionsMenuPortal.*' | xargs -I{} rg -nC3 -H 'Props|interface|type|categoryId|getCategoryName|onEdit|onDelete'
# menu.categoryId 소비 지점 확인
rg -nC2 'menu\.categoryId|categoryId=' --type=tsx -g '!**/node_modules/**'Length of output: 292588
🏁 Script executed:
#!/bin/bash
# useAnchoredMenu 훅의 정의와 시그니처 확인
rg -n "useAnchoredMenu|categoryId" apps/client/src/shared/hooks/useAnchoredMenu.ts -A10 -B2
# OptionsMenuPortal 컴포넌트에서 categoryId prop 확인
rg -n "categoryId|getCategoryName" apps/client/src/shared/components/sidebar/OptionsMenuPortal.tsx -A3 -B3Length of output: 1747
중요: openMenu에 articleId가 전달되어 categoryId 의미와 불일치 — 수정 필요
useAnchoredMenu.open(categoryId: number, anchorEl)와 OptionsMenuPortal(props.categoryId, getCategoryName)는 'categoryId(카테고리 id)'를 기대합니다. Remind.tsx의 onOptionsClick에서 openMenu(article.articleId, ...)를 호출해 articleId가 state.categoryId로 설정되어 메뉴에 잘못된 이름/동작을 초래합니다.
- 조치: Remind.tsx에서 openMenu에 article의 category id (예: article.categoryId 또는 article.category.id)를 전달하거나, 의도적으로 articleId를 전달하려면 useAnchoredMenu/OptionsMenuPortal의 prop명과 state명을 articleId로 전역적으로 변경해 사용처를 일치시키세요.
수정 포인터: apps/client/src/pages/remind/Remind.tsx (onOptionsClick), apps/client/src/shared/hooks/useAnchoredMenu.ts (state.categoryId / open 시그니처), apps/client/src/shared/components/sidebar/OptionsMenuPortal.tsx (props categoryId, getCategoryName).
🤖 Prompt for AI Agents
In apps/client/src/pages/remind/Remind.tsx around lines 132-135, the
onOptionsClick currently calls openMenu(article.articleId, ...) which passes an
articleId where the anchored menu expects a categoryId; update onOptionsClick to
pass the article's category id (e.g., article.categoryId or article.category.id)
to openMenu so state.categoryId is set correctly, or if the intended key is
articleId, consistently rename state and props across useAnchoredMenu
(state/categoryId and open signature) and OptionsMenuPortal (props.categoryId
and getCategoryName usages) to reflect articleId — apply the smallest scoped
fix: change the argument in Remind.tsx to the category id and verify callers and
menu label resolution still work.
📌 Related Issues
📄 Tasks
⭐ PR Point (To Reviewer)
📷 Screenshot
Summary by CodeRabbit