-
Notifications
You must be signed in to change notification settings - Fork 2
[Feat] 그룹 상세보기 추가 & 그룹 수정/삭제 #92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughCorkageMap에 다중 뷰 바텀 시트(list/store)를 도입하고, List를 props 기반으로 리팩토링하며 MyGroup의 클릭/전파 처리를 추가하고 MyStore 및 StoreCard 컴포넌트를 새로 추가했습니다. 시트 상태와 선택 그룹을 상위로 올려 뷰 전환을 제어합니다. Changes
Sequence Diagram(s)%%{init: {"themeVariables":{"actorBorder":"#6B7280","actorBg":"#F3F4F6","noteBg":"#FEF3C7"}}}%%
sequenceDiagram
participant User as 사용자
participant List as List 컴포넌트
participant Corkage as CorkageMap
participant MyStore as MyStore
User->>List: 그룹 항목 클릭
List->>Corkage: onSelectGroup(group)
Corkage->>Corkage: setSelectedGroup(group), setSheetView('store'), open sheet
Corkage->>MyStore: render(selectedGroup)
MyStore-->>User: MyStore UI 표시 (마커, StoreCard 목록)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 주의 깊게 검토할 영역:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
There was a problem hiding this 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
🧹 Nitpick comments (3)
src/shared/components/storecard/StoreCard.tsx (1)
7-78: 목업 컴포넌트로는 충분하지만, 추후를 위해 props 설계를 고려해 주세요현재 식당 이름·평점·리뷰 수·영업 상태·이미지·비용/기타 문구가 모두 하드코딩되어 있어서, 나중에 실제 API 데이터를 연결할 때 한 번에 크게 손봐야 할 것 같습니다.
다음 단계에서 실제 데이터를 붙일 때,
storeName,rating,reviewCount,isOpen,closeTime,images,costText,etcText같은 props를 미리 정의해 두고- 더미 데이터는 상위에서 이 props에 넣어주는 방식
으로 바꾸면 재사용성과 테스트가 훨씬 좋아질 것 같습니다. 지금 단계에서 반드시 수정할 필요는 없지만, 다음 연동 작업 시 함께 리팩터링하는 것을 추천드립니다.
src/pages/corkagemap/list/MyGroup.tsx (1)
42-106: 그룹 클릭/메뉴 클릭 이벤트 분리가 잘 되어 있습니다
onClick을 루트 컨테이너에 두고, 메뉴 토글/메뉴 내부 버튼에서e.stopPropagation()으로 부모 클릭을 막아주는 구조라서:
- 그룹 행 클릭 → 상위
onClick(= 그룹 선택/네비게이션)만 호출되고- 점 3개 아이콘 및 팝업 메뉴 클릭 → 메뉴 토글/편집/삭제만 일어나며 상위 네비게이션은 트리거되지 않는 점이 명확합니다.
모바일 위주라면 큰 문제는 아니지만, 접근성을 조금 더 챙기고 싶다면:
- 루트
div에role="button"과tabIndex={0}를 주고onKeyDown에서Enter/Space에 대해onClick을 호출같은 형태로 키보드 조작까지 대응하는 것도 고려해 볼 수 있겠습니다.
src/pages/corkagemap/CorkageMap.tsx (1)
9-25: 그룹 선택 → MyStore 전환 흐름은 자연스럽지만,currentTopSnapVh주석/값을 맞추는 것이 좋겠습니다
myGroups를 상위CorkageMap으로 끌어올려 BottomSheet가 닫혀도 데이터가 유지되도록 한 구조는 의도에 잘 맞습니다.handleGroupSelect에서selectedGroup을 설정하고sheetView를'store'로 바꾸고,MyStore에서group이 없으면 렌더링하지 않도록 한 가드도 안정적입니다.- BottomSheet 닫을 때
setTimeout으로sheetView를'list'로 돌리는 것도 사용자 입장에서 “다시 열면 목록부터”라는 기대와 잘 맞습니다.다만 주석에는
- List일 때
topSnapVh = 17.8- MyStore일 때
topSnapVh = 19.8라고 되어 있는데, 실제 코드에서는
const currentTopSnapVh = sheetView === 'store' ? 17.8 : 17.8;로 두 뷰 모두 17.8로 고정되어 있습니다.실제 의도가
- 당분간 두 뷰 모두 17.8로 통일하려는 것인지,
- 아니면 BottomSheet 구현 이슈 때문에 임시로 맞춰둔 것인지
를 주석에 명시하거나, 의도가 정리되면 19.8 값을 반영해 주면 이후 읽는 사람이 혼동 없이 이해하기 쉬울 것 같습니다.
Also applies to: 34-42, 76-85
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
src/pages/corkagemap/mystore/v.svgis excluded by!**/*.svgsrc/shared/components/storecard/dummyFood.svgis excluded by!**/*.svgsrc/shared/components/storecard/notsave.svgis excluded by!**/*.svgsrc/shared/components/storecard/save.svgis excluded by!**/*.svgsrc/shared/components/storecard/share.svgis excluded by!**/*.svg
📒 Files selected for processing (5)
src/pages/corkagemap/CorkageMap.tsx(2 hunks)src/pages/corkagemap/list/List.tsx(3 hunks)src/pages/corkagemap/list/MyGroup.tsx(4 hunks)src/pages/corkagemap/mystore/MyStore.tsx(1 hunks)src/shared/components/storecard/StoreCard.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/pages/corkagemap/mystore/MyStore.tsx (1)
src/pages/corkagemap/list/List.tsx (1)
Group(11-17)
src/pages/corkagemap/CorkageMap.tsx (1)
src/pages/corkagemap/list/List.tsx (1)
Group(11-17)
🪛 GitHub Check: build-test
src/pages/corkagemap/list/List.tsx
[warning] 22-22:
'group' is defined but never used. Allowed unused args must match /^_$/u
🔇 Additional comments (1)
src/pages/corkagemap/list/List.tsx (1)
2-3: 그룹 상태 리프팅과onSelectGroup연동이 자연스럽습니다
Group타입을 export 해서 List, MyStore, CorkageMap이 공통 타입을 공유하도록 한 점,ListProps에myGroups/setMyGroups/onSelectGroup를 명시해서 데이터 흐름을 부모 → List → MyGroup으로 명확히 만든 점,MyGroup의onClick={() => onSelectGroup(group)}로 그룹 객체 전체를 상위에 전달해 바로selectedGroup으로 사용할 수 있게 한 구조가 서로 잘 맞물려 있습니다.
이제 List 내부에서 초기 그룹 데이터를 만들지 않기 때문에, 실제 초기 데이터가 있다면 반드시 상위(CorkageMap 등)에서
myGroups에 넣어주는지만 한 번 더 확인해 보시면 될 것 같습니다. 현재 구조 자체에는 문제 없어 보입니다.빌드/테스트 환경에서 List가 사용되는 모든 곳에서
myGroups,setMyGroups,onSelectGroup세 props가 빠짐없이 전달되는지 한 번씩 체크해 주세요(특히 다른 화면에서 List를 재사용하고 있다면).Also applies to: 11-25, 140-152
| import V from './v.svg'; | ||
| import StoreCard from '../../../shared/components/storecard/StoreCard'; // 경로 확인 필요 | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find MyStore.tsx
fd MyStore.tsx
# Find Tailwind configuration
fd -e 'tailwind.config.js' -o -e 'tailwind.config.ts' -o -e 'tailwind.config.cjs' -o -e 'tailwind.config.mjs'Repository: CorkCharge/CorkCharge-FE
Length of output: 287
🏁 Script executed:
#!/bin/bash
# Read the MyStore.tsx file
cat -n src/pages/corkagemap/mystore/MyStore.tsx
# Search for Tailwind config files
find . -maxdepth 3 -type f \( -name 'tailwind.config.js' -o -name 'tailwind.config.ts' -o -name 'tailwind.config.cjs' -o -name 'tailwind.config.mjs' \)Repository: CorkCharge/CorkCharge-FE
Length of output: 3683
🏁 Script executed:
#!/bin/bash
# Read the Tailwind config
cat -n ./tailwind.config.jsRepository: CorkCharge/CorkCharge-FE
Length of output: 596
The z-100 class will be stripped during the Tailwind build because it's not defined in the project's configuration, and the StoreCard repetition should be refactored for maintainability.
Based on verification of the codebase:
-
z-100 is invalid: The tailwind.config.js does NOT extend z-index values. Tailwind's default z-index scale only includes
z-0, z-10, z-20, z-30, z-40, z-50, z-auto. Thez-100class on line 52 will be stripped during the build, causing the marker image to potentially be hidden behind other elements. -
Correct fix: Use
z-[100](arbitrary value syntax) instead:- className="z-100 absolute left-[40px] top-0 h-[60px] w-[60px] -translate-y-1/2" + className="absolute left-[40px] top-0 h-[60px] w-[60px] -translate-y-1/2 z-[100]"
-
StoreCard repetition (lines 77–80): The four hardcoded
<StoreCard />instances should be refactored into a loop for clarity and to ease future data binding:- <StoreCard /> - <StoreCard /> - <StoreCard /> - <StoreCard /> + {Array.from({ length: 4 }).map((_, idx) => ( + <StoreCard key={idx} /> + ))}
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/pages/corkagemap/mystore/MyStore.tsx (reference lines ~52 and ~77-80),
replace the invalid Tailwind class z-100 with the arbitrary value syntax z-[100]
on the marker image so it isn't stripped by the build, and refactor the four
repeated <StoreCard /> instances into a single mapped render over an array of
store data (e.g., stores.map(...)) to remove repetition and make future data
binding straightforward; ensure each mapped StoreCard has a unique key and pass
props from the data objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/pages/corkagemap/list/ConfirmationModal.tsx (1)
2-2:createPortal로 모달을document.body에 렌더링하는 변경은 적절해 보입니다전반적으로 포털 사용 방식은 자연스럽고, z-index·오버레이 포함 구조도 문제 없어 보입니다. 다만 몇 가지 고려사항이 있습니다.
(선택) SSR 환경 사용 시
document가드 권장
Next.js 등 SSR을 쓴다면 서버 렌더 단계에서document가 없어서 에러가 날 수 있습니다. 그런 경우에는 컴포넌트 상단에서:if (typeof document === 'undefined') return null;처럼 가드를 추가하거나, 상위에서 “클라이언트 전용”으로만 이 컴포넌트를 렌더하도록 조건을 두는 걸 권장합니다.
(선택) 모달 접근성 속성 보강
변경 전에도 같은 상태였을 가능성이 높지만, 이제 전역 포털로 올린 만큼 추후에라도 아래 정도를 추가해 두면 좋을 것 같습니다.
motion.div에role="dialog"및aria-modal="true"- 헤딩/타이틀에
id부여 후aria-labelledby연결- 필요 시 ESC 키/배경 클릭으로 닫기(현재는 버튼만 onClose) 등은 디자인 정책에 따라 선택
구현 자체는 문제 없어 보이므로 그대로 머지해도 될 것 같고, 위 내용은 추후 작업 여유 있을 때 반영을 고려해 보시면 좋겠습니다.
Also applies to: 77-103
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/pages/corkagemap/list/ConfirmationModal.tsx(3 hunks)src/pages/corkagemap/mystore/MyStore.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/pages/corkagemap/mystore/MyStore.tsx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/shared/components/storecard/StoreCard.tsx (4)
7-23: 상단 헤더(가게명·아이콘)를 향후 props 기반으로 리팩토링하는 것을 고려해 주세요.목업이라 하드코딩된
"엔비햄버거"와 아이콘들이 이해되지만, 실제 그룹/매장 데이터 연동 시에는StoreCardProps(예:name,isSaved,onToggleSave,onShare)를 받아서 렌더링하도록 바꾸면 재사용성이 좋아집니다. 버튼 클릭 핸들러도 props로 올리면 상위(MyStore 등)에서 상태를 일관되게 관리하기 수월합니다.
25-42: 평점·리뷰·영업 정보 영역에 접근성과 재사용성을 조금 더 신경 쓰면 좋겠습니다.
- 평점/리뷰 수가 시각적으로는 잘 보이지만, 스크린 리더 입장에선 단순 숫자라 의미가 모호할 수 있습니다. 예를 들어
aria-label이나 문장형 텍스트(“평점 4.2점, 리뷰 333개”)를 추가하면 접근성이 좋아집니다."영업중","4:00 영업종료"도 실제 데이터로 대체될 예정이라면, 이후에는 영업 상태/마감 시간을 props로 분리해 두면 다양한 매장 상태(휴무, 브레이크타임 등)를 표현하기 편합니다.
44-55: 이미지 리스트에서 overflow 전략과 스크롤 UX를 한 번 더 점검해 보시면 좋겠습니다.주석에서는 “가로 스크롤 가능성”을 언급하지만, 현재는
overflow-hidden이라 실제로는 스크롤이 차단됩니다.
향후 썸네일이 더 많아질 것을 대비해 가로 스크롤을 의도하신 거라면overflow-x-auto+overflow-y-hidden조합이나, 스와이프 UX에 맞는 클래스로 조정하는 방향을 고려해 주세요.
56-76: 하단 비용/기타 영역의 하드코딩 텍스트와 레이아웃을 데이터/리스트 기반으로 풀어두면 확장성이 좋아집니다.
"병당 1만원","잔 제공, 얼음 제공, 리뷰이벤트: 한병 무료 ddddddddddddddd"는 명백한 더미 텍스트라 실제 배포 전에는 placeholder 티가 나지 않도록 정리하는 게 좋겠습니다(특히dddd...부분).- 비용/혜택 항목이 늘어날 여지가 크므로, 나중에는
costDescription,extras: string[]같은 형태로 props를 받아map으로 렌더링하면 텍스트 길이나 개수 변화에 더 유연하게 대응할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/shared/components/storecard/share.svgis excluded by!**/*.svg
📒 Files selected for processing (1)
src/shared/components/storecard/StoreCard.tsx(1 hunks)
🔇 Additional comments (1)
src/shared/components/storecard/StoreCard.tsx (1)
1-5: SVG 및 리소스 import 구조는 현재 형태로 무난합니다.에셋을
img src로 사용하는 구조도 일관되고, 목업 단계에서 NotSave 주석 import를 남겨둔 것도 이해됩니다. 나중에 실제 상태 토글 아이콘 구현 시점에만 정리해 주시면 될 것 같습니다.
📝 작업 내용
📢 PR Point
이미지 첨부
🔧 다음 할 일
Summary by CodeRabbit
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.