Feat : 콜키지맵 바텀시트 ->저장리스트 구현 #91
Conversation
WalkthroughFramer Motion 기반 바텀시트, 모달, 토글 컴포넌트와 그룹 관리 UI를 추가하고, 전체 화면 CorkageMap 레이아웃 및 두 개의 예제 페이지와 라우트를 도입했으며 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Router
participant ExamplePage
participant CorkageMap
participant BottomSheet
participant List
participant EditGroup
participant ConfirmationModal
User->>Router: /example 또는 /example2 요청
Router->>ExamplePage: 페이지 렌더
User->>CorkageMap: 우측 버튼 클릭 (목록 열기)
CorkageMap->>BottomSheet: isOpen=true
BottomSheet->>List: List 렌더링
User->>List: "새 그룹" 클릭
List->>EditGroup: edit 뷰로 전환
User->>EditGroup: 데이터 입력 후 저장
EditGroup->>List: onSave({name, iconName, privacy})
List->>List: myGroups 업데이트
List->>ConfirmationModal: showConfirmModal=true
User->>ConfirmationModal: 닫기 클릭
ConfirmationModal->>List: onClose()
List->>BottomSheet: 목록 갱신
User->>BottomSheet: 드래그/백드롭 클릭
BottomSheet->>CorkageMap: onClose()
sequenceDiagram
participant User
participant BottomSheet
participant FramerMotion
User->>BottomSheet: 열기 (isOpen=true)
BottomSheet->>FramerMotion: animate(y → MID)
User->>BottomSheet: 드래그 (y축)
BottomSheet->>BottomSheet: onDragEnd: velocity 검사
alt 빠른 아래로 (velocity.y > threshold)
BottomSheet->>BottomSheet: onClose()
else 빠른 위로 (velocity.y < -threshold)
BottomSheet->>FramerMotion: animate(y → TOP)
else 느린 드래그
BottomSheet->>BottomSheet: 가장 가까운 스냅 계산
BottomSheet->>FramerMotion: animate(y → nearest_snap)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45분
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (7)
src/pages/corkagemap/CorkageMap.tsx (1)
52-52: 매직 넘버에 대한 설명이 필요합니다.
topSnapVh={17.8}값의 의미가 명확하지 않습니다. 이 값이 왜 17.8인지, 무엇을 나타내는지 주석으로 설명하거나 상수로 추출하는 것을 권장합니다.+// 바텀시트 최상단 위치: 화면 높이의 17.8% 지점에서 시작 +const BOTTOM_SHEET_TOP_SNAP_VH = 17.8; + const CorkageMap = () => { ... - <BottomSheet isOpen={isSheetOpen} onClose={handleSheetClose} topSnapVh={17.8}> + <BottomSheet isOpen={isSheetOpen} onClose={handleSheetClose} topSnapVh={BOTTOM_SHEET_TOP_SNAP_VH}></comment_end>
src/pages/corkagemap/list/EditGroup.tsx (1)
49-55:alert()대신 더 나은 UX 패턴을 사용하세요.
alert()는 사용자 경험을 방해하고 모던한 웹 애플리케이션에서는 권장되지 않습니다. 또한isValid체크로 버튼이 비활성화되어 있기 때문에 이 alert는 실행되지 않을 것입니다.다음과 같이 수정하는 것을 권장합니다:
const handleSave = () => { - if (!groupName || !selectedIcon) { - alert('그룹명과 마커를 선택해주세요.'); - return; - } + // isValid 체크로 이미 방지됨 onSave({ name: groupName, iconName: selectedIcon, privacy }); };또는 인라인 에러 메시지나 토스트 알림을 사용하는 것을 고려하세요.
</comment_end>src/pages/corkagemap/list/List.tsx (1)
34-39: ID 생성 방식이 충돌 위험이 있습니다.
Date.now()를 사용한 ID 생성은 사용자가 빠르게 여러 그룹을 생성할 경우 충돌이 발생할 수 있습니다. 프로토타입 단계에서는 괜찮지만, 나중에 백엔드 API와 연동할 때는 서버에서 생성된 고유 ID를 사용해야 합니다.임시 해결책으로 더 안전한 ID 생성을 원한다면:
const handleSaveGroup = (data: Omit<Group, 'id' | 'count'>) => { const newGroup: Group = { ...data, - id: Date.now(), + id: Date.now() + Math.random(), // 충돌 가능성 감소 count: 0, };또는 임시 ID에 접두사를 사용:
- id: Date.now(), + id: `temp-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`,</comment_end>
src/pages/ExamplePage.tsx (1)
37-173: 로컬 BottomSheet 대신 공유 컴포넌트 재사용 고려이 파일 안에 BottomSheet 구현이 그대로 들어가 있는데,
src/shared/components/bottomsheet/BottomSheet.tsx에도 거의 동일한 구현이 존재합니다. 장기적으로 유지보수를 줄이려면 ExamplePage에서는 공유 BottomSheet를 import 해서 사용하는 형태로 맞추는 게 좋아 보입니다.src/pages/ExamplePage2.tsx (1)
60-72:topSnapVh변경 가능성을 고려한 deps 정리 (선택사항)
calculateSnapPx안에서topSnapVh를 사용하지만useEffect의 deps 배열이 빈 배열이라, 부모에서topSnapVh를 바꾸더라도 스냅 포인트 px 값은 갱신되지 않습니다. 현재는 상수(17.8)를 넘기고 있어서 문제는 없지만, 추후 동적으로 바꿀 계획이 있다면 다음처럼 deps에 포함시키는 편이 안전합니다.- useEffect(() => { + 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]);src/shared/components/bottomsheet/BottomSheet.tsx (2)
60-72:topSnapVh에 대한 반응형 스냅 재계산 (선택사항)여기도 ExamplePage2와 마찬가지로
calculateSnapPx에서topSnapVh를 사용하지만 deps 배열이 비어 있습니다. 현재는 대부분 고정 값으로 쓸 것 같아 당장 문제는 없지만, prop이 동적으로 바뀔 수 있는 구조라면 아래처럼 deps에 포함하는 편이 React hook 관점에서는 더 자연스럽습니다.- useEffect(() => { + 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]);
41-187: BottomSheet 단일 소스로의 통합 제안현재 ExamplePage2 내부 BottomSheet와 이 공용 BottomSheet가 거의 동일한 로직을 두 번 가지고 있습니다. 바텀시트 동작(스냅 지점, 드래그, 오버레이 등)이 앞으로 자주 바뀔 예정이라면, 이 파일만 단일 소스로 유지·개선하고 예제 페이지들은 모두 여기서 export한 컴포넌트를 import 하는 식으로 통합하는 걸 추천드립니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsrc/pages/corkagemap/list/X.svgis excluded by!**/*.svgsrc/pages/corkagemap/list/editDots.svgis excluded by!**/*.svgsrc/pages/corkagemap/list/plus.svgis excluded by!**/*.svgsrc/pages/corkagemap/list/savemarker/SaveMarker1.svgis excluded by!**/*.svgsrc/pages/corkagemap/list/savemarker/SaveMarker10.svgis excluded by!**/*.svgsrc/pages/corkagemap/list/savemarker/SaveMarker11.svgis excluded by!**/*.svgsrc/pages/corkagemap/list/savemarker/SaveMarker12.svgis excluded by!**/*.svgsrc/pages/corkagemap/list/savemarker/SaveMarker2.svgis excluded by!**/*.svgsrc/pages/corkagemap/list/savemarker/SaveMarker3.svgis excluded by!**/*.svgsrc/pages/corkagemap/list/savemarker/SaveMarker4.svgis excluded by!**/*.svgsrc/pages/corkagemap/list/savemarker/SaveMarker5.svgis excluded by!**/*.svgsrc/pages/corkagemap/list/savemarker/SaveMarker6.svgis excluded by!**/*.svgsrc/pages/corkagemap/list/savemarker/SaveMarker7.svgis excluded by!**/*.svgsrc/pages/corkagemap/list/savemarker/SaveMarker8.svgis excluded by!**/*.svgsrc/pages/corkagemap/list/savemarker/SaveMarker9.svgis excluded by!**/*.svgsrc/pages/corkagemap/save.svgis excluded by!**/*.svg
📒 Files selected for processing (11)
package.json(1 hunks)src/pages/ExamplePage.tsx(1 hunks)src/pages/ExamplePage2.tsx(1 hunks)src/pages/corkagemap/CorkageMap.tsx(1 hunks)src/pages/corkagemap/list/ConfirmationModal.tsx(1 hunks)src/pages/corkagemap/list/EditGroup.tsx(1 hunks)src/pages/corkagemap/list/List.tsx(1 hunks)src/pages/corkagemap/list/MyGroup.tsx(1 hunks)src/pages/corkagemap/list/ToggleSwitch.tsx(1 hunks)src/shared/components/bottomsheet/BottomSheet.tsx(1 hunks)src/shared/router/Router.tsx(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI (develop)
src/pages/ExamplePage.tsx
[error] 83-83: TypeScript error TS6133: 'event' is declared but its value is never read.
🪛 GitHub Check: build-test
src/pages/ExamplePage.tsx
[failure] 83-83:
'event' is declared but its value is never read.
src/shared/components/bottomsheet/BottomSheet.tsx
[failure] 97-97:
'event' is declared but its value is never read.
src/pages/ExamplePage2.tsx
[failure] 97-97:
'event' is declared but its value is never read.
🔇 Additional comments (9)
src/shared/router/Router.tsx (1)
43-44: 예제 라우트의 목적을 명확히 해주세요.
ExamplePage와ExamplePage2라우트가 추가되었습니다. 이것들이 개발/테스트 목적의 임시 라우트인지, 프로덕션에 포함되어야 하는 기능인지 확인이 필요합니다. 만약 테스트용이라면 머지 전에 제거하는 것을 권장합니다.</comment_end>
Also applies to: 211-218
src/pages/corkagemap/CorkageMap.tsx (1)
1-17: 상태 관리가 잘 구현되었습니다.바텀시트 열기/닫기 상태와 버튼 활성화 상태를 함께 관리하는
handleSheetClose구현이 명확하고 올바릅니다.</comment_end>
src/pages/corkagemap/list/EditGroup.tsx (2)
145-156: 절대 위치 버튼의 반응형 동작을 확인해주세요.버튼이
calc(75vh - 48px)위치에 고정되어 있습니다. 주석으로 의도가 설명되어 있지만, 다양한 화면 크기와 바텀시트 높이에서 올바르게 작동하는지 확인이 필요합니다. 특히 스크롤 가능한 콘텐츠와 함께 사용될 때 버튼이 콘텐츠를 가릴 수 있습니다.다음 시나리오에서 테스트를 권장합니다:
- 작은 화면 (모바일)
- 큰 화면 (태블릿)
- 바텀시트가 완전히 확장된 상태
- 바텀시트가 중간 위치에 있는 상태
</comment_end>
68-83: 입력 검증이 잘 구현되었습니다.10자 제한과 실시간 문자 수 카운터가 올바르게 구현되어 있습니다. 사용자 경험이 좋습니다.
</comment_end>
src/pages/corkagemap/list/List.tsx (2)
100-108: AnimatePresence 사용이 올바릅니다.모달의 마운트/언마운트 애니메이션을 위한
AnimatePresence사용이 적절합니다. framer-motion의 권장 패턴을 잘 따르고 있습니다.</comment_end>
18-26: 상태 관리 구조가 명확합니다.뷰 전환(
list/edit), 그룹 데이터, 모달 상태를 명확하게 분리하여 관리하고 있습니다. 컴포넌트의 의도를 이해하기 쉽습니다.</comment_end>
package.json (1)
16-16: framer-motion 버전이 최신 버전이며 보안상 문제가 없습니다.검증 결과:
- npm 레지스트리 최신 버전: 12.23.24 ✓
- 현재 package.json 버전: ^12.23.24 ✓
- 보안 취약점: 없음 ✓
지정된 버전이 최신이고 보안 권고사항상 문제가 없으므로 추가 조치가 필요하지 않습니다.
src/pages/corkagemap/list/ConfirmationModal.tsx (1)
57-57: z-index 충돌 우려는 실제 문제가 아닙니다.코드 검토 결과, ConfirmationModal의
z-[110]과 BottomSheet의z-[101]은 충돌하지 않습니다. z-index 값들이 올바르게 계층화되어 있습니다:
- ConfirmationModal: 110 (최상위)
- BottomSheet: 101
- NextButton: 100
- Footer: 99
ConfirmationModal이 BottomSheet보다 높은 z-index를 가지므로 확인 모달이 다른 컴포넌트 위에 올바르게 표시되는 것이 맞는 동작입니다. 실제 z-index 충돌은 없습니다.
Likely an incorrect or invalid review comment.
src/pages/corkagemap/list/ToggleSwitch.tsx (1)
17-17:layoutprop은 framer-motion v12.23.24에서 올바르게 지원됩니다.검증 결과, framer-motion 버전 12.23.24는
motion.div의layoutprop을 완전히 지원합니다. 공식 Motion 문서와 npm 패키지 정보에서 확인되었으므로 호환성 문제가 없습니다.
src/pages/corkagemap/list/List.tsx
Outdated
| {/* 토글 스위치 */} | ||
| <div className="flex h-8 cursor-pointer items-center gap-2 rounded-[20px] py-1"> | ||
| <span className="text-[16px] font-[400] text-[#80818B]">콜키지 정보 on/off</span> | ||
| <ToggleSwitch /> | ||
| </div> |
There was a problem hiding this comment.
토글 스위치가 기능과 연결되지 않았습니다.
"콜키지 정보 on/off" 텍스트가 있지만, ToggleSwitch 컴포넌트에 props가 없어서 실제로 어떤 기능도 제어하지 않습니다. 이는 사용자에게 혼란을 줄 수 있습니다.
다음 중 하나를 선택해야 합니다:
- 기능 구현:
ToggleSwitch를 제어 가능한 컴포넌트로 만들고 실제 콜키지 정보 표시/숨김 기능을 구현 - 임시 제거: 아직 구현되지 않았다면 이 UI 요소를 제거하고 나중에 추가
ToggleSwitch.tsx 파일에도 관련 리뷰 코멘트를 남겼습니다.
</comment_end>
🤖 Prompt for AI Agents
In src/pages/corkagemap/list/List.tsx around lines 66-70, the ToggleSwitch is
rendered without props so it does not control anything; make it a controlled
toggle that actually shows/hides corkage info: add a local state (e.g., const
[showCorkage, setShowCorkage] = useState<boolean>(true)), pass value and
onChange props to ToggleSwitch (value={showCorkage} onChange={setShowCorkage}),
and use showCorkage to conditionally render the corkage-related UI elsewhere in
this file; if you prefer to defer implementation instead, remove the entire
toggle block here and reintroduce it once the feature is implemented.
There was a problem hiding this comment.
@KingSeokJin, 알겠습니다! 나중에 구현하시는 것도 좋은 방법입니다.
추후 작업을 추적하기 위해 follow-up 이슈를 생성하시겠습니까? 원하시면 제가 도와드릴 수 있습니다.
| const ToggleSwitch = () => { | ||
| const [isOn, setIsOn] = useState(false); | ||
|
|
||
| return ( | ||
| <button | ||
| className={`relative flex h-[31px] w-[54px] items-center rounded-[16px] p-1 transition-colors ${ | ||
| isOn ? 'bg-[#90212A]' : 'bg-gray-300' | ||
| }`} | ||
| onClick={() => setIsOn(!isOn)} | ||
| aria-pressed={isOn} |
There was a problem hiding this comment.
토글 스위치를 제어 가능한 컴포넌트로 변경해야 합니다.
현재 ToggleSwitch는 내부 상태만 가지고 있어서 부모 컴포넌트가 상태를 제어하거나 변경사항을 감지할 수 없습니다. List.tsx에서 "콜키지 정보 on/off" 기능을 구현하려면 이 컴포넌트가 제어 가능해야 합니다.
다음과 같이 수정하는 것을 권장합니다:
-const ToggleSwitch = () => {
- const [isOn, setIsOn] = useState(false);
+type ToggleSwitchProps = {
+ isOn: boolean;
+ onToggle: (isOn: boolean) => void;
+};
+
+const ToggleSwitch = ({ isOn, onToggle }: ToggleSwitchProps) => {
return (
<button
className={`relative flex h-[31px] w-[54px] items-center rounded-[16px] p-1 transition-colors ${
isOn ? 'bg-[#90212A]' : 'bg-gray-300'
}`}
- onClick={() => setIsOn(!isOn)}
+ onClick={() => onToggle(!isOn)}
aria-pressed={isOn}
></comment_end>
🤖 Prompt for AI Agents
In src/pages/corkagemap/list/ToggleSwitch.tsx around lines 4 to 13, the
component currently holds internal state so parents cannot control or observe
its value; change it to a controlled component by removing internal useState and
accepting props (e.g., checked: boolean, onChange: (newValue: boolean) => void),
use the checked prop instead of isOn for rendering and aria-pressed, and call
onChange(!checked) from the click handler; update any parent (List.tsx) to keep
the boolean state and pass it down plus a handler to update it.
There was a problem hiding this comment.
토글 기능 구현할때 참고하여 반영하겠습니다.
There was a problem hiding this comment.
@KingSeokJin 감사합니다! 토글 기능 구현 시 해당 피드백이 도움이 되길 바랍니다. 추가로 필요한 사항이 있으시면 언제든지 말씀해 주세요! 👍
src/pages/ExamplePage.tsx
Outdated
| const onDragEnd = (event: MouseEvent | TouchEvent | PointerEvent, info: PanInfo) => { | ||
| const { velocity } = info; | ||
|
|
||
| // 아래로 빠르게 쓸어내리면 닫기 | ||
| if (velocity.y > DRAG_VELOCITY_THRESHOLD) { | ||
| onClose(); | ||
| return; | ||
| } | ||
|
|
||
| // 위로 빠르게 쓸어올리면 최대 높이로 | ||
| if (velocity.y < -DRAG_VELOCITY_THRESHOLD) { | ||
| controls.start('snapTop'); | ||
| return; | ||
| } | ||
|
|
||
| // 2. 느린 드래그 (위치 기반 스냅) | ||
| // 현재 y 위치 가져오기 | ||
| const currentY = sheetRef.current ? sheetRef.current.getBoundingClientRect().y : 0; | ||
|
|
||
| // 현재 vh 값 계산 (대략적) | ||
| const currentVh = (currentY / window.innerHeight) * 100; | ||
|
|
||
| // 각 스냅 지점의 중간값 계산 | ||
| const midPointTopMid = (SNAP_POINTS.TOP + SNAP_POINTS.MID) / 2; | ||
| const midPointMidMin = (SNAP_POINTS.MID + SNAP_POINTS.MIN) / 2; | ||
| const midPointMinClose = (SNAP_POINTS.MIN + SNAP_POINTS.HIDDEN) / 2; | ||
|
|
||
| if (currentVh < midPointTopMid) { | ||
| controls.start('snapTop'); // 최대 높이로 | ||
| } else if (currentVh < midPointMidMin) { | ||
| controls.start('snapMid'); // 중간 높이로 | ||
| } else if (currentVh < midPointMinClose) { | ||
| controls.start('snapMin'); // 최소 높이로 | ||
| } else { | ||
| onClose(); // 닫기 | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
onDragEnd의 event 미사용으로 TS6133 오류 발생
CI 로그대로 onDragEnd에서 event를 선언만 하고 읽지 않아 TS6133(noUnusedParameters)로 빌드가 깨집니다. event를 사용하지 않을 거라면, 다음과 같이 “사용한 것”으로 처리해 컴파일 경고를 제거하는 쪽이 안전합니다.
- const onDragEnd = (event: MouseEvent | TouchEvent | PointerEvent, info: PanInfo) => {
- const { velocity } = info;
+ const onDragEnd = (event: MouseEvent | TouchEvent | PointerEvent, info: PanInfo) => {
+ void event; // 사용하지 않는 파라미터임을 명시적으로 표시
+ const { velocity } = info;수정 후 yarn build/npm run build로 CI 에러가 사라지는지 한 번 확인 부탁드립니다.
🏁 Script executed:
#!/bin/bash
# 1. ExamplePage.tsx 파일 찾기
fd -t f "ExamplePage.tsx"Length of output: 25
🏁 Script executed:
#!/bin/bash
# 2. tsconfig.json 확인 (noUnusedParameters 설정)
fd -t f "tsconfig.json" | head -5Length of output: 13
🏁 Script executed:
#!/bin/bash
# 3. ExamplePage.tsx에서 onDragEnd 함수 주변 코드 확인 (더 넓은 범위)
if fd -t f "ExamplePage.tsx" > /dev/null 2>&1; then
FILE=$(fd -t f "ExamplePage.tsx" | head -1)
if [ -n "$FILE" ]; then
echo "=== File: $FILE ==="
sed -n '80,125p' "$FILE"
fi
fiLength of output: 1311
🏁 Script executed:
#!/bin/bash
# 4. event 파라미터가 onDragEnd 함수 내에서 사용되는지 검색
if fd -t f "ExamplePage.tsx" > /dev/null 2>&1; then
FILE=$(fd -t f "ExamplePage.tsx" | head -1)
if [ -n "$FILE" ]; then
echo "=== Checking for 'event' usage in ExamplePage.tsx ==="
rg -n "event" "$FILE" -A 5 -B 5
fi
fiLength of output: 382
🏁 Script executed:
#!/bin/bash
# tsconfig.json에서 noUnusedParameters 설정 확인
cat tsconfig.jsonLength of output: 118
🏁 Script executed:
#!/bin/bash
# tsconfig.app.json 확인
cat tsconfig.app.jsonLength of output: 791
onDragEnd의 event 파라미터 미사용으로 TS6133 오류 발생
검증 결과 src/pages/ExamplePage.tsx 83번 라인의 onDragEnd 함수에서 event 파라미터가 선언되지만 함수 본문에서 사용되지 않고 있습니다. tsconfig.app.json에서 "noUnusedParameters": true로 설정되어 있으므로, 현재 코드는 TypeScript 컴파일 오류(TS6133)를 발생시킵니다.
제안된 수정은 정확하고 표준적인 패턴입니다:
- const onDragEnd = (event: MouseEvent | TouchEvent | PointerEvent, info: PanInfo) => {
- const { velocity } = info;
+ const onDragEnd = (event: MouseEvent | TouchEvent | PointerEvent, info: PanInfo) => {
+ void event; // 사용하지 않는 파라미터임을 명시적으로 표시
+ const { velocity } = info;🧰 Tools
🪛 GitHub Actions: CI (develop)
[error] 83-83: TypeScript error TS6133: 'event' is declared but its value is never read.
🪛 GitHub Check: build-test
[failure] 83-83:
'event' is declared but its value is never read.
🤖 Prompt for AI Agents
src/pages/ExamplePage.tsx around lines 83-119: the onDragEnd function declares
an unused event parameter which triggers TS6133; fix by removing the unused
event parameter from the function signature (or rename it to _event to
intentionally mark it unused) and adjust the type annotation so the function
only requires the PanInfo parameter (ensure any callers/handlers remain
compatible after the signature change).
| const onDragEnd = (event: MouseEvent | TouchEvent | PointerEvent, info: PanInfo) => { | ||
| const { velocity } = info; | ||
|
|
||
| // 1. 빠른 플릭(flick) 제스처 감지 | ||
| if (velocity.y > DRAG_VELOCITY_THRESHOLD) { | ||
| onClose(); // (useEffect[isOpen]이 y를 HIDDEN으로 애니메이션함) | ||
| return; | ||
| } | ||
| if (velocity.y < -DRAG_VELOCITY_THRESHOLD) { | ||
| animate(y, snapPx.TOP, { type: 'tween', duration: 0.3, ease: 'easeOut' }); | ||
| setCurrentSnap('TOP'); | ||
| return; | ||
| } | ||
|
|
||
| // 2. 느린 드래그 (위치 기반 스냅) | ||
| const currentY = y.get(); // 현재 y 픽셀 값 | ||
|
|
||
| // 각 스냅 지점의 중간값 계산 | ||
| const midPointTopMid = (snapPx.TOP + snapPx.MID) / 2; | ||
| const midPointMidMin = (snapPx.MID + snapPx.MIN) / 2; | ||
| const midPointMinClose = (snapPx.MIN + snapPx.HIDDEN) / 2; | ||
|
|
||
| if (currentY < midPointTopMid) { | ||
| animate(y, snapPx.TOP, { type: 'tween', duration: 0.3, ease: 'easeOut' }); | ||
| setCurrentSnap('TOP'); | ||
| } else if (currentY < midPointMidMin) { | ||
| animate(y, snapPx.MID, { type: 'tween', duration: 0.3, ease: 'easeOut' }); | ||
| setCurrentSnap('MID'); | ||
| } else if (currentY < midPointMinClose) { | ||
| animate(y, snapPx.MIN, { type: 'tween', duration: 0.3, ease: 'easeOut' }); | ||
| setCurrentSnap('MIN'); | ||
| } else { | ||
| onClose(); // (useEffect[isOpen]이 y를 HIDDEN으로 애니메이션함) | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
공용 BottomSheet의 onDragEnd도 TS6133 대상
실제 앱에서 사용하는 공용 BottomSheet에서도 onDragEnd의 event 파라미터를 사용하지 않아 TS6133이 발생합니다. 빌드를 깨는 부분이므로 여기서도 동일하게 처리해 주는 게 좋겠습니다.
- const onDragEnd = (event: MouseEvent | TouchEvent | PointerEvent, info: PanInfo) => {
- const { velocity } = info;
+ const onDragEnd = (event: MouseEvent | TouchEvent | PointerEvent, info: PanInfo) => {
+ void event;
+ const { velocity } = info;🏁 Script executed:
#!/bin/bash
# BottomSheet 파일 존재 및 onDragEnd 함수 확인
find . -name "BottomSheet.tsx" -type f 2>/dev/null | head -5Length of output: 51
🏁 Script executed:
#!/bin/bash
# onDragEnd 함수의 event 파라미터 사용 여부 확인
if [ -f "src/shared/components/bottomsheet/BottomSheet.tsx" ]; then
sed -n '97,131p' "src/shared/components/bottomsheet/BottomSheet.tsx"
fiLength of output: 1273
🏁 Script executed:
#!/bin/bash
# TypeScript 설정 확인 (tsconfig.json 내 noUnusedParameters 설정)
if [ -f "tsconfig.json" ]; then
cat tsconfig.json | grep -A5 -B5 "noUnusedParameters\|noImplicitAny\|strict"
fiLength of output: 0
🏁 Script executed:
#!/bin/bash
# ExamplePage2와의 유사 패턴 확인
rg -n "const onDragEnd" --type=tsx --type=ts -C3Length of output: 27
🏁 Script executed:
#!/bin/bash
# tsconfig.json 전체 확인
cat tsconfig.jsonLength of output: 118
🏁 Script executed:
#!/bin/bash
# ExamplePage2 onDragEnd 검색 (grep 사용)
grep -rn "onDragEnd" --include="*.tsx" --include="*.ts" src/ 2>/dev/null | head -20Length of output: 580
🏁 Script executed:
#!/bin/bash
# package.json의 빌드 및 타입체크 스크립트 확인
cat package.json | grep -A10 "scripts"Length of output: 248
🏁 Script executed:
#!/bin/bash
# tsconfig.app.json 확인 (strict 모드 및 noUnusedParameters)
cat tsconfig.app.jsonLength of output: 791
🏁 Script executed:
#!/bin/bash
# ExamplePage2의 onDragEnd를 상세히 확인 (라인 97 주변)
sed -n '95,105p' src/pages/ExamplePage2.tsxLength of output: 349
BottomSheet의 onDragEnd 파라미터 미사용 확인
검증 결과, 리뷰 코멘트의 지적이 정확합니다.
tsconfig.app.json에"noUnusedParameters": true로 설정되어 있음- BottomSheet.tsx 라인 97의
onDragEnd함수에서event파라미터가 실제로 미사용 중 - 빌드 스크립트(
tsc -b && vite build)에서 TypeScript 컴파일 단계에서 TS6133 오류 발생 - ExamplePage.tsx(라인 83)와 ExamplePage2.tsx(라인 97)도 동일한 문제 보유
제안된 수정(void event;)이 올바른 TypeScript 관례이며, 빌드 실패를 해결합니다.
- const onDragEnd = (event: MouseEvent | TouchEvent | PointerEvent, info: PanInfo) => {
- const { velocity } = info;
+ const onDragEnd = (event: MouseEvent | TouchEvent | PointerEvent, info: PanInfo) => {
+ void event;
+ const { velocity } = info;🧰 Tools
🪛 GitHub Check: build-test
[failure] 97-97:
'event' is declared but its value is never read.
🤖 Prompt for AI Agents
In src/shared/components/bottomsheet/BottomSheet.tsx around lines 97 to 131, the
onDragEnd signature includes an unused first parameter (event) which triggers
TS6133 under "noUnusedParameters"; to fix, mark the parameter as intentionally
unused by referencing it with a void expression (e.g., void event;) at the start
of the function or rename it to _event and reference it similarly, then run the
build to confirm the TypeScript error is resolved; apply the same void-event fix
to ExamplePage.tsx (line 83) and ExamplePage2.tsx (line 97) where the same
unused-parameter error occurs.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
src/shared/components/bottomsheet/BottomSheet.tsx (1)
97-131:onDragEnd의_event미사용으로 TS6133 계속 발생 가능이전 리뷰에서 지적됐던 것처럼,
tsconfig.app.json에noUnusedParameters: true가 켜져 있다면, 이름만_event로 바꿔도 TypeScript 컴파일러는 여전히 “사용되지 않은 파라미터”로 간주합니다.TS6133를 확실히 피하려면 파라미터를 실제로 한 번 읽어 주거나 시그니처에서 제거해야 합니다. 아래처럼
void로 소비하는 패턴이 무난합니다.- const onDragEnd = (_event: MouseEvent | TouchEvent | PointerEvent, info: PanInfo) => { - const { velocity } = info; + const onDragEnd = (event: MouseEvent | TouchEvent | PointerEvent, info: PanInfo) => { + void event; + const { velocity } = info;같은 패턴을 사용하는 다른 파일(예:
ExamplePage.tsx,ExamplePage2.tsx)에도 동일하게 적용해 두면 CI에서 타입 체크 실패를 막을 수 있습니다.src/pages/ExamplePage2.tsx (1)
97-131: 여기도_event미사용으로 TS6133 위험 (ExamplePage와 동일 패턴)이 파일의
onDragEnd도 첫 번째 인자_event를 전혀 읽지 않아,noUnusedParameters: true설정에서는 컴파일 에러가 날 수 있습니다. 이름만_event로 바꾸면 ESLint 규칙에는 도움이 될 수 있지만, TypeScript 컴파일러는 여전히 “미사용”으로 판단합니다.공용 컴포넌트와 동일하게
void로 소모해 주는 식으로 정리하는 걸 추천드립니다.- const onDragEnd = (_event: MouseEvent | TouchEvent | PointerEvent, info: PanInfo) => { - const { velocity } = info; + const onDragEnd = (event: MouseEvent | TouchEvent | PointerEvent, info: PanInfo) => { + void event; + const { velocity } = info;src/pages/ExamplePage.tsx (2)
83-119:onDragEnd의_event미사용 – TS6133 여전히 남아 있을 가능성이전 리뷰에서 지적된 것처럼,
_event로 이름만 바꾸면 TypeScript의noUnusedParameters에러는 그대로 남습니다. 이 구현에서도_event를 전혀 읽지 않고 있어 타입 체크 단계에서 막힐 수 있습니다.공용 BottomSheet에서와 동일하게, 파라미터를 실제로 한 번 소비하도록 변경하는 쪽을 권장합니다.
- const onDragEnd = (_event: MouseEvent | TouchEvent | PointerEvent, info: PanInfo) => { - const { velocity } = info; + const onDragEnd = (event: MouseEvent | TouchEvent | PointerEvent, info: PanInfo) => { + void event; + const { velocity } = info;
151-162:height: calc(100vh - ${SNAP_POINTS.TOP})에서 단위 누락으로 CSS 무효 가능성
SNAP_POINTS.TOP이 vh 단위 전제로 정의되어 있는데, height 계산에서${SNAP_POINTS.TOP}뒤에 단위가 없어calc(100vh - 17.8)같은 형태가 됩니다. 이 값은 CSS에서 유효한 길이 단위가 아니어서, 브라우저에 따라 height가 무시될 수 있습니다.아래처럼 vh 단위를 명시하는 편이 안전합니다.
- style={{ - height: `calc(100vh - ${SNAP_POINTS.TOP})`, // 최대 높이 + style={{ + height: `calc(100vh - ${SNAP_POINTS.TOP}vh)`, // 최대 높이
🧹 Nitpick comments (5)
src/shared/components/bottomsheet/BottomSheet.tsx (2)
52-72:useEffect에서topSnapVh의존성 누락
calculateSnapPx안에서topSnapVh를 사용하지만 의존성 배열이[]라서, 부모가topSnapVh를 변경해도 스냅 지점(px)이 갱신되지 않습니다.
topSnapVh를 의존성에 추가해서 prop 변동 시에도 올바르게 재계산되도록 하는 편이 안전해 보입니다.- useEffect(() => { + 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]);
111-167: 느린 드래그로 “아래로 닫기”가 사실상 불가능한 스냅/제약 조합
midPointMinClose를 기준으로 느린 드래그 시 바텀시트를 닫는 로직을 두셨지만,dragConstraints.bottom이snapPx.MIN(열려 있는 최소 위치)라서 사용자가 천천히 아래로 끌어도currentY가midPointMinClose까지 내려가지 못해onClose()분기로 거의 진입하지 못합니다.의도하신 UX가 “천천히 내려도 닫힘”이라면:
dragConstraints.bottom을snapPx.HIDDEN근처로 넓히거나,- 또는
midPointMinClose를snapPx.MIN기준으로 다시 정의하는 쪽이 자연스러울 것 같습니다. 지금 상태도 기능상 치명적이진 않지만, 기대한 제스처와 실제 동작이 다를 수 있어 한 번 더 UX 관점에서 점검해 보시면 좋겠습니다.
src/pages/ExamplePage2.tsx (2)
52-72:useEffect의topSnapVh의존성 추가 권장 (lint 경고 반영)
calculateSnapPx안에서topSnapVh를 사용하지만 의존성 배열이 비어 있어, ESLint 경고대로 prop 변경 시 스냅 지점이 갱신되지 않습니다.
topSnapVh를 의존성에 포함해 두면 부모가 최대 높이를 바꿀 때도 안전하게 따라갑니다.- useEffect(() => { + 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]);
8-131: 공용 BottomSheet와 로직이 1:1로 중복 – import로 통합 권장
src/shared/components/bottomsheet/BottomSheet.tsx의 구현과 이 파일 상단 BottomSheet 구현이 거의 동일해서, 두 군데가 앞으로 쉽게 벌어질 수 있습니다.예시 페이지라 하더라도, 가능하면 공용 컴포넌트를 import 해서 사용하는 쪽이 유지보수성이 좋아 보입니다.
예시:
-import { motion, useMotionValue, useTransform, animate } from 'framer-motion'; -import { useState, useEffect } from 'react'; -import type { ReactNode } from 'react'; -import type { PanInfo } from 'framer-motion'; +import type { ReactNode } from 'react'; +import BottomSheet from '@/shared/components/bottomsheet/BottomSheet'; ... -// BottomSheet.tsx 컴포넌트 코드 -... -const BottomSheet = ({ isOpen, onClose, children }: BottomSheetProps) => { - ... -}; +// 공용 BottomSheet 컴포넌트를 사용합니다.(실제 import 경로는 프로젝트 alias에 맞춰 조정 필요)
src/pages/ExamplePage.tsx (1)
11-174: ExamplePage 내 BottomSheet 구현도 공용 컴포넌트로 대체 고려이 파일도 ExamplePage2,
src/shared/components/bottomsheet/BottomSheet.tsx와 별도의 BottomSheet 구현을 갖고 있어, 총 3군데에서 바텀시트 로직이 관리되는 구조입니다. 향후 스냅 포인트나 제스처 로직을 수정할 때 동기화 비용이 커질 수 있습니다.예제 페이지라 하더라도:
- 공용 BottomSheet만 남기고,
- ExamplePage/ExamplePage2는 모두 import 해서 사용하는 형태
로 통합하면 유지보수/버그 방지 측면에서 더 깔끔할 것 같습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/pages/ExamplePage.tsx(1 hunks)src/pages/ExamplePage2.tsx(1 hunks)src/shared/components/bottomsheet/BottomSheet.tsx(1 hunks)
🧰 Additional context used
🪛 GitHub Check: build-test
src/pages/ExamplePage2.tsx
[warning] 72-72:
React Hook useEffect has a missing dependency: 'topSnapVh'. Either include it or remove the dependency array. If 'setSnapPx' needs the current value of 'topSnapVh', you can also switch to useReducer instead of useState and read 'topSnapVh' in the reducer
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/pages/corkagemap/list/List.tsx (1)
119-122: 토글 스위치가 여전히 기능과 연결되지 않았습니다."콜키지 정보 on/off" 텍스트와 함께
ToggleSwitch컴포넌트가 렌더링되지만, props가 없어 아무 기능도 제어하지 않습니다. 이는 이전 리뷰에서도 지적된 사항입니다.제어 가능한 토글로 만들거나, 아직 구현되지 않았다면 해당 UI 요소를 제거하는 것을 권장합니다.
🧹 Nitpick comments (5)
src/pages/corkagemap/list/MyGroup.tsx (2)
72-74: 키보드 접근성 개선이 필요합니다.점 메뉴 버튼이 마우스 클릭만 지원하고 키보드 탐색을 지원하지 않습니다.
다음과 같이 수정하여 접근성을 개선하세요:
- <div className="cursor-pointer p-1" onClick={handleToggleMenu}> + <button + className="cursor-pointer p-1" + onClick={handleToggleMenu} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + handleToggleMenu(); + } + }} + aria-label="그룹 메뉴 열기" + aria-expanded={isMenuOpen} + > <img src={EditDots} alt="EditDots" /> - </div> + </button>
45-46: 아이콘 폴백 처리를 고려하세요.
iconName이smallMarkers맵에 없는 경우 아이콘이 표시되지 않습니다. 현재 조건부 렌더링으로 안전하게 처리되지만, 기본 아이콘이나 플레이스홀더를 표시하면 사용자 경험이 향상될 수 있습니다.Also applies to: 66-66
src/pages/corkagemap/list/List.tsx (2)
83-83: ID 생성 방식을 개선하는 것을 고려하세요.
Date.now()를 ID로 사용하는 것은 대부분의 경우 작동하지만, 빠르게 연속으로 그룹을 생성하면 ID 충돌이 발생할 수 있습니다.더 안전한 대안:
- 카운터 기반 ID (예:
useRef로 관리)- UUID 라이브러리 사용
- 서버에서 ID 생성 (향후 백엔드 통합 시)
71-78: 상태 업데이트 패턴을 더 명확하게 개선할 수 있습니다.Line 72에서
myGroups.find()로 이전 상태에서 그룹을 찾고 있는데, Line 63-69의setMyGroups가 아직 반영되지 않았을 수 있습니다. 현재는data와 병합하여 동작하지만, 더 명확한 패턴은 모든 필요한 정보를data에서 직접 구성하는 것입니다.다음과 같이 리팩토링을 고려하세요:
const updatedGroup = myGroups.find((g) => g.id === editingGroupId); - const newGroupInfo = { ...updatedGroup, ...data } as Group; + const newGroupInfo: Group = { + id: editingGroupId, + count: updatedGroup?.count ?? 0, + ...data, + };src/pages/corkagemap/list/EditGroup.tsx (1)
64-70: 유효성 검증 피드백을 개선하는 것을 고려하세요.
alert()를 사용한 유효성 검증은 기능적으로 작동하지만, 인라인 에러 메시지나 토스트 알림이 더 나은 사용자 경험을 제공합니다. Line 60의isValid체크로 Line 162의 버튼을 비활성화하므로, 이 alert는 실제로는 방어 코드입니다.
alert()를 제거하거나, 인라인 에러 메시지 UI로 교체하는 것을 고려하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/pages/corkagemap/list/edit.svgis excluded by!**/*.svgsrc/pages/corkagemap/list/trash.svgis excluded by!**/*.svg
📒 Files selected for processing (4)
src/pages/corkagemap/list/ConfirmationModal.tsx(1 hunks)src/pages/corkagemap/list/EditGroup.tsx(1 hunks)src/pages/corkagemap/list/List.tsx(1 hunks)src/pages/corkagemap/list/MyGroup.tsx(1 hunks)
🧰 Additional context used
🪛 GitHub Check: build-test
src/pages/corkagemap/list/EditGroup.tsx
[warning] 42-42:
'data' is defined but never used. Allowed unused args must match /^_$/u
src/pages/corkagemap/list/MyGroup.tsx
[warning] 41-41:
'id' is defined but never used. Allowed unused args must match /^_$/u
[warning] 40-40:
'id' is defined but never used. Allowed unused args must match /^_$/u
🔇 Additional comments (5)
src/pages/corkagemap/list/MyGroup.tsx (1)
84-105: 이전 리뷰 코멘트가 해결되었습니다.편집 및 삭제 버튼이 이제 적절한
onClick핸들러를 가지고 있으며, 각각onEdit(id)와onDelete(id)를 호출합니다. 메뉴가 닫히고 부모 컴포넌트에 올바르게 알림을 전달합니다.src/pages/corkagemap/list/ConfirmationModal.tsx (2)
46-74: 이전 리뷰 코멘트가 해결되었습니다.모달이 이제
modeprop을 받아 생성/편집/삭제 상황에 맞는 메시지를 표시합니다. Switch 문을 사용한 구현이 명확하고 확장 가능합니다.
59-60: 아이콘 및 색상 처리가 안전합니다.
iconName이 맵에 없는 경우를 조건부 렌더링과 fallback 값으로 적절히 처리하고 있습니다.Also applies to: 88-88
src/pages/corkagemap/list/EditGroup.tsx (2)
87-97: 입력 길이 제한 구현이 우수합니다.
onChange에서 최대 길이를 강제하고 실시간 카운터를 제공하는 것은 좋은 UX 패턴입니다.
159-169: 버튼 위치 계산을 확인하세요.Line 159의 주석에는 "680px / 852px = 79.8% -> ~80vh"라고 되어 있지만, Line 168에서는
75vh를 사용하고 있습니다. 이것이 의도적인 조정인지 확인이 필요합니다.다양한 화면 크기에서 버튼 위치가 적절한지 테스트해보세요.
|
음 savemarker가 svg내부에 겹치는게 많아서 하나의 파일로 만들고 색을 props로 받으면 유지보수에 좋지 않을까? 라는 고민을 했었는데 몇몇 marker는 안에가 좀 복잡하게 생겨서 힘들거 같네요... 아모튼 고생했어유! |
음 비슷하게 했던거같은데 초반에 제가 marker svg 따로 한 파일에 몰아넣고 import를 하나만해서 말씀하신대로 꺼내쓰려고했는데...! 왠지모르게 잘 안되서... 해당 부분은 추후 유지보수 단계에서 최우선 수정사안으로 고려해보겠습니다...! |
📝 작업 내용
📢 PR Point
이미지 첨부
PR.mp4
🔧 다음 할 일
Summary by CodeRabbit
새로운 기능
의존성
✏️ Tip: You can customize this high-level summary in your review settings.