Merged
Conversation
|
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:
Walkthrough그룹 선택 UI를 컴포넌트 내부로 이동해 포털 기반으로 렌더링(SSR 안전 포함)하도록 변경했고, 그룹 색상↔아이콘 매핑 유틸을 적용했습니다. 레스토랑 북마크 상태 조회 API가 추가됐고, 상세·스토어 카드에 그룹 선택 UI와 공유/복사 모달, 평점 포맷(소수 1자리) 동기화 로직이 도입되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant StoreCard
participant GroupSelector
participant BookmarkAPI
participant GroupList
User->>StoreCard: 저장 버튼 클릭 / 공유 버튼 클릭
StoreCard->>GroupSelector: 열기 (isGroupSelectorOpen = true) (포털로 렌더)
GroupSelector->>BookmarkAPI: getRestaurantBookmarkStatus(restaurantId)
BookmarkAPI-->>GroupSelector: 그룹 상태 응답
GroupSelector->>GroupList: 그룹 목록 렌더 (아이콘 = mapColorToIcon)
User->>GroupList: 그룹 선택
GroupList-->>StoreCard: 선택 결과 전달 (isKeep 업데이트)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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: 2
🧹 Nitpick comments (2)
src/shared/components/storecard/StoreCardInSave.tsx (1)
35-35: LGTM —Number()래핑은 생략 가능
rating이미number타입으로 선언되어 있으므로Number(rating)없이rating.toFixed(1)로 축약할 수 있습니다. PR 전반(DetailHeader, StoreCardInMultiPinList)에서도 동일한 패턴이 사용되므로, 공통 유틸 함수로 분리하면 이후 형식 변경 시 단일 지점에서 관리됩니다.♻️ 제안: 공통 유틸 함수 추출
// src/shared/utils/formatRating.ts (신규) +export const formatRating = (rating: number): string => rating.toFixed(1);- const displayRating = Number(rating).toFixed(1); + const displayRating = formatRating(rating);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/storecard/StoreCardInSave.tsx` at line 35, The line creating displayRating uses Number(rating).toFixed(1) even though rating is already typed as number; change instances (in StoreCardInSave, DetailHeader, StoreCardInMultiPinList) to use rating.toFixed(1) and extract a shared helper (e.g., formatRating(value: number): string) into a common utils module, then replace the inline expressions with calls to formatRating to centralize formatting logic.src/shared/components/home/GroupSelector.tsx (1)
51-62: 리사이즈 핸들러에서topSnapVh스테일 클로저 가능성의존성 배열이
[]이므로calculateSnapPx는 마운트 시점의topSnapVh를 캡처합니다.topSnapVh프롭이 변경된 후 창 크기를 조절하면TOP스냅 포인트가 구 값으로 계산됩니다. 현재 사용처에서는 고정값(17.8)이 전달되므로 실질적 영향은 없지만, 방어적으로 의존성을 추가하는 편이 안전합니다.♻️ 제안: 의존성 배열에 topSnapVh 추가
useEffect(() => { const calculateSnapPx = () => { setSnapPx({ TOP: vhToPx(topSnapVh), MID: vhToPx(SNAP_POINTS.MID), MIN: vhToPx(SNAP_POINTS.MIN), HIDDEN: vhToPx(SNAP_POINTS.HIDDEN), }); }; window.addEventListener('resize', calculateSnapPx); return () => window.removeEventListener('resize', calculateSnapPx); - }, []); + }, [topSnapVh]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/home/GroupSelector.tsx` around lines 51 - 62, The resize handler calculateSnapPx inside the useEffect captures topSnapVh from mount because the dependency array is empty; update the effect so it depends on topSnapVh (add topSnapVh to the deps) so the handler uses the latest value, ensure calculateSnapPx is invoked immediately inside the effect to set the initial snap values, and keep the existing cleanup (removeEventListener) so the listener is re-registered with the new function when topSnapVh changes; reference calculateSnapPx, setSnapPx, topSnapVh, vhToPx and SNAP_POINTS.
🤖 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/shared/components/storecard/StoreCardInMultiPinList.tsx`:
- Around line 56-59: The handleShareClick handler in StoreCardInMultiPinList
currently only logs and leaves the share action unimplemented; either implement
the real sharing behavior or remove/disable the handler and UI affordance:
locate handleShareClick and the share button in StoreCardInMultiPinList, and
replace the console.log with the actual share flow (e.g., call the app’s share
util, open a share modal, or invoke navigator.share with the store URL and
data), or if sharing isn’t ready remove the onClick and aria-label from the
button and set the button's disabled state (and tooltip) so users can’t interact
with it.
- Around line 43-54: handleBookmarkClick currently only logs when isKeep is true
and does not call the unbookmark API or update state/parent; implement an
unbookmark flow in handleBookmarkClick: call the appropriate API to remove the
scrap (use existing API util used elsewhere), optimistically update local state
so the bookmark UI toggles (e.g., set a local isBookmarked state or call a
provided setter), handle API errors (revert optimistic update and show error),
and ensure you notify the parent MultipinList to refetch or update the scrap
prop (add/emit a callback prop like onScrapChange or invoke an existing parent
refresh method). Also ensure GroupList.handleSave triggers the same parent
refresh callback after success so scrap prop is updated consistently. Use
function names/props handleBookmarkClick, isKeep, setIsGroupSelectorOpen,
GroupList.handleSave, MultipinList and scrap to locate where to add the API
call, state update, and parent notification.
---
Nitpick comments:
In `@src/shared/components/home/GroupSelector.tsx`:
- Around line 51-62: The resize handler calculateSnapPx inside the useEffect
captures topSnapVh from mount because the dependency array is empty; update the
effect so it depends on topSnapVh (add topSnapVh to the deps) so the handler
uses the latest value, ensure calculateSnapPx is invoked immediately inside the
effect to set the initial snap values, and keep the existing cleanup
(removeEventListener) so the listener is re-registered with the new function
when topSnapVh changes; reference calculateSnapPx, setSnapPx, topSnapVh, vhToPx
and SNAP_POINTS.
In `@src/shared/components/storecard/StoreCardInSave.tsx`:
- Line 35: The line creating displayRating uses Number(rating).toFixed(1) even
though rating is already typed as number; change instances (in StoreCardInSave,
DetailHeader, StoreCardInMultiPinList) to use rating.toFixed(1) and extract a
shared helper (e.g., formatRating(value: number): string) into a common utils
module, then replace the inline expressions with calls to formatRating to
centralize formatting logic.
Comment on lines
43
to
54
| const handleBookmarkClick = (e: React.MouseEvent) => { | ||
| e.stopPropagation(); // 카드 클릭(상세이동) 이벤트 방지 | ||
|
|
||
| if (isKeep) { | ||
| // 이미 저장된 경우: 보통은 여기서 바로 해제 API를 쏘거나 확인 모달을 띄움 | ||
| console.log('이미 저장됨: 해제 로직 필요'); | ||
| // 예: if(confirm('저장을 취소하시겠습니까?')) { ... API 호출 ... } | ||
| } else { | ||
| // 저장 안 된 경우: 그룹 선택 바텀시트 열기 | ||
| setIsGroupSelectorOpen(true); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine StoreCardInMultiPinList.tsx
find . -type f -name "StoreCardInMultiPinList.tsx" | head -5Repository: CorkCharge/CorkCharge-FE
Length of output: 129
🏁 Script executed:
# Also check the general structure
git ls-files | grep -i "storecard" | head -20Repository: CorkCharge/CorkCharge-FE
Length of output: 427
🏁 Script executed:
cat -n ./src/shared/components/storecard/StoreCardInMultiPinList.tsxRepository: CorkCharge/CorkCharge-FE
Length of output: 6869
🏁 Script executed:
# Find and examine GroupList component
find . -type f -name "GroupList.tsx" | head -5Repository: CorkCharge/CorkCharge-FE
Length of output: 110
🏁 Script executed:
# Search for editBookmarkGroup or similar API calls
rg -n "editBookmarkGroup|removeBookmark|deleteBookmark" --type=tsx -B 2 -A 2 | head -50Repository: CorkCharge/CorkCharge-FE
Length of output: 95
🏁 Script executed:
# Find parent components that use StoreCardInMultiPinList
rg -n "StoreCardInMultiPinList" --type=tsx -B 5 -A 5 | head -100Repository: CorkCharge/CorkCharge-FE
Length of output: 95
🏁 Script executed:
# Search for editBookmarkGroup without type filter
rg -n "editBookmarkGroup|removeBookmark|deleteBookmark" -B 2 -A 2 | head -80Repository: CorkCharge/CorkCharge-FE
Length of output: 6020
🏁 Script executed:
# Search for StoreCardInMultiPinList usage
rg -n "StoreCardInMultiPinList" -B 3 -A 3 | head -80Repository: CorkCharge/CorkCharge-FE
Length of output: 1421
🏁 Script executed:
# Examine GroupList.tsx
cat -n ./src/shared/components/home/GroupList.tsx | head -100Repository: CorkCharge/CorkCharge-FE
Length of output: 3642
🏁 Script executed:
# Continue reading GroupList.tsx from line 100
sed -n '100,150p' ./src/shared/components/home/GroupList.tsxRepository: CorkCharge/CorkCharge-FE
Length of output: 1835
🏁 Script executed:
# Also check the parent component MultipinList to see if it refetches data
cat -n ./src/pages/corkagemap/multipinlist/MultipinList.tsx | head -150Repository: CorkCharge/CorkCharge-FE
Length of output: 6583
🏁 Script executed:
# Check if StoreCardInMultiPinList uses Zustand store
rg -n "useBookmarkStore\|selectedStores" ./src/shared/components/storecard/StoreCardInMultiPinList.tsxRepository: CorkCharge/CorkCharge-FE
Length of output: 50
🏁 Script executed:
# Verify if Zustand store has any effect on the component
rg -n "useBookmarkStore" ./src/shared/components/storecard/ -A 3 | head -50Repository: CorkCharge/CorkCharge-FE
Length of output: 50
🏁 Script executed:
# Check the bookmark API to confirm editBookmarkGroup can be used for removal
cat -n ./src/shared/apis/bookmark/bookmark.api.tsRepository: CorkCharge/CorkCharge-FE
Length of output: 3226
🏁 Script executed:
# Final verification: Check if there's any post-save refetch callback in GroupSelector/GroupList
rg -n "onSave\|onClose\|callback" ./src/shared/components/home/GroupList.tsx | head -20Repository: CorkCharge/CorkCharge-FE
Length of output: 50
🏁 Script executed:
# Check if MultipinList or parent has any mechanism to refetch after save
rg -n "GroupSelector\|useBookmarkStore" ./src/pages/corkagemap/multipinlist/MultipinList.tsxRepository: CorkCharge/CorkCharge-FE
Length of output: 50
이미 저장된 매장의 북마크 해제 로직이 구현되지 않음
isKeep === true 브랜치에서 console.log만 출력하고 아무 동작도 수행하지 않습니다. 사용자가 이미 저장된 매장의 북마크 아이콘을 누르면 UI 피드백도 없고 저장 해제도 되지 않습니다.
또한 GroupList.handleSave 성공 후 isKeep은 scrap 프롭이 부모에서 다시 전달될 때까지 false 상태로 남아 있어, 저장 완료 후에도 북마크 아이콘이 업데이트되지 않을 수 있습니다. 부모 컴포넌트(MultipinList)가 저장 성공 시 데이터를 재조회하여 scrap 프롭을 갱신하는 메커니즘이 없습니다.
🐛 해제 로직 구현 예시
const handleBookmarkClick = (e: React.MouseEvent) => {
e.stopPropagation();
if (isKeep) {
- // 이미 저장된 경우: 보통은 여기서 바로 해제 API를 쏘거나 확인 모달을 띄움
- console.log('이미 저장됨: 해제 로직 필요');
- // 예: if(confirm('저장을 취소하시겠습니까?')) { ... API 호출 ... }
+ editBookmarkGroup({ restaurantId: resId, groupIds: [] })
+ .then(() => setIsKeep(false))
+ .catch((err) => console.error('북마크 해제 실패', err));
} else {
setIsGroupSelectorOpen(true);
}
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/components/storecard/StoreCardInMultiPinList.tsx` around lines 43
- 54, handleBookmarkClick currently only logs when isKeep is true and does not
call the unbookmark API or update state/parent; implement an unbookmark flow in
handleBookmarkClick: call the appropriate API to remove the scrap (use existing
API util used elsewhere), optimistically update local state so the bookmark UI
toggles (e.g., set a local isBookmarked state or call a provided setter), handle
API errors (revert optimistic update and show error), and ensure you notify the
parent MultipinList to refetch or update the scrap prop (add/emit a callback
prop like onScrapChange or invoke an existing parent refresh method). Also
ensure GroupList.handleSave triggers the same parent refresh callback after
success so scrap prop is updated consistently. Use function names/props
handleBookmarkClick, isKeep, setIsGroupSelectorOpen, GroupList.handleSave,
MultipinList and scrap to locate where to add the API call, state update, and
parent notification.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/shared/components/restaurant_detail/DetailHeader.tsx (1)
193-195:⚠️ Potential issue | 🟡 Minor
map내<p>요소에keyprop 누락
corkageOption.map에서 렌더링되는<p>요소에key가 없어 React 경고가 발생하며, 리스트 업데이트 시 비효율적인 리렌더링이 발생할 수 있습니다.🐛 수정 제안
- {corkageOption.map((option) => ( - <p>{option}</p> + {corkageOption.map((option, index) => ( + <p key={index}>{option}</p> ))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/restaurant_detail/DetailHeader.tsx` around lines 193 - 195, The mapped <p> elements in corkageOption.map are missing a key prop which causes React warnings; update the mapping inside DetailHeader's render (the corkageOption.map callback) to supply a stable key for each <p> — preferably key={option} if option values are unique, otherwise use key={index} as a fallback (use the map's index parameter) to ensure each <p> has a key.src/pages/corkagemap/mystore/MyStore.tsx (1)
64-64:⚠️ Potential issue | 🟡 Minor정렬 파라미터와 UI 텍스트 불일치
API 호출(Line 64)에서는
'LATEST'(최신순)로 정렬하지만, 드롭다운 UI(Line 96)에는"리뷰많은순"으로 표시되어 있습니다. 또한 드롭다운에 클릭 핸들러가 없어 정렬 변경 기능이 동작하지 않습니다. 사용자에게 잘못된 정렬 기준을 보여주는 것은 혼동을 줄 수 있습니다.Also applies to: 95-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/corkagemap/mystore/MyStore.tsx` at line 64, The code calls getBookmarkGroupDetail(group.id, 'LATEST') but the dropdown UI shows "리뷰많은순" and has no handler, causing a mismatch and non-functional sorting; update the dropdown to use a bound sort state (e.g., sort, setSort) and wire an onChange/onClick handler (e.g., handleSortChange) that sets the selected sort key (e.g., 'LATEST' or 'MOST_REVIEWED') and re-calls getBookmarkGroupDetail(group.id, selectedSort) so the API sort parameter matches the dropdown label and user interactions actually change sorting.
🧹 Nitpick comments (6)
src/shared/components/restaurant_detail/DetailHeader.tsx (2)
63-66: 빈useEffect와 주석 처리된 코드 정리 필요Line 49의 주석 처리된
useBookmarkStore코드와 Lines 63-66의 빈useEffect(본문이 주석만 포함)는 데드 코드입니다. 프로덕션에 불필요한 코드가 남아있으면 가독성을 저하시킵니다.♻️ 제안: 데드 코드 제거
- //const selectedStores = useBookmarkStore((state) => state.selectedStores); - const [isContactModalOpen, setIsContactModalOpen] = useState(false);- useEffect(() => { - // state 초기화 (새로고침 시 안 뜨게) - //navigate(`/detail-info/${resId}`, { replace: true }); - }, [location.state]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/restaurant_detail/DetailHeader.tsx` around lines 63 - 66, Remove the dead/commented code in DetailHeader.tsx: delete the commented-out useBookmarkStore line and the empty useEffect block that only contains a commented navigate call, and then remove any now-unused imports (e.g., useEffect or navigate) triggered by those deletions; ensure no logic relied on those comments remains and run linter to confirm no unused symbols (check for useBookmarkStore, useEffect, and navigate references).
19-32: 인터페이스에 선언된adr,aliasprops가 사용되지 않음
detailProps인터페이스에adr과alias가 정의되어 있지만, 컴포넌트 구조 분해(Line 34-45)에서도, JSX에서도 사용되지 않습니다. 불필요한 props는 인터페이스에서 제거하거나 UI에 반영하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/restaurant_detail/DetailHeader.tsx` around lines 19 - 32, The interface detailProps declares adr and alias but they are not used in the DetailHeader component or its props destructuring; either remove adr and alias from detailProps (and from the component props list) to avoid unused prop types, or add them to the component’s destructuring and render them in the JSX where appropriate (e.g., show adr as address text and alias as secondary name). Update detailProps and the DetailHeader props handling consistently so no unused properties remain.src/shared/components/storecard/StoreCardInMultiPinList.tsx (2)
12-24:addressprop이 인터페이스에 정의되었지만 사용되지 않음
StoreCardProps인터페이스의address(Line 18)가 컴포넌트 구조 분해에도 JSX에도 사용되지 않습니다. 주석에 "필요하다면 UI에 추가"라고 되어 있으므로, 현재 불필요하면 인터페이스에서 제거하거나, 필요하면 UI에 반영하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/storecard/StoreCardInMultiPinList.tsx` around lines 12 - 24, StoreCardProps declares an unused address prop; either remove address from the interface or wire it into the component: update the StoreCardInMultiPinList component to include address in the props destructuring (e.g., add address to the function signature where StoreCardProps is used) and render it in the JSX (for example under the store name or in a dedicated address element), or simply delete address from StoreCardProps if you don't intend to display it—make sure to update any usages of StoreCardProps so types remain consistent.
147-161: 카드 리스트에서 카드마다GroupSelector인스턴스가 생성됨 — 성능 고려리스트의 각
StoreCard마다 독립적인GroupSelector(Portal 기반 BottomSheet)가 렌더링됩니다. 카드 수가 많아지면 DOM에 불필요한 Portal 노드가 다수 마운트될 수 있습니다. 한 번에 하나의GroupSelector만 열리므로, 부모 레벨에서 단일GroupSelector를 공유하고 열릴 때 해당 카드의 데이터를 주입하는 방식이 더 효율적입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/storecard/StoreCardInMultiPinList.tsx` around lines 147 - 161, Each StoreCard currently mounts its own GroupSelector and GroupList (see GroupSelector, GroupList, isGroupSelectorOpen, setIsGroupSelectorOpen), which causes many portal nodes; instead lift the GroupSelector to the parent list component and render a single shared GroupSelector instance there, managing open state and the active card data via a selectedRestaurantId/selectedRestaurantName (or similar) state in the parent; update StoreCard to only call a prop like onOpenGroupSelector(resId, name) to set the parent state and open the shared GroupSelector, and remove the per-card GroupSelector/GroupList from StoreCard so only one portal node is mounted at a time.src/pages/corkagemap/mystore/MyStore.tsx (1)
112-112: 주석 처리된 prop 제거
//currentGroupId={group.id}는 더 이상StoreCardInSave에서 사용하지 않는 prop이므로 주석 대신 완전히 삭제하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/corkagemap/mystore/MyStore.tsx` at line 112, Remove the stale commented prop from MyStore.tsx: delete the line "//currentGroupId={group.id}" because StoreCardInSave no longer accepts or uses currentGroupId; ensure no other references to currentGroupId remain in the render where StoreCardInSave is used and remove any related unused imports or variables if they become unnecessary.src/shared/components/storecard/StoreCardInSave.tsx (1)
139-154:StoreCardInMultiPinList와 동일한 성능 고려사항리스트의 각 카드마다
GroupSelectorPortal 인스턴스가 생성되는 구조입니다.StoreCardInMultiPinList.tsx에 대한 코멘트와 동일하게, 부모 레벨에서 단일GroupSelector를 공유하는 방식이 더 효율적입니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/storecard/StoreCardInSave.tsx` around lines 139 - 154, This card renders a separate GroupSelector portal per card (GroupSelector and GroupList inside StoreCardInSave), causing wasted Portal mounts; refactor so the parent component owns a single shared GroupSelector instance and cards only request it to open with payload (restaurantId, name) and close callbacks (e.g., expose events or setter like setGroupSelectorPayload and setIsGroupSelectorOpen), remove per-card GroupSelector/GroupList rendering from StoreCardInSave and instead pass an onOpenGroupSelector handler from the parent that stops propagation and sets the shared modal's state; keep identifiers GroupSelector, GroupList, isGroupSelectorOpen, restaurantId, name and onClose to wire the shared modal correctly.
🤖 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/shared/apis/bookmark/bookmark.api.ts`:
- Around line 83-91: The comment says the endpoint returns storedFlag per group
but the GroupListResponse type lacks it; update the GroupListResponse (the group
item/interface used by getRestaurantBookmarkStatus) to include a storedFlag:
boolean field for each group entry so callers can safely read storedFlag, and
ensure any serialization/mapping that builds GroupListResponse (used by
getRestaurantBookmarkStatus) populates that field; alternatively, if the
endpoint does not return storedFlag, remove/update the comment in
getRestaurantBookmarkStatus to avoid claiming storedFlag is included.
In `@src/shared/components/storecard/StoreCardInSave.tsx`:
- Around line 65-72: The click handler is bound to the <img> instead of the
<button> in StoreCardInSave.tsx; move the onClick={handleKeepClick} from the img
element up to the surrounding button (make the button <button type="button"
onClick={handleKeepClick}>) and remove the onClick from the img, mirroring
StoreCardInMultiPinList.tsx; also ensure the img retains src={isKeep ? Save :
NotSave} and alt text and consider adding an accessible label on the button if
one is missing.
---
Outside diff comments:
In `@src/pages/corkagemap/mystore/MyStore.tsx`:
- Line 64: The code calls getBookmarkGroupDetail(group.id, 'LATEST') but the
dropdown UI shows "리뷰많은순" and has no handler, causing a mismatch and
non-functional sorting; update the dropdown to use a bound sort state (e.g.,
sort, setSort) and wire an onChange/onClick handler (e.g., handleSortChange)
that sets the selected sort key (e.g., 'LATEST' or 'MOST_REVIEWED') and re-calls
getBookmarkGroupDetail(group.id, selectedSort) so the API sort parameter matches
the dropdown label and user interactions actually change sorting.
In `@src/shared/components/restaurant_detail/DetailHeader.tsx`:
- Around line 193-195: The mapped <p> elements in corkageOption.map are missing
a key prop which causes React warnings; update the mapping inside DetailHeader's
render (the corkageOption.map callback) to supply a stable key for each <p> —
preferably key={option} if option values are unique, otherwise use key={index}
as a fallback (use the map's index parameter) to ensure each <p> has a key.
---
Duplicate comments:
In `@src/shared/components/storecard/StoreCardInMultiPinList.tsx`:
- Around line 44-52: handleKeepClick currently always opens the GroupSelector
regardless of isKeep; update it to check the isKeep flag and perform the correct
action: if isKeep is true, call the existing unkeep/remove function (or open a
confirm/unkeep flow) to immediately un-save the store, otherwise
setIsGroupSelectorOpen(true) to add/save to a group; reference handleKeepClick,
isKeep and GroupSelector to locate the logic. Also replace the placeholder
console.log in handleShareClick with the real share behavior (e.g., open the
Share modal or invoke the share handler used elsewhere), referencing
handleShareClick and whatever share modal/handler exists in the component to
implement the actual sharing flow.
---
Nitpick comments:
In `@src/pages/corkagemap/mystore/MyStore.tsx`:
- Line 112: Remove the stale commented prop from MyStore.tsx: delete the line
"//currentGroupId={group.id}" because StoreCardInSave no longer accepts or uses
currentGroupId; ensure no other references to currentGroupId remain in the
render where StoreCardInSave is used and remove any related unused imports or
variables if they become unnecessary.
In `@src/shared/components/restaurant_detail/DetailHeader.tsx`:
- Around line 63-66: Remove the dead/commented code in DetailHeader.tsx: delete
the commented-out useBookmarkStore line and the empty useEffect block that only
contains a commented navigate call, and then remove any now-unused imports
(e.g., useEffect or navigate) triggered by those deletions; ensure no logic
relied on those comments remains and run linter to confirm no unused symbols
(check for useBookmarkStore, useEffect, and navigate references).
- Around line 19-32: The interface detailProps declares adr and alias but they
are not used in the DetailHeader component or its props destructuring; either
remove adr and alias from detailProps (and from the component props list) to
avoid unused prop types, or add them to the component’s destructuring and render
them in the JSX where appropriate (e.g., show adr as address text and alias as
secondary name). Update detailProps and the DetailHeader props handling
consistently so no unused properties remain.
In `@src/shared/components/storecard/StoreCardInMultiPinList.tsx`:
- Around line 12-24: StoreCardProps declares an unused address prop; either
remove address from the interface or wire it into the component: update the
StoreCardInMultiPinList component to include address in the props destructuring
(e.g., add address to the function signature where StoreCardProps is used) and
render it in the JSX (for example under the store name or in a dedicated address
element), or simply delete address from StoreCardProps if you don't intend to
display it—make sure to update any usages of StoreCardProps so types remain
consistent.
- Around line 147-161: Each StoreCard currently mounts its own GroupSelector and
GroupList (see GroupSelector, GroupList, isGroupSelectorOpen,
setIsGroupSelectorOpen), which causes many portal nodes; instead lift the
GroupSelector to the parent list component and render a single shared
GroupSelector instance there, managing open state and the active card data via a
selectedRestaurantId/selectedRestaurantName (or similar) state in the parent;
update StoreCard to only call a prop like onOpenGroupSelector(resId, name) to
set the parent state and open the shared GroupSelector, and remove the per-card
GroupSelector/GroupList from StoreCard so only one portal node is mounted at a
time.
In `@src/shared/components/storecard/StoreCardInSave.tsx`:
- Around line 139-154: This card renders a separate GroupSelector portal per
card (GroupSelector and GroupList inside StoreCardInSave), causing wasted Portal
mounts; refactor so the parent component owns a single shared GroupSelector
instance and cards only request it to open with payload (restaurantId, name) and
close callbacks (e.g., expose events or setter like setGroupSelectorPayload and
setIsGroupSelectorOpen), remove per-card GroupSelector/GroupList rendering from
StoreCardInSave and instead pass an onOpenGroupSelector handler from the parent
that stops propagation and sets the shared modal's state; keep identifiers
GroupSelector, GroupList, isGroupSelectorOpen, restaurantId, name and onClose to
wire the shared modal correctly.
Comment on lines
83
to
91
| // 특정 매장이 각 그룹에 저장되어 있는지 여부 조회 (storedFlag 포함) | ||
| export const getRestaurantBookmarkStatus = async ( | ||
| restaurantId: number | ||
| ): Promise<GroupListResponse> => { | ||
| const response = await apiClient.get<GroupListResponse>( | ||
| `/bookmarks/groups/restaurants/${restaurantId}` | ||
| ); | ||
| return response.data; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify if storedFlag is used anywhere in the codebase and check GroupListResponse type
echo "=== GroupListResponse type definition ==="
rg -n "GroupListResponse" --type=ts -C 5
echo ""
echo "=== storedFlag usage ==="
rg -n "storedFlag" --type=ts -C 3
echo ""
echo "=== getRestaurantBookmarkStatus usage ==="
rg -n "getRestaurantBookmarkStatus" --type=ts -C 3Repository: CorkCharge/CorkCharge-FE
Length of output: 4399
🏁 Script executed:
#!/bin/bash
# Get the complete GroupListResponse type definition
echo "=== Complete GroupListResponse type definition ==="
sed -n '95,115p' src/shared/apis/bookmark/bookmark.type.ts
echo ""
echo "=== Check if there are any other similar response types with storedFlag ==="
rg -n "storedFlag" --type=tsRepository: CorkCharge/CorkCharge-FE
Length of output: 756
🏁 Script executed:
#!/bin/bash
# Search for all usages of getRestaurantBookmarkStatus in components and pages
echo "=== getRestaurantBookmarkStatus full usage search ==="
rg "getRestaurantBookmarkStatus" --type=ts --type=tsx
echo ""
echo "=== Check all files in src directory ==="
fd . src --type f -name "*.ts" -o -name "*.tsx" | xargs rg "getRestaurantBookmarkStatus"Repository: CorkCharge/CorkCharge-FE
Length of output: 531
GroupListResponse 타입에 storedFlag 필드가 누락됨
주석에 "storedFlag 포함"이라고 명시되어 있지만, GroupListResponse 타입의 groups 배열 항목에는 해당 필드가 정의되어 있지 않습니다. 실제 타입 정의는 groupId, name, color, visibility, storeCount, createdAt, updatedAt만 포함하고 있습니다.
이 엔드포인트가 그룹별 저장 여부(storedFlag)를 반환한다면 타입을 확장해야 하며, 그렇지 않다면 주석을 수정하세요. 그렇지 않으면 향후 이 데이터에 접근하는 코드에서 타입 에러가 발생하거나 undefined를 처리해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/apis/bookmark/bookmark.api.ts` around lines 83 - 91, The comment
says the endpoint returns storedFlag per group but the GroupListResponse type
lacks it; update the GroupListResponse (the group item/interface used by
getRestaurantBookmarkStatus) to include a storedFlag: boolean field for each
group entry so callers can safely read storedFlag, and ensure any
serialization/mapping that builds GroupListResponse (used by
getRestaurantBookmarkStatus) populates that field; alternatively, if the
endpoint does not return storedFlag, remove/update the comment in
getRestaurantBookmarkStatus to avoid claiming storedFlag is included.
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
src/shared/components/restaurant_detail/DetailHeader.tsx (2)
23-24: 주석 처리된 인터페이스 프로퍼티 제거 필요
adr과alias가 사용되지 않는 채로 주석으로 남아 있습니다. 불필요하면 제거하고, 향후 필요하다면 타입 정의에 복원하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/restaurant_detail/DetailHeader.tsx` around lines 23 - 24, Remove the commented-out interface properties "adr" and "alias" from DetailHeader.tsx (they currently appear as //adr: string; and //alias?: string;) so the codebase doesn't carry unused commented type members; if these props are actually required later, re-add them properly to the relevant interface/type (e.g., the DetailHeader props/interface) rather than leaving them commented in the file.
46-46:Number()변환 불필요
rating은 이미number타입이므로Number(rating)래핑은 중복입니다.♻️ 수정 제안
- const displayRating = Number(rating).toFixed(1); + const displayRating = rating.toFixed(1);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/restaurant_detail/DetailHeader.tsx` at line 46, The code wraps an already-number `rating` in Number(), which is redundant; update the `displayRating` declaration in DetailHeader (the `displayRating` variable) to call toFixed directly on `rating` (e.g., use `rating.toFixed(1)`) so the unnecessary Number() conversion is removed while preserving the one-decimal formatting.src/shared/components/storecard/StoreCardInSave.tsx (1)
152-197: 공유 모달 중복 코드
StoreCardInMultiPinList.tsx와 동일한 공유 모달 + 복사완료 토스트 코드가 반복됩니다.StoreCardInMultiPinList.tsx에서 제안한 공통ShareModal컴포넌트 추출 시 이 파일도 함께 적용하세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/storecard/StoreCardInSave.tsx` around lines 152 - 197, Duplicate share-modal and copy-toast markup in StoreCardInSave.tsx should be replaced by the shared ShareModal component used in StoreCardInMultiPinList.tsx; remove the inline Modal + copy-toast JSX (the block using isShareModalOpen, clipLink, and isCopiedModalOpen) and render <ShareModal .../> instead, passing the same props/handlers (e.g., name, logo, clipLink, isShareModalOpen, setIsShareModalOpen, isCopiedModalOpen, setIsCopiedModalOpen) so behavior remains identical; keep the surrounding event.stopPropagation wrappers and do not modify the GroupSelector block (GroupSelector, GroupList, restaurantId, restaurantName) — just swap the duplicated modal/toast code for the ShareModal import and prop usage.src/shared/components/storecard/StoreCardInMultiPinList.tsx (2)
41-41:Number()변환 불필요 —rating은 이미number타입
DetailHeader.tsx와 동일한 패턴으로,rating.toFixed(1)으로 충분합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/storecard/StoreCardInMultiPinList.tsx` at line 41, The expression creating displayRating uses Number(rating).toFixed(1) but rating is already a number; change the code in the StoreCardInMultiPinList component to call rating.toFixed(1) directly (update the displayRating binding to use rating.toFixed(1)), keeping the same variable name and behavior.
162-191: 공유 모달 코드 중복 —ShareModal공통 컴포넌트 추출 권장공유하기 모달 + 복사완료 토스트 조합이
DetailHeader.tsx,StoreCardInMultiPinList.tsx,StoreCardInSave.tsx세 곳에 거의 동일하게 복사되어 있습니다. 공통ShareModal컴포넌트로 추출하면 유지보수 부담을 줄일 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/storecard/StoreCardInMultiPinList.tsx` around lines 162 - 191, Extract the duplicated "공유하기 모달 + 복사완료 토스트" into a reusable ShareModal component and replace the inline JSX in StoreCardInMultiPinList (and the other two files) with it: create a ShareModal that renders the Modal and the copied-toast, accepts props {isOpen, onClose, onCopy (clipLink), isCopied, name, logo, check} (or let ShareModal manage isCopied internally and expose onCopy callback), preserves the outer onClick={(e)=>e.stopPropagation()} behavior, and reuses Modal and Button; then update calls in StoreCardInMultiPinList to pass isShareModalOpen, setIsShareModalOpen, clipLink, isCopiedModalOpen, name, logo and check to the new ShareModal.
🤖 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/shared/components/restaurant_detail/DetailHeader.tsx`:
- Around line 108-110: Replace the clickable <div> that toggles the group
selector with an accessible <button> element: in the DetailHeader component use
a <button type="button"> (instead of the div with onClick) that calls
setIsGroupSelectorOpen(true), preserve the <img src={isScrap ? save : notsave}
alt="bookmark" className="h-[32px] w-[32px]" /> inside it, and add an accessible
state prop such as aria-pressed={isScrap} and an appropriate aria-label (e.g.,
"Toggle bookmark groups") so keyboard and screen-reader users can interact with
the control.
In `@src/shared/components/storecard/StoreCardInMultiPinList.tsx`:
- Around line 50-53: The GroupList save flow currently only calls onClose(), so
the parent StoreCardInMultiPinList's isKeep (used to render the bookmark icon)
isn't updated; update the GroupList success handler to either call
setIsKeep(true) on successful save (e.g., in the GroupList onSave or onClose
callback invoked by handleKeepClick) or trigger a tanstack-query cache
invalidation/refetch for the query that provides the scrap prop so the parent
re-reads the latest scrap state and the icon updates.
- Around line 61-66: clipLink currently copies window.location.href (the
multi-pin list page) instead of the specific store detail URL; update the
clipLink function to construct and copy the store-specific URL (e.g., using the
store identifier available in this component such as store.id or store.slug or
an existing helper like getStoreUrl/storeDetailPath) — replace
navigator.clipboard.writeText(window.location.href) with
navigator.clipboard.writeText(<constructedStoreUrl>), keep the modal state calls
(setIsShareModalOpen, setIsCopiedModalOpen) and timeout logic unchanged.
In `@src/shared/components/storecard/StoreCardInSave.tsx`:
- Line 37: The local state isKeep (and setIsKeep) causes incorrect initial UI;
remove the useState and related useEffect in StoreCardInSave.tsx and compute the
keep flag directly from the Zustand store instead (derive isKeep from
selectedStores, e.g. check selectedStores for the current store id/unique key).
Replace all uses of isKeep/setIsKeep with the derived boolean and any handler
should call the existing selector/actions on the Zustand store (e.g., add/remove
store) instead of toggling local state.
- Around line 56-61: The clipLink function currently copies window.location.href
(the saved-list page) instead of the specific store detail URL; update clipLink
to build and copy the store's detail URL (e.g., using the store's unique
identifier/slug prop available to the component) rather than
window.location.href — locate clipLink in StoreCardInSave.tsx and replace
navigator.clipboard.writeText(window.location.href) with writing the constructed
detail URL (for example buildDetailUrl(store.id || store.slug) or use an
existing getStoreDetailUrl/route helper), keeping the rest of the modal
open/close logic the same.
---
Duplicate comments:
In `@src/shared/apis/bookmark/bookmark.api.ts`:
- Around line 83-91: The GroupListResponse used by getRestaurantBookmarkStatus
is missing the storedFlag boolean on each groups item; update the response type
in bookmark.type.ts (either add storedFlag: boolean to the groups item interface
or create a new response type used by getRestaurantBookmarkStatus) so the API
return shape matches actual data and consumers won't receive undefined when
accessing storedFlag.
---
Nitpick comments:
In `@src/shared/components/restaurant_detail/DetailHeader.tsx`:
- Around line 23-24: Remove the commented-out interface properties "adr" and
"alias" from DetailHeader.tsx (they currently appear as //adr: string; and
//alias?: string;) so the codebase doesn't carry unused commented type members;
if these props are actually required later, re-add them properly to the relevant
interface/type (e.g., the DetailHeader props/interface) rather than leaving them
commented in the file.
- Line 46: The code wraps an already-number `rating` in Number(), which is
redundant; update the `displayRating` declaration in DetailHeader (the
`displayRating` variable) to call toFixed directly on `rating` (e.g., use
`rating.toFixed(1)`) so the unnecessary Number() conversion is removed while
preserving the one-decimal formatting.
In `@src/shared/components/storecard/StoreCardInMultiPinList.tsx`:
- Line 41: The expression creating displayRating uses Number(rating).toFixed(1)
but rating is already a number; change the code in the StoreCardInMultiPinList
component to call rating.toFixed(1) directly (update the displayRating binding
to use rating.toFixed(1)), keeping the same variable name and behavior.
- Around line 162-191: Extract the duplicated "공유하기 모달 + 복사완료 토스트" into a
reusable ShareModal component and replace the inline JSX in
StoreCardInMultiPinList (and the other two files) with it: create a ShareModal
that renders the Modal and the copied-toast, accepts props {isOpen, onClose,
onCopy (clipLink), isCopied, name, logo, check} (or let ShareModal manage
isCopied internally and expose onCopy callback), preserves the outer
onClick={(e)=>e.stopPropagation()} behavior, and reuses Modal and Button; then
update calls in StoreCardInMultiPinList to pass isShareModalOpen,
setIsShareModalOpen, clipLink, isCopiedModalOpen, name, logo and check to the
new ShareModal.
In `@src/shared/components/storecard/StoreCardInSave.tsx`:
- Around line 152-197: Duplicate share-modal and copy-toast markup in
StoreCardInSave.tsx should be replaced by the shared ShareModal component used
in StoreCardInMultiPinList.tsx; remove the inline Modal + copy-toast JSX (the
block using isShareModalOpen, clipLink, and isCopiedModalOpen) and render
<ShareModal .../> instead, passing the same props/handlers (e.g., name, logo,
clipLink, isShareModalOpen, setIsShareModalOpen, isCopiedModalOpen,
setIsCopiedModalOpen) so behavior remains identical; keep the surrounding
event.stopPropagation wrappers and do not modify the GroupSelector block
(GroupSelector, GroupList, restaurantId, restaurantName) — just swap the
duplicated modal/toast code for the ShareModal import and prop usage.
Comment on lines
108
to
110
| <div className="cursor-pointer" onClick={() => setIsGroupSelectorOpen(true)}> | ||
| <img src={isScrap ? save : notsave} alt="bookmark" className="h-[32px] w-[32px]" /> | ||
| </div> |
There was a problem hiding this comment.
인터랙티브 요소에 <button> 사용 필요 — 접근성
북마크 클릭 영역이 <div>로 구현되어 키보드 접근 및 스크린 리더 인식이 불가합니다.
♻️ 수정 제안
- <div className="cursor-pointer" onClick={() => setIsGroupSelectorOpen(true)}>
- <img src={isScrap ? save : notsave} alt="bookmark" className="h-[32px] w-[32px]" />
- </div>
+ <button type="button" onClick={() => setIsGroupSelectorOpen(true)} aria-label="북마크">
+ <img src={isScrap ? save : notsave} alt="" className="h-[32px] w-[32px]" />
+ </button>📝 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.
Suggested change
| <div className="cursor-pointer" onClick={() => setIsGroupSelectorOpen(true)}> | |
| <img src={isScrap ? save : notsave} alt="bookmark" className="h-[32px] w-[32px]" /> | |
| </div> | |
| <button type="button" onClick={() => setIsGroupSelectorOpen(true)} aria-label="북마크"> | |
| <img src={isScrap ? save : notsave} alt="" className="h-[32px] w-[32px]" /> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/components/restaurant_detail/DetailHeader.tsx` around lines 108 -
110, Replace the clickable <div> that toggles the group selector with an
accessible <button> element: in the DetailHeader component use a <button
type="button"> (instead of the div with onClick) that calls
setIsGroupSelectorOpen(true), preserve the <img src={isScrap ? save : notsave}
alt="bookmark" className="h-[32px] w-[32px]" /> inside it, and add an accessible
state prop such as aria-pressed={isScrap} and an appropriate aria-label (e.g.,
"Toggle bookmark groups") so keyboard and screen-reader users can interact with
the control.
| corkageOption, | ||
| }: StoreCardProps) => { | ||
| const [isSaved, setIsSaved] = useState(true); | ||
| const [isKeep, setIsKeep] = useState(true); |
There was a problem hiding this comment.
isKeep 초기값 true로 인한 렌더링 불일치
useState(true)로 초기화하면 첫 렌더에 항상 "저장됨" 아이콘이 표시되고, useEffect가 실행된 이후에야 올바른 상태로 교정됩니다. selectedStores는 Zustand로 이미 반응형이므로 별도 로컬 state 없이 파생값으로 직접 계산하는 것이 깔끔합니다.
♻️ 수정 제안
- const [isKeep, setIsKeep] = useState(true);
+ const isKeep = restaurantId in selectedStores;
- useEffect(() => {
- setIsKeep(restaurantId in selectedStores);
- }, [restaurantId, selectedStores]);Also applies to: 63-65
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/components/storecard/StoreCardInSave.tsx` at line 37, The local
state isKeep (and setIsKeep) causes incorrect initial UI; remove the useState
and related useEffect in StoreCardInSave.tsx and compute the keep flag directly
from the Zustand store instead (derive isKeep from selectedStores, e.g. check
selectedStores for the current store id/unique key). Replace all uses of
isKeep/setIsKeep with the derived boolean and any handler should call the
existing selector/actions on the Zustand store (e.g., add/remove store) instead
of toggling local state.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/shared/components/storecard/StoreCardInSave.tsx (1)
50-56:handleCloseGroupSelector의isKeep수동 업데이트가useEffect와 중복
handleCloseGroupSelector내부의setIsKeep호출(54-55행)은 71-73행의useEffect가 이미 처리하므로 제거해도 동작이 동일합니다.♻️ 수정 제안
const handleCloseGroupSelector = () => { setIsGroupSelectorOpen(false); - - const isCurrentlySaved = restaurantId in selectedStores; - setIsKeep(isCurrentlySaved); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/storecard/StoreCardInSave.tsx` around lines 50 - 56, The setIsKeep call inside handleCloseGroupSelector is redundant because the useEffect that watches selectedStores already updates isKeep; remove the manual setIsKeep from handleCloseGroupSelector (keep the setIsGroupSelectorOpen(false) behavior) so isKeep is only updated by the existing useEffect that references selectedStores and restaurantId.
🤖 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/shared/components/restaurant_detail/DetailHeader.tsx`:
- Around line 60-63: The effect that runs setIsKeep based on "resId in
selectedStores" is overwriting the server-provided isScrap initial state; change
the useEffect (the one referencing setIsKeep, resId, selectedStores) so it only
updates local state when the store explicitly contains this resId (i.e., if
(resId in selectedStores) setIsKeep(true); otherwise do nothing) so that we
don't flip a server-true isScrap to false on mount; keep useState(isScrap) as
the authoritative initial value and only override it when the zustand store
clearly indicates the item is saved.
In `@src/shared/components/storecard/StoreCardInMultiPinList.tsx`:
- Around line 201-207: The "copied" toast overlay in StoreCardInMultiPinList
(rendered when isCopiedModalOpen) sits inside the clickable card and doesn't
stop event propagation, so clicks on the overlay still trigger the card's
onClick (navigate); update the overlay container (the element rendered when
isCopiedModalOpen) to call stopPropagation on click events (e.g., add an onClick
handler that calls event.stopPropagation())—apply this to the outer fixed
overlay and/or the inner toast box to mirror the fix used in StoreCardInSave and
prevent navigation while the toast is visible.
- Around line 70-83: Two useEffect hooks (the one updating isKeep from scrap and
the one from selectedStores) conflict and cause server scrap to be overwritten;
consolidate into a single useEffect that treats scrap as the source of truth
when it's defined and otherwise falls back to checking selectedStores. In the
combined effect (referencing isKeep, setIsKeep, scrap, selectedStores, resId)
first check if scrap is not undefined/null and call setIsKeep(scrap); if scrap
is unavailable, compute isCurrentlyInStore safely (e.g., guard selectedStores is
an object/Map and use resId in selectedStores or check
selectedStores[resId]?.length) and call setIsKeep(isCurrentlyInStore); ensure
the effect depends on [scrap, resId, selectedStores].
In `@src/shared/components/storecard/StoreCardInSave.tsx`:
- Around line 183-189: The overlay rendered when isCopiedModalOpen is true is
currently covering the whole card and letting clicks bubble to the card's
onClick={navigate}; prevent this by stopping propagation on the overlay
container (the element rendering the fixed inset-0 overlay / the inner toast
wrapper) so clicks on the toast don’t trigger the parent card
navigation—implement an onClick handler that calls stopPropagation (same
approach used for the share modal) on the overlay wrapper around the check/toast
elements.
---
Duplicate comments:
In `@src/shared/components/restaurant_detail/DetailHeader.tsx`:
- Around line 120-122: The bookmark <button> in DetailHeader.tsx is missing
explicit type and accessibility attributes; update the <button> (where
setIsGroupSelectorOpen is used and isKeep/save/notsave are referenced) to
include type="button", an aria-label that reflects the action (e.g.,
aria-label={isKeep ? "Remove bookmark" : "Add bookmark"}), and
aria-pressed={isKeep} so screen readers can perceive the toggle state.
In `@src/shared/components/storecard/StoreCardInSave.tsx`:
- Line 37: The component StoreCardInSave currently initializes isKeep with
useState(true) causing the saved icon to flash on first render; change it to
derive the initial value from selectedStores (or make isKeep a pure derived
value) so the initial render uses selectedStores.includes(store.id) (or a
useMemo that checks selectedStores and store.id) and remove/adjust the useEffect
that overwrites isKeep on mount; target the isKeep state in StoreCardInSave and
the effect that updates it to implement this derived initialization.
---
Nitpick comments:
In `@src/shared/components/storecard/StoreCardInSave.tsx`:
- Around line 50-56: The setIsKeep call inside handleCloseGroupSelector is
redundant because the useEffect that watches selectedStores already updates
isKeep; remove the manual setIsKeep from handleCloseGroupSelector (keep the
setIsGroupSelectorOpen(false) behavior) so isKeep is only updated by the
existing useEffect that references selectedStores and restaurantId.
Comment on lines
+201
to
+207
| {isCopiedModalOpen && ( | ||
| <div className="fixed inset-0 z-50 flex justify-center bg-black/50"> | ||
| <div className="absolute top-12 flex h-12 w-[125px] items-center justify-center rounded-xl bg-white p-6 font-semibold text-[var(--primary)] shadow-lg"> | ||
| <img src={check} /> | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
"복사완료" 토스트 오버레이에 stopPropagation 누락
StoreCardInSave와 동일한 문제입니다. fixed inset-0 토스트가 카드의 onClick={navigate} div 안에 래퍼 없이 위치하여, 1초 동안 화면 클릭 시 상세 페이지로 이동합니다.
🐛 수정 제안
- {isCopiedModalOpen && (
- <div className="fixed inset-0 z-50 flex justify-center bg-black/50">
- <div className="absolute top-12 flex h-12 w-[125px] items-center justify-center rounded-xl bg-white p-6 font-semibold text-[var(--primary)] shadow-lg">
- <img src={check} />
- </div>
- </div>
- )}
+ {isCopiedModalOpen && (
+ <div onClick={(e) => e.stopPropagation()}>
+ <div className="fixed inset-0 z-50 flex justify-center bg-black/50">
+ <div className="absolute top-12 flex h-12 w-[125px] items-center justify-center rounded-xl bg-white p-6 font-semibold text-[var(--primary)] shadow-lg">
+ <img src={check} alt="복사 완료" />
+ </div>
+ </div>
+ </div>
+ )}📝 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.
Suggested change
| {isCopiedModalOpen && ( | |
| <div className="fixed inset-0 z-50 flex justify-center bg-black/50"> | |
| <div className="absolute top-12 flex h-12 w-[125px] items-center justify-center rounded-xl bg-white p-6 font-semibold text-[var(--primary)] shadow-lg"> | |
| <img src={check} /> | |
| </div> | |
| </div> | |
| )} | |
| {isCopiedModalOpen && ( | |
| <div onClick={(e) => e.stopPropagation()}> | |
| <div className="fixed inset-0 z-50 flex justify-center bg-black/50"> | |
| <div className="absolute top-12 flex h-12 w-[125px] items-center justify-center rounded-xl bg-white p-6 font-semibold text-[var(--primary)] shadow-lg"> | |
| <img src={check} alt="복사 완료" /> | |
| </div> | |
| </div> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/components/storecard/StoreCardInMultiPinList.tsx` around lines 201
- 207, The "copied" toast overlay in StoreCardInMultiPinList (rendered when
isCopiedModalOpen) sits inside the clickable card and doesn't stop event
propagation, so clicks on the overlay still trigger the card's onClick
(navigate); update the overlay container (the element rendered when
isCopiedModalOpen) to call stopPropagation on click events (e.g., add an onClick
handler that calls event.stopPropagation())—apply this to the outer fixed
overlay and/or the inner toast box to mirror the fix used in StoreCardInSave and
prevent navigation while the toast is visible.
Comment on lines
+183
to
+189
| {isCopiedModalOpen && ( | ||
| <div className="fixed inset-0 z-50 flex justify-center bg-black/50"> | ||
| <div className="absolute top-12 flex h-12 w-[125px] items-center justify-center rounded-xl bg-white p-6 font-semibold text-[var(--primary)] shadow-lg"> | ||
| <img src={check} /> | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
"복사완료" 토스트 클릭 시 카드 네비게이션 트리거 가능
isCopiedModalOpen 오버레이(fixed inset-0)가 카드 외부 <div onClick={navigate}> 내부에 stopPropagation 래퍼 없이 렌더됩니다. 토스트가 표시된 1초간 화면 어디를 클릭해도 상세 페이지로 이동하게 됩니다. 공유 모달과 동일하게 감싸 주세요.
🐛 수정 제안
- {isCopiedModalOpen && (
- <div className="fixed inset-0 z-50 flex justify-center bg-black/50">
- <div className="absolute top-12 ...">
- <img src={check} />
- </div>
- </div>
- )}
+ {isCopiedModalOpen && (
+ <div onClick={(e) => e.stopPropagation()}>
+ <div className="fixed inset-0 z-50 flex justify-center bg-black/50">
+ <div className="absolute top-12 ...">
+ <img src={check} alt="복사 완료" />
+ </div>
+ </div>
+ </div>
+ )}📝 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.
Suggested change
| {isCopiedModalOpen && ( | |
| <div className="fixed inset-0 z-50 flex justify-center bg-black/50"> | |
| <div className="absolute top-12 flex h-12 w-[125px] items-center justify-center rounded-xl bg-white p-6 font-semibold text-[var(--primary)] shadow-lg"> | |
| <img src={check} /> | |
| </div> | |
| </div> | |
| )} | |
| {isCopiedModalOpen && ( | |
| <div onClick={(e) => e.stopPropagation()}> | |
| <div className="fixed inset-0 z-50 flex justify-center bg-black/50"> | |
| <div className="absolute top-12 flex h-12 w-[125px] items-center justify-center rounded-xl bg-white p-6 font-semibold text-[var(--primary)] shadow-lg"> | |
| <img src={check} alt="복사 완료" /> | |
| </div> | |
| </div> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/components/storecard/StoreCardInSave.tsx` around lines 183 - 189,
The overlay rendered when isCopiedModalOpen is true is currently covering the
whole card and letting clicks bubble to the card's onClick={navigate}; prevent
this by stopping propagation on the overlay container (the element rendering the
fixed inset-0 overlay / the inner toast wrapper) so clicks on the toast don’t
trigger the parent card navigation—implement an onClick handler that calls
stopPropagation (same approach used for the share modal) on the overlay wrapper
around the check/toast elements.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/shared/components/restaurant_detail/DetailHeader.tsx (2)
23-24: 인터페이스에서 주석 처리된 필드 정리 필요
adr와alias가 주석 처리된 채로 남아 있습니다. 향후 사용 계획이 없다면 삭제하는 것이 좋습니다.♻️ 수정 제안
interface detailProps { resId: number; name: string; rating: number; - //adr: string; - //alias?: string; isOpen: boolean;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/restaurant_detail/DetailHeader.tsx` around lines 23 - 24, Remove the commented-out interface fields "adr" and "alias" from the DetailHeader.tsx interface declaration (the commented lines shown in the DetailHeader component) if they are not planned to be used; alternatively, if they are intended for future use, convert them to properly typed optional properties (e.g., adr?: string; alias?: string;) or add a clear TODO comment explaining why they're kept. Ensure the change targets the interface in DetailHeader.tsx so no leftover commented fields remain.
297-304:GroupList가isGroupSelectorOpen과 무관하게 항상 마운트됨
GroupList는 마운트 시useGetGroupList()훅을 통해 API 요청을 실행합니다.GroupSelector가 닫혀 있을 때도GroupList가 렌더링되어 있으므로, 불필요한 네트워크 요청이 발생합니다.StoreCardInMultiPinList에서 이미 적용한 조건부 렌더링 패턴을 참고하세요.♻️ 수정 제안
<GroupSelector isOpen={isGroupSelectorOpen} topSnapVh={17.8} onClose={handleCloseGroupSelector} > - <GroupList onClose={handleCloseGroupSelector} restaurantName={name} restaurantId={resId} /> + {isGroupSelectorOpen && ( + <GroupList onClose={handleCloseGroupSelector} restaurantName={name} restaurantId={resId} /> + )} </GroupSelector>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/components/restaurant_detail/DetailHeader.tsx` around lines 297 - 304, GroupList is always mounted and triggers useGetGroupList() even when the selector is closed; update DetailHeader to conditionally render the GroupList only when isGroupSelectorOpen is true (e.g., render <GroupList ... /> inside GroupSelector only when isGroupSelectorOpen) mirroring the StoreCardInMultiPinList pattern so the hook isn't called while the selector is closed; reference the GroupList and GroupSelector components and the isGroupSelectorOpen state when making this 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/shared/components/storecard/StoreCardInMultiPinList.tsx`:
- Around line 65-66: The setTimeout call in clipLink that sets
setIsCopiedModalOpen(true) stores no timer ID and can leak if the component
unmounts within 1s—capture the timeout ID (e.g., via a ref or local variable)
when calling setTimeout in clipLink, clear any existing timer before creating a
new one, and ensure you clearTimeout(timerId) in the component cleanup
(useEffect return) or when the component unmounts; reference the clipLink
handler and the setIsCopiedModalOpen state so you locate and update the exact
logic.
---
Duplicate comments:
In `@src/shared/components/restaurant_detail/DetailHeader.tsx`:
- Around line 126-128: The button that toggles the group selector should include
explicit type and ARIA state: update the button in DetailHeader (the element
using onClick={() => setIsGroupSelectorOpen(true)} and showing img based on
isKeep/save/notsave) to add type="button" and aria-pressed={isKeep} so keyboard
forms don't submit unexpectedly and screen readers know the toggle state; ensure
aria-pressed reflects the current isKeep boolean.
In `@src/shared/components/storecard/StoreCardInMultiPinList.tsx`:
- Around line 204-211: The copied-toast overlay currently lives inside the
StoreCardInMultiPinList clickable area so clicks on the fixed `div` bubble to
the card `onClick={navigate}`; update the overlay render (guarded by
`isCopiedModalOpen`) to prevent event propagation by attaching an onClick
handler that calls e.stopPropagation() (and e.preventDefault() if appropriate)
on the outer fixed container, and also add a descriptive `alt` attribute to the
`<img src={check} />` (e.g., alt="복사완료" or "copy complete") so the image is
accessible.
- Around line 51-54: handleKeepClick currently always opens GroupSelector and
never triggers an unbookmark API call when isKeep is true; change
handleKeepClick to check the isKeep flag and, if true, call the unbookmark flow
(e.g., invoke the existing unbookmark API helper or prop like
onUnkeep/unbookmarkStore) instead of opening GroupSelector, otherwise open the
selector as now; additionally verify and, if needed, update GroupList's
selection handler (e.g., onGroupsChange/in GroupList component) so that when the
selected groups array becomes empty it calls the same unbookmark API path (or
invokes a provided onUnkeep callback) to actually remove the bookmark.
---
Nitpick comments:
In `@src/shared/components/restaurant_detail/DetailHeader.tsx`:
- Around line 23-24: Remove the commented-out interface fields "adr" and "alias"
from the DetailHeader.tsx interface declaration (the commented lines shown in
the DetailHeader component) if they are not planned to be used; alternatively,
if they are intended for future use, convert them to properly typed optional
properties (e.g., adr?: string; alias?: string;) or add a clear TODO comment
explaining why they're kept. Ensure the change targets the interface in
DetailHeader.tsx so no leftover commented fields remain.
- Around line 297-304: GroupList is always mounted and triggers
useGetGroupList() even when the selector is closed; update DetailHeader to
conditionally render the GroupList only when isGroupSelectorOpen is true (e.g.,
render <GroupList ... /> inside GroupSelector only when isGroupSelectorOpen)
mirroring the StoreCardInMultiPinList pattern so the hook isn't called while the
selector is closed; reference the GroupList and GroupSelector components and the
isGroupSelectorOpen state when making this change.
Comment on lines
+65
to
+66
| setIsCopiedModalOpen(true); | ||
| setTimeout(() => setIsCopiedModalOpen(false), 1000); |
There was a problem hiding this comment.
setTimeout 반환 ID를 정리하지 않아 잠재적 메모리 누수
clipLink 내 setTimeout의 반환 ID를 저장하지 않아 컴포넌트가 1초 이내에 언마운트될 경우 정리되지 않습니다.
🛠️ 수정 제안
+ const timerRef = useRef<ReturnType<typeof setTimeout> | null>(null);
+
+ useEffect(() => {
+ return () => {
+ if (timerRef.current) clearTimeout(timerRef.current);
+ };
+ }, []);
+
const clipLink = () => {
navigator.clipboard.writeText(`${window.location.origin}/detail-info/${resId}`);
setIsShareModalOpen(false);
setIsCopiedModalOpen(true);
- setTimeout(() => setIsCopiedModalOpen(false), 1000);
+ if (timerRef.current) clearTimeout(timerRef.current);
+ timerRef.current = setTimeout(() => setIsCopiedModalOpen(false), 1000);
};📝 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.
Suggested change
| setIsCopiedModalOpen(true); | |
| setTimeout(() => setIsCopiedModalOpen(false), 1000); | |
| const timerRef = useRef<ReturnType<typeof setTimeout> | null>(null); | |
| useEffect(() => { | |
| return () => { | |
| if (timerRef.current) clearTimeout(timerRef.current); | |
| }; | |
| }, []); | |
| const clipLink = () => { | |
| navigator.clipboard.writeText(`${window.location.origin}/detail-info/${resId}`); | |
| setIsShareModalOpen(false); | |
| setIsCopiedModalOpen(true); | |
| if (timerRef.current) clearTimeout(timerRef.current); | |
| timerRef.current = setTimeout(() => setIsCopiedModalOpen(false), 1000); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/shared/components/storecard/StoreCardInMultiPinList.tsx` around lines 65
- 66, The setTimeout call in clipLink that sets setIsCopiedModalOpen(true)
stores no timer ID and can leak if the component unmounts within 1s—capture the
timeout ID (e.g., via a ref or local variable) when calling setTimeout in
clipLink, clear any existing timer before creating a new one, and ensure you
clearTimeout(timerId) in the component cleanup (useEffect return) or when the
component unmounts; reference the clipLink handler and the setIsCopiedModalOpen
state so you locate and update the exact logic.
TakioN
approved these changes
Feb 23, 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
2 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.
📝 작업 내용
📢 PR Point
이미지 첨부
🔧 다음 할 일
Summary by CodeRabbit
새로운 기능
개선 사항