Skip to content

Feat(client): QA 반영 리마인드 옵션버튼 작동 문제 해결#138

Merged
jjangminii merged 6 commits intodevelopfrom
fix/#137/remind-card-option-btn
Sep 20, 2025
Merged

Feat(client): QA 반영 리마인드 옵션버튼 작동 문제 해결#138
jjangminii merged 6 commits intodevelopfrom
fix/#137/remind-card-option-btn

Conversation

@jjangminii
Copy link
Collaborator

@jjangminii jjangminii commented Sep 20, 2025

📌 Related Issues

관련된 Issue를 태그해주세요. (e.g. - close #25)

📄 Tasks

  • 이미지 재저장
  • 리마인드 삭제 id수정
  • 리마인드 수정 모달

⭐ PR Point (To Reviewer)

📷 Screenshot

Summary by CodeRabbit

  • 버그 수정
    • 알림 페이지에서 기사 목록의 옵션 메뉴가 잘못된 항목에 연결되던 문제를 수정했습니다. 이제 옵션 버튼을 누르면 해당 기사에 정확히 앵커링된 메뉴가 열립니다.
    • 메뉴에서의 편집/삭제 등 선택이 의도한 기사에만 안정적으로 적용되어 오작동으로 인한 혼동이 줄었습니다.
    • 여러 항목을 연속 관리할 때도 일관된 동작으로 올바른 항목에만 영향이 반영됩니다.

@jjangminii jjangminii linked an issue Sep 20, 2025 that may be closed by this pull request
@vercel
Copy link

vercel bot commented Sep 20, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
pinback-client-client Ready Ready Preview Comment Sep 20, 2025 3:36pm
pinback-client-landing Ready Ready Preview Comment Sep 20, 2025 3:36pm

@coderabbitai
Copy link

coderabbitai bot commented Sep 20, 2025

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (2)
  • packages/design-system/src/icons/source/tooltip_4.svg is excluded by !**/*.svg
  • packages/design-system/src/icons/source/tooltip_5.svg is excluded by !**/*.svg

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

리마인드 페이지에서 옵션 메뉴 오픈 시 전달하는 식별자를 카테고리 ID에서 아티클 ID로 변경하여, 메뉴 앵커 대상이 아티클 기준으로 바뀌었습니다. 공개/내보내기 시그니처 변경은 없습니다.

Changes

Cohort / File(s) Summary of changes
Remind 옵션 메뉴 대상 ID 변경
apps/client/src/pages/remind/Remind.tsx
onOptionsClick에서 openMenu 인자를 article.category.categoryIdarticle.articleId로 교체

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: 아티클에 앵커된 옵션 메뉴 표시
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested reviewers

  • jllee000
  • constantly-dev

Poem

토끼는 톡, 메뉴를 톡—
카테고리 대신 아티클로 쏙!
버튼을 누르면 길이 바로 열리고,
수정·삭제도 이제 또록또록.
풋, 버그는 훌쩍 뛰어넘었쥐! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Linked Issues Check ⚠️ Warning PR은 이슈 #137에서 요구한 리마인드 옵션 버튼 작동 문제 수정 내용을 정확히 반영하지만, 이슈 #25(progress bar 구현) 관련 요구사항은 전혀 포함되지 않아 연결된 이슈 요구사항을 완전히 충족하지 않습니다. 관련 이슈 태그에서 실제 구현 내용이 없는 #25를 제거하거나 해당 이슈의 작업을 함께 구현하여 모든 연결된 이슈 요구사항을 충족하도록 수정해야 합니다.
✅ Passed checks (4 passed)
Check name Status Explanation
Title Check ✅ Passed PR 제목은 “리마인드 옵션 버튼 작동 문제 해결”이라는 주요 변경 사항을 구체적으로 요약하고 있어 변경의 핵심을 잘 전달하며, ‘Feat(client):’ 접두사로 모듈 범위도 명확하게 나타냅니다.
Out of Scope Changes Check ✅ Passed 변경된 코드는 리마인드 옵션 버튼 작동 문제 해결을 위한 부분만을 수정하고 있어 연결된 이슈 #137의 범위를 벗어나는 코드 변경은 포함되어 있지 않습니다.
Description Check ✅ Passed PR 설명은 관련 이슈와 작업 내역 섹션을 포함하여 주요 정보를 잘 정리했으며, 필수 템플릿 구조를 충족합니다. 비록 PR Point와 Screenshot 섹션이 비어 있지만 이는 선택적이므로 설명이 전체적으로 완전합니다.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@jjangminii jjangminii changed the title Fix/#137/remind card option btn Feat(client): QA 반영 리마인드 옵션버튼 작동 문제 해결 Sep 20, 2025
@github-actions
Copy link

github-actions bot commented Sep 20, 2025

✅ Storybook chromatic 배포 확인:
🐿️ storybook

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 4031b94 and 0affa69.

⛔ Files ignored due to path filters (2)
  • packages/design-system/src/icons/source/tooltip_1.svg is excluded by !**/*.svg
  • packages/design-system/src/icons/source/tooltip_2.svg is 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를 호출하므로 무효화가 정상 동작합니다.

Comment on lines 132 to 135
onOptionsClick={(e) => {
e.stopPropagation();
openMenu(article.category.categoryId, e.currentTarget);
openMenu(article.articleId, e.currentTarget);
}}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

🧩 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 -B3

Length 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix 버그 수정하라 러브버그

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Fix] sp1 QA 리마인드 옵션 버튼 작동 문제

1 participant