[FEAT] BottomSheet 컴포넌트, 스토리북 #41
Conversation
Walkthrough이 변경 사항은 Changes
Possibly related issues
Possibly related PRs
Suggested labels
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
Update: 2024년 11월 15일 19시 42분 26초 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (10)
src/components/common/BottomSheet/BottomSheet.types.ts (2)
3-6: Props 타입에 대한 문서화가 필요합니다.특히
resetTrigger프로퍼티의 용도와 사용 방법에 대한 설명이 있으면 좋을 것 같습니다. JSDoc 주석을 추가하는 것을 추천드립니다.다음과 같이 문서화를 추가해보세요:
+/** + * BottomSheet 컴포넌트의 Props 타입 + * @property {number} resetTrigger - BottomSheet를 초기 상태로 되돌리는 트리거 값 + * @property {ReactNode} children - BottomSheet 내부에 렌더링될 컨텐츠 + */ export type Props = ComponentPropsWithoutRef<'div'> & { resetTrigger?: number; children?: ReactNode; };
8-10: DragInfo 타입의 개선을 제안드립니다.좀 더 명확한 타입 정의와 문서화가 필요해 보입니다.
다음과 같은 개선을 제안드립니다:
+/** + * 드래그 동작에 대한 정보를 담는 타입 + * @property {Object} delta - 드래그로 인한 좌표 변화량 + */ export type DragInfo = { - delta: { x: number; y: number }; + delta: { + /** X축 방향으로의 변화량 (픽셀 단위) */ + x: number; + /** Y축 방향으로의 변화량 (픽셀 단위) */ + y: number; + }; };src/components/common/BottomSheet/BottomSheet.stories.tsx (2)
1-8: ESLint 규칙 비활성화에 대한 설명이 필요합니다.
no-restricted-exports규칙을 비활성화한 이유에 대한 주석 설명을 추가하면 좋겠습니다. 이는 코드 유지보수성을 향상시키는데 도움이 될 것입니다.-/* eslint-disable no-restricted-exports */ +/* eslint-disable no-restricted-exports */ +// Storybook의 메타 설정을 위해 default export가 필요합니다.
18-41: 스토리 구현의 개선이 필요합니다.현재 구현에서 다음 사항들을 개선하면 좋겠습니다:
- 접근성 속성 추가 (aria-label, role 등)
- 다양한 상태 (로딩, 에러) 테스트 케이스 추가
- 실제 사용 사례를 반영한 더 복잡한 컨텐츠 구현
예시 구현:
<Button size="medium" onClick={handleClickBottomSheet} variant="primary"> Open BottomSheet </Button> -<BottomSheet resetTrigger={resetTrigger}> +<BottomSheet + resetTrigger={resetTrigger} + aria-label="bottom sheet content" + role="dialog" +> <div className="flex flex-col gap-6 items-center"> - <div>Contents</div> + <h2 className="text-lg font-bold">바텀시트 제목</h2> + <div className="space-y-4"> + <p>실제 사용될 수 있는 예시 컨텐츠입니다.</p> + <ul className="list-disc pl-4"> + <li>메뉴 항목 1</li> + <li>메뉴 항목 2</li> + </ul> + </div> </div> </BottomSheet>src/hooks/common/useBottomSheet.tsx (5)
6-10: 상태 관리가 깔끔하게 구현되었습니다.상태와 ref의 분리가 명확하며, 이름이 직관적입니다. 다만, 타입 안정성을 위해 명시적인 타입 선언을 추가하는 것을 고려해보세요.
- const dragOffsetRef = useRef(0); - const initialPositionRef = useRef(initialHeight); + const dragOffsetRef = useRef<number>(0); + const initialPositionRef = useRef<number>(initialHeight);
12-18: 리셋 로직에 대한 문서화가 필요합니다.리셋 로직은 잘 구현되어 있지만,
resetTrigger의 용도와 언제 사용되어야 하는지에 대한 설명이 있으면 좋겠습니다.+ // resetTrigger가 변경되거나 initialHeight가 변경될 때 바텀시트를 초기 상태로 리셋합니다. useEffect(() => {
20-34: 드래그 핸들러의 개선이 필요합니다.현재 구현은 기본적인 기능은 제공하지만, 다음 사항들을 고려해보시기 바랍니다:
- 드래그 속도에 따른 관성 스크롤
- 터치 이벤트 취소 처리
- 드래그 범위 제한 (최대 높이)
const handleDrag = useCallback( (_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => { if (isInteractionDisabled) return; + // 드래그 속도 계산 + const velocity = info.velocity.y; + const dragAmount = -info.delta.y; dragOffsetRef.current += dragAmount; + const maxHeight = window.innerHeight * 0.8; // 최대 높이를 화면의 80%로 제한 const newHeight = Math.max( - initialPositionRef.current + dragOffsetRef.current, + Math.min( + initialPositionRef.current + dragOffsetRef.current, + maxHeight + ), minVisibleHeight ); setSheetHeight(newHeight); }, - [isInteractionDisabled, minVisibleHeight] + [isInteractionDisabled, minVisibleHeight, initialPositionRef] );
36-41: 드래그 종료 시 애니메이션 처리가 필요합니다.현재는 바로 숨김 처리가 되는데, 부드러운 사용자 경험을 위해 애니메이션을 추가하는 것이 좋겠습니다.
const handleDragEnd = useCallback(() => { if (sheetHeight <= minVisibleHeight) { + // 애니메이션과 함께 숨김 처리 + const animation = { + height: 0, + opacity: 0, + transition: { duration: 0.3 } + }; setIsHidden(true); setIsInteractionDisabled(true); } }, [sheetHeight, minVisibleHeight]);
43-49: 리셋 함수에 에러 처리가 필요합니다.현재 구현은 기본적인 리셋 기능은 잘 동작하지만, 예외 상황에 대한 처리가 없습니다.
const resetSheet = useCallback(() => { + try { setSheetHeight(initialHeight); setIsHidden(false); setIsInteractionDisabled(false); dragOffsetRef.current = 0; initialPositionRef.current = initialHeight; + } catch (error) { + console.error('바텀시트 리셋 중 오류 발생:', error); + // 기본값으로 폴백 + setSheetHeight(150); + setIsHidden(true); + } }, [initialHeight]);src/components/common/BottomSheet/index.tsx (1)
8-10: 스타일 커스터마이징 옵션 추가 제안현재 구현은 잘 작동하지만, 재사용성을 높이기 위해 다음과 같은 개선을 고려해보세요:
- padding 값을 props로 받아 커스터마이징 가능하도록 수정
- text 색상을 props로 받아 유연하게 변경 가능하도록 수정
-const Content = ({ children }: React.PropsWithChildren) => ( - <div className="w-full text-black p-6">{children}</div> +interface ContentProps extends React.PropsWithChildren { + padding?: string; + textColor?: string; +} + +const Content = ({ children, padding = "p-6", textColor = "text-black" }: ContentProps) => ( + <div className={`w-full ${textColor} ${padding}`}>{children}</div> );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (6)
.pnp.cjs(4 hunks)package.json(1 hunks)src/components/common/BottomSheet/BottomSheet.stories.tsx(1 hunks)src/components/common/BottomSheet/BottomSheet.types.ts(1 hunks)src/components/common/BottomSheet/index.tsx(1 hunks)src/hooks/common/useBottomSheet.tsx(1 hunks)
🔇 Additional comments (8)
.pnp.cjs (2)
82-82: 패키지 의존성이 올바르게 추가되었습니다.
react-use-measure 패키지가 프로젝트 의존성으로 정상적으로 추가되었습니다.
Also applies to: 8163-8163
6585-6593: debounce 패키지가 적절하게 구성되었습니다.
debounce 패키지가 올바른 버전(1.2.1)으로 추가되었으며, 의존성 설정이 정확합니다.
src/components/common/BottomSheet/BottomSheet.types.ts (1)
1-1: 깔끔한 import 구문입니다!
필요한 타입만 정확하게 import하여 사용하고 있습니다.
src/components/common/BottomSheet/BottomSheet.stories.tsx (1)
9-16: 메타 설정이 잘 구성되어 있습니다!
TypeScript의 satisfies 연산자를 사용하여 타입 안정성을 보장하고, autodocs 태그를 포함하여 자동 문서화를 지원하는 등 스토리북의 모범 사례를 잘 따르고 있습니다.
src/hooks/common/useBottomSheet.tsx (2)
1-5: 훅 시그니처가 잘 설계되었습니다!
매개변수의 기본값이 적절하게 설정되어 있으며, 필요한 의존성들이 올바르게 임포트되어 있습니다.
51-52: 반환값이 잘 구성되어 있습니다!
필요한 모든 상태와 핸들러가 명확하게 노출되어 있습니다.
src/components/common/BottomSheet/index.tsx (1)
1-7: 코드 구조가 깔끔하고 적절합니다!
필요한 의존성들이 잘 정리되어 있으며, 성능 최적화를 위한 memo의 사용이 적절합니다.
package.json (1)
37-37: 의존성 추가가 적절해 보입니다.
react-use-measure는 DOM 요소의 크기와 위치를 측정하는데 유용한 라이브러리이며, BottomSheet 컴포넌트 구현에 적절한 선택입니다.
보안 검사를 실행하여 새로운 의존성의 안전성을 확인하세요:
✅ Verification successful
react-use-measure 의존성이 안전하게 확인되었습니다
이 패키지는 MIT 라이선스를 사용하며, 정기적으로 유지보수되고 있습니다. 단 하나의 의존성(debounce)만을 가지고 있어 안전성 측면에서 우수합니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: 새로운 의존성의 보안 취약점 확인
yarn npm audit react-use-measure
Length of output: 249
Script:
#!/bin/bash
# Check package security with npm audit
npm audit
# Get detailed information about react-use-measure
npm view react-use-measure
Length of output: 982
.pnp.cjs
Outdated
| ["react-use-measure", [\ | ||
| ["npm:2.1.1", {\ | ||
| "packageLocation": "./.yarn/cache/react-use-measure-npm-2.1.1-7e53301142-77b035189d.zip/node_modules/react-use-measure/",\ | ||
| "packageDependencies": [\ | ||
| ["react-use-measure", "npm:2.1.1"]\ | ||
| ],\ | ||
| "linkType": "SOFT"\ | ||
| }],\ | ||
| ["virtual:496772d107f3d2c371d44f7464355fcd2fcfd9b1940f6002dce91963b0955dbca515f91fecb79ab9255c9de181f2db37be71afcdc7b32ce3d95b36cadf9d8518#npm:2.1.1", {\ | ||
| "packageLocation": "./.yarn/__virtual__/react-use-measure-virtual-110e8e1c71/0/cache/react-use-measure-npm-2.1.1-7e53301142-77b035189d.zip/node_modules/react-use-measure/",\ | ||
| "packageDependencies": [\ | ||
| ["react-use-measure", "virtual:496772d107f3d2c371d44f7464355fcd2fcfd9b1940f6002dce91963b0955dbca515f91fecb79ab9255c9de181f2db37be71afcdc7b32ce3d95b36cadf9d8518#npm:2.1.1"],\ | ||
| ["@types/react", "npm:18.3.10"],\ | ||
| ["@types/react-dom", "npm:18.3.0"],\ | ||
| ["debounce", "npm:1.2.1"],\ | ||
| ["react", "npm:18.3.1"],\ | ||
| ["react-dom", "virtual:496772d107f3d2c371d44f7464355fcd2fcfd9b1940f6002dce91963b0955dbca515f91fecb79ab9255c9de181f2db37be71afcdc7b32ce3d95b36cadf9d8518#npm:18.3.1"]\ | ||
| ],\ | ||
| "packagePeers": [\ | ||
| "@types/react-dom",\ | ||
| "@types/react",\ | ||
| "react-dom",\ | ||
| "react"\ | ||
| ],\ | ||
| "linkType": "HARD"\ | ||
| }]\ | ||
| ]],\ |
There was a problem hiding this comment.
💡 Codebase verification
react-use-measure 패키지 설정이 올바르지만 현재 사용되지 않고 있습니다.
패키지 설정은 다음과 같이 올바르게 되어 있으나:
package.json에 의존성으로 등록됨 (v2.1.1)yarn.lock에서 필요한 peer 의존성 확인됨 (react >= 16.13, react-dom >= 16.13)
현재 코드베이스에서 이 패키지를 실제로 사용하는 곳이 없습니다. 불필요한 의존성을 제거하는 것이 좋습니다.
🔗 Analysis chain
react-use-measure의 peer 의존성이 올바르게 설정되었습니다.
react-use-measure 패키지의 설정이 다음과 같이 적절하게 구성되었습니다:
- 필수 peer 의존성: React, ReactDOM
- 타입 의존성: @types/react, @types/react-dom
- 추가 의존성: debounce
다음 스크립트를 실행하여 패키지의 사용을 확인하겠습니다:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of react-use-measure in the codebase
# Expected: Find components using the measure hook
# Search for imports of react-use-measure
echo "Checking react-use-measure imports:"
rg "from ['|\"]react-use-measure" -l
# Search for useMeasure hook usage
echo -e "\nChecking useMeasure hook usage:"
rg "useMeasure\(" -l
Length of output: 223
Script:
#!/bin/bash
# Let's verify if the package is actually installed and used in the project
# Check package.json for react-use-measure
echo "Checking package.json for react-use-measure:"
fd "package.json" --exec cat {} | jq 'select(.dependencies["react-use-measure"] or .devDependencies["react-use-measure"] or .peerDependencies["react-use-measure"])'
# Check yarn.lock for react-use-measure
echo -e "\nChecking yarn.lock for react-use-measure entries:"
rg "react-use-measure@" yarn.lock -A 5
# Check for any files that might contain the package name
echo -e "\nChecking for any files containing react-use-measure:"
rg "react-use-measure" -l
Length of output: 3566
| className={`absolute top-0 left-0 w-full h-full transition-opacity duration-300 ${ | ||
| sheetHeight > 10 ? 'opacity-70 pointer-events-auto' : 'opacity-0 pointer-events-none' | ||
| }`} | ||
| onClick={resetSheet} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
백드롭 투명도 전환 로직 개선 필요
현재 백드롭의 투명도 전환이 하드코딩된 값(10)에 의존하고 있습니다. 이는 버그를 유발할 수 있으며, 의도를 파악하기 어렵게 만듭니다.
-sheetHeight > 10 ? 'opacity-70 pointer-events-auto' : 'opacity-0 pointer-events-none'
+sheetHeight > minVisibleHeight * 0.2 ? 'opacity-70 pointer-events-auto' : 'opacity-0 pointer-events-none'Committable suggestion skipped: line range outside the PR's diff.
| const initialHeight = 150; | ||
| const minVisibleHeight = 60; | ||
| const dragControls = useDragControls(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
매직 넘버를 설정 가능한 props로 변경 필요
initialHeight와 minVisibleHeight가 하드코딩되어 있습니다. 이 값들을 props로 받아 컴포넌트의 유연성을 높이는 것이 좋겠습니다.
-export const BottomSheet = memo(({ resetTrigger = 0, children }: Props) => {
- const initialHeight = 150;
- const minVisibleHeight = 60;
+export const BottomSheet = memo(({
+ resetTrigger = 0,
+ initialHeight = 150,
+ minVisibleHeight = 60,
+ children
+}: Props & {
+ initialHeight?: number;
+ minVisibleHeight?: number;
+}) => {📝 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.
| const initialHeight = 150; | |
| const minVisibleHeight = 60; | |
| const dragControls = useDragControls(); | |
| export const BottomSheet = memo(({ | |
| resetTrigger = 0, | |
| initialHeight = 150, | |
| minVisibleHeight = 60, | |
| children | |
| }: Props & { | |
| initialHeight?: number; | |
| minVisibleHeight?: number; | |
| }) => { | |
| const dragControls = useDragControls(); |
| return ( | ||
| <> | ||
| <div | ||
| className={`absolute top-0 left-0 w-full h-full transition-opacity duration-300 ${ | ||
| sheetHeight > 10 ? 'opacity-70 pointer-events-auto' : 'opacity-0 pointer-events-none' | ||
| }`} | ||
| onClick={resetSheet} | ||
| /> | ||
| <AnimatePresence> | ||
| {!isHidden && ( | ||
| <motion.div | ||
| className="absolute bottom-0 left-0 w-full bg-white shadow-[0px_-4px_10px_0px_rgba(0,0,0,0.1)] rounded-t-3xl p-4 overflow-hidden" | ||
| style={{ height: sheetHeight }} | ||
| drag="y" | ||
| dragControls={dragControls} | ||
| dragElastic={0} | ||
| dragConstraints={{ top: 0, bottom: 0 }} | ||
| onDrag={handleDrag} | ||
| onDragEnd={handleDragEnd} | ||
| animate={{ height: sheetHeight, opacity: isHidden ? 0 : 1 }} | ||
| transition={{ type: 'spring', stiffness: 170, damping: 30, duration: 0.3 }} | ||
| onPointerDown={(e) => !isInteractionDisabled && dragControls.start(e)} | ||
| exit={{ height: 0, opacity: 0 }} | ||
| > | ||
| <div className="w-20 h-1.5 bg-primary mx-auto rounded-full" /> | ||
| <Content>{children}</Content> | ||
| </motion.div> | ||
| )} | ||
| </AnimatePresence> | ||
| </> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
접근성 개선이 필요합니다
현재 구현에는 다음과 같은 접근성 관련 개선사항들이 필요합니다:
- ARIA 속성 추가
- 키보드 네비게이션 지원
- 스크린 리더 지원
<motion.div
+ role="dialog"
+ aria-modal="true"
+ aria-label="바텀 시트"
className="absolute bottom-0 left-0 w-full bg-white shadow-[0px_-4px_10px_0px_rgba(0,0,0,0.1)] rounded-t-3xl p-4 overflow-hidden"
+ onKeyDown={(e) => {
+ if (e.key === 'Escape') {
+ resetSheet();
+ }
+ }}
+ tabIndex={0}또한 성능 최적화를 위해 다음 사항들을 고려해보세요:
AnimatePresence컴포넌트를 별도의 컴포넌트로 분리- 백드롭 컴포넌트를 메모이제이션
Committable suggestion skipped: line range outside the PR's diff.
| import { ComponentPropsWithoutRef, ReactNode } from 'react'; | ||
|
|
||
| export type Props = ComponentPropsWithoutRef<'div'> & { | ||
| resetTrigger?: number; |
There was a problem hiding this comment.
resetTrigger 값이 0인 경우 BottomSheet가 close 상태인건가요??
There was a problem hiding this comment.
resetTrigger가 0인 경우가 BottomSheet의 상태를 나타내는 것은 아니고 사이드메뉴의 즐겨찾기 버튼을 한 번도 누르지 않은 초기 상태를 말합니다
resetTrigger 값이 바뀔 때마다(버튼을 누를 때마다) BottomSheet가 initialHeight(=150)으로 맞춰지며 BottomSheet가 열리게 됩니다
There was a problem hiding this comment.
그럼 0보다는 boolean이 더 적합하지 않나요? 숫자형으로 하신 이유가 궁금합니다!
There was a problem hiding this comment.
엇 맞아요 그래서 요건 지금 boolean으로 관리하고 있습니다!
| {!isHidden && ( | ||
| <motion.div | ||
| className="absolute bottom-0 left-0 w-full bg-white shadow-[0px_-4px_10px_0px_rgba(0,0,0,0.1)] rounded-t-3xl p-4 overflow-hidden" | ||
| style={{ height: sheetHeight }} |
There was a problem hiding this comment.
style과 tailwindCSS 둘 다 사용하신 이유가 있으신가욥??
There was a problem hiding this comment.
sheetHeight은 드래그에 따라 동적으로 변하는 값이어서 tailwind를 통해 적용하기 어려워, style 속성을 따로 적용했습니다
There was a problem hiding this comment.
아하 JIT로 tailwind도 동적으로 적용 가능한 것 확인했습니다!
그렇지만 바텀시트 특성상 드래그에 따라 sheetHeight 값이 자주 변하고, css 속성 중 height 값만 변하기 때문에 동적 클래스로 적용하는 않는 편이 성능이나 유지보수에서 더 나을 것이라고 생각하는데 그대로 두어도 괜찮을까요?
There was a problem hiding this comment.
height만 동적으로 변경하면 되지 않을까요?? 성능이나 유지보수 어디에 문제가 생기는지 궁금합니당
There was a problem hiding this comment.
문제가 생긴다기 보다..! tailwind에서 매번 새로운 동적 클래스를 생성하는 성능보다 style에서 직접 다루는 것이 더 나을 거라고 생각했습니당
큰 차이가 없다면 통일성을 위해서 tailwind로 바꿀까요?
There was a problem hiding this comment.
어짜피 계산된 값이 들어갈텐데, style속성에 넣어도 새로운 값을 업데이트 하는건 똑같지 않나요??
There was a problem hiding this comment.
저의 생각은 굳이 style,tailwind를 나눌 필요는 없어 보여요! 유지보수 측면에서도 css가 나눠져 있는 것 보단 하나에서 관리 하는게 좋을 것 같아요. 실제 성능상 유의미한 변화가 있는 지 체크 해보시면 좋을 것 같습니다~!
There was a problem hiding this comment.
아하 알겠습니다! 확인해보고 수정해서 올리겠습니당
| <AnimatePresence> | ||
| {!isHidden && ( | ||
| <motion.div | ||
| className="absolute bottom-0 left-0 w-full bg-white shadow-[0px_-4px_10px_0px_rgba(0,0,0,0.1)] rounded-t-3xl p-4 overflow-hidden" |
There was a problem hiding this comment.
shadow 값이 혹시 적용이 안돼서 요렇게 하신건가요?!
shadow-[0px_-4px_10px_0px_rgba(0,0,0,0.1)]
There was a problem hiding this comment.
넵 기본 tailwind shadow 설정을 적용했을 때는 그림자가 잘 보이지 않아서 커스텀 값 넣었습니다
| drag="y" | ||
| dragControls={dragControls} | ||
| dragElastic={0} | ||
| dragConstraints={{ top: 0, bottom: 0 }} |
There was a problem hiding this comment.
요 속성들이 다 적용되고 있는게 맞나요??
불필요한 속성이 있다면 지워도 될 것 같아요.
There was a problem hiding this comment.
네 이 속성들은 다 적용되고 있습니당
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (7)
src/hooks/common/useBottomSheet.tsx (4)
1-4: import 구문의 타입 안전성 개선 필요
DragInfo타입과 상수들의 import 구문이 잘 구성되어 있습니다. 하지만 타입 안전성을 더욱 강화하기 위해 다음과 같은 개선을 제안합니다.-import { DragInfo } from '@/components/common/BottomSheet/BottomSheet.types'; +import type { DragInfo } from '@/components/common/BottomSheet/BottomSheet.types';
6-11: 상태 관리 로직 최적화 제안상태 관리가 잘 구현되어 있지만, 성능 최적화를 위해 다음 사항들을 고려해보시기 바랍니다:
isHidden과isInteractionDisabled는 서로 연관되어 있으므로 하나의 상태로 통합할 수 있습니다.- ref 값들의 초기화를 useMemo로 최적화할 수 있습니다.
-const [isHidden, setIsHidden] = useState(false); -const [isInteractionDisabled, setIsInteractionDisabled] = useState(false); +const [sheetState, setSheetState] = useState<'visible' | 'hidden' | 'disabled'>('visible'); -const dragOffsetRef = useRef(0); -const initialPositionRef = useRef(INITIAL_HEIGHT); +const refs = useMemo(() => ({ + dragOffset: { current: 0 }, + initialPosition: { current: INITIAL_HEIGHT } +}), []);
46-52: 리셋 함수의 타입 안전성 강화 필요리셋 함수가 기본적인 기능은 잘 수행하고 있으나, 타입 안전성과 에러 처리를 강화할 필요가 있습니다.
-const resetSheet = useCallback(() => { +const resetSheet = useCallback((): void => { + try { setSheetHeight(INITIAL_HEIGHT); setIsHidden(false); setIsInteractionDisabled(false); dragOffsetRef.current = 0; initialPositionRef.current = INITIAL_HEIGHT; + } catch (error) { + console.error('바텀 시트 리셋 중 오류 발생:', error); + // 에러 처리 로직 추가 + } }, []);
54-55: 반환값 타입 명시 필요반환되는 객체의 타입이 명시되어 있지 않습니다. 타입 안전성을 위해 명시적인 타입 정의가 필요합니다.
+interface BottomSheetHookReturn { + sheetHeight: number; + isHidden: boolean; + isInteractionDisabled: boolean; + handleDrag: (event: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => void; + handleDragEnd: () => void; + resetSheet: () => void; +} -return { sheetHeight, isHidden, isInteractionDisabled, handleDrag, handleDragEnd, resetSheet }; +return { + sheetHeight, + isHidden, + isInteractionDisabled, + handleDrag, + handleDragEnd, + resetSheet +} as BottomSheetHookReturn;src/components/common/BottomSheet/index.tsx (3)
1-6: 임포트 구조 최적화 제안임포트 구문을 다음과 같이 구조화하면 코드의 가독성과 성능이 향상될 것 같습니다:
- 외부 라이브러리
- 내부 커스텀 훅
- 타입 정의
또한 AnimatePresence를 동적 임포트로 변경하면 초기 번들 크기를 줄일 수 있습니다.
-import { motion, useDragControls, AnimatePresence } from 'framer-motion'; +import { motion, useDragControls } from 'framer-motion'; +const AnimatePresence = dynamic(() => import('framer-motion').then(mod => mod.AnimatePresence)); +import dynamic from 'next/dynamic';
8-10: Content 컴포넌트 개선 제안Content 컴포넌트의 유연성과 재사용성을 높이기 위한 제안사항입니다:
- className prop을 추가하여 외부에서 스타일 커스터마이징이 가능하도록 함
- padding과 색상값을 props로 받아 설정 가능하도록 함
-const Content = ({ children }: React.PropsWithChildren) => ( - <div className="w-full text-black p-6">{children}</div> +type ContentProps = React.PropsWithChildren<{ + className?: string; + padding?: string; + textColor?: string; +}>; + +const Content = ({ + children, + className = '', + padding = 'p-6', + textColor = 'text-black' +}: ContentProps) => ( + <div className={`w-full ${padding} ${textColor} ${className}`}> + {children} + </div> );
12-54: 에러 처리와 로딩 상태 관리 개선 필요컴포넌트의 안정성을 높이기 위해 다음 사항들을 추가하면 좋을 것 같습니다:
- ErrorBoundary 추가
- 로딩 상태 처리
- drag 이벤트 핸들러에서의 에러 처리
// BottomSheetErrorBoundary.tsx export class BottomSheetErrorBoundary extends React.Component { static getDerivedStateFromError(error: Error) { return { hasError: true }; } componentDidCatch(error: Error, errorInfo: React.ErrorInfo) { console.error('BottomSheet 에러:', error, errorInfo); } render() { if (this.state.hasError) { return <div>바텀 시트를 불러오는 중 문제가 발생했습니다.</div>; } return this.props.children; } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
src/components/common/BottomSheet/index.tsx(1 hunks)src/constants/bottomSheetOptions.ts(1 hunks)src/hooks/common/useBottomSheet.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/constants/bottomSheetOptions.ts
| export const BottomSheet = memo(({ children, resetTrigger = 0 }: Props) => { | ||
| const dragControls = useDragControls(); | ||
|
|
||
| const { sheetHeight, isHidden, isInteractionDisabled, handleDrag, handleDragEnd, resetSheet } = | ||
| useBottomSheet(resetTrigger); | ||
|
|
||
| return ( | ||
| <> | ||
| <div | ||
| className={`absolute top-0 left-0 w-full h-full transition-opacity duration-300 ${ | ||
| isHidden ? 'opacity-0 pointer-events-none' : 'opacity-70 pointer-events-auto' | ||
| }`} | ||
| onClick={() => { | ||
| if (!isInteractionDisabled) { | ||
| resetSheet(); | ||
| } | ||
| }} | ||
| /> | ||
|
|
||
| <AnimatePresence> | ||
| {!isHidden && ( | ||
| <motion.div | ||
| className="absolute bottom-0 left-0 w-full bg-white shadow-[0px_-4px_10px_0px_rgba(0,0,0,0.1)] rounded-t-3xl p-4 overflow-hidden" | ||
| style={{ height: sheetHeight }} | ||
| drag="y" | ||
| dragControls={dragControls} | ||
| dragElastic={0} | ||
| dragConstraints={{ top: 0, bottom: 0 }} | ||
| onDrag={handleDrag} | ||
| onDragEnd={handleDragEnd} | ||
| animate={{ height: sheetHeight, opacity: isHidden ? 0 : 1 }} | ||
| transition={{ type: 'spring', stiffness: 170, damping: 30, duration: 0.3 }} | ||
| onPointerDown={(e) => !isInteractionDisabled && dragControls.start(e)} | ||
| exit={{ height: 0, opacity: 0 }} | ||
| > | ||
| <div className="w-20 h-1.5 bg-primary mx-auto rounded-full" /> | ||
| <Content>{children}</Content> | ||
| </motion.div> | ||
| )} | ||
| </AnimatePresence> | ||
| </> | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
접근성과 설정 개선이 필요합니다
현재 구현에 다음과 같은 개선사항들이 필요해 보입니다:
-
접근성 관련:
- ARIA 속성 추가
- 키보드 제어 지원
- 스크린 리더 지원
-
설정 관련:
- 애니메이션 설정값들을 props로 받아 커스터마이징 가능하도록 개선
- 백드롭 색상과 투명도를 props로 설정 가능하도록 변경
export const BottomSheet = memo(({
children,
- resetTrigger = 0
+ resetTrigger = 0,
+ stiffness = 170,
+ damping = 30,
+ duration = 0.3,
+ backdropOpacity = 0.7
}: Props) => {
// ...
return (
<>
<div
className={`absolute top-0 left-0 w-full h-full transition-opacity duration-300 ${
- isHidden ? 'opacity-0 pointer-events-none' : 'opacity-70 pointer-events-auto'
+ isHidden ? 'opacity-0 pointer-events-none' : `opacity-${backdropOpacity * 100} pointer-events-auto`
}`}
onClick={() => {
if (!isInteractionDisabled) {
resetSheet();
}
}}
/>
<AnimatePresence>
{!isHidden && (
<motion.div
+ role="dialog"
+ aria-modal="true"
+ aria-label="바텀 시트"
className="absolute bottom-0 left-0 w-full bg-white shadow-[0px_-4px_10px_0px_rgba(0,0,0,0.1)] rounded-t-3xl p-4 overflow-hidden"
style={{ height: sheetHeight }}
drag="y"
dragControls={dragControls}
dragElastic={0}
dragConstraints={{ top: 0, bottom: 0 }}
onDrag={handleDrag}
onDragEnd={handleDragEnd}
animate={{ height: sheetHeight, opacity: isHidden ? 0 : 1 }}
- transition={{ type: 'spring', stiffness: 170, damping: 30, duration: 0.3 }}
+ transition={{ type: 'spring', stiffness, damping, duration }}
+ onKeyDown={(e) => {
+ if (e.key === 'Escape' && !isInteractionDisabled) {
+ resetSheet();
+ }
+ }}
+ tabIndex={0}
onPointerDown={(e) => !isInteractionDisabled && dragControls.start(e)}
exit={{ height: 0, opacity: 0 }}
>📝 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.
| export const BottomSheet = memo(({ children, resetTrigger = 0 }: Props) => { | |
| const dragControls = useDragControls(); | |
| const { sheetHeight, isHidden, isInteractionDisabled, handleDrag, handleDragEnd, resetSheet } = | |
| useBottomSheet(resetTrigger); | |
| return ( | |
| <> | |
| <div | |
| className={`absolute top-0 left-0 w-full h-full transition-opacity duration-300 ${ | |
| isHidden ? 'opacity-0 pointer-events-none' : 'opacity-70 pointer-events-auto' | |
| }`} | |
| onClick={() => { | |
| if (!isInteractionDisabled) { | |
| resetSheet(); | |
| } | |
| }} | |
| /> | |
| <AnimatePresence> | |
| {!isHidden && ( | |
| <motion.div | |
| className="absolute bottom-0 left-0 w-full bg-white shadow-[0px_-4px_10px_0px_rgba(0,0,0,0.1)] rounded-t-3xl p-4 overflow-hidden" | |
| style={{ height: sheetHeight }} | |
| drag="y" | |
| dragControls={dragControls} | |
| dragElastic={0} | |
| dragConstraints={{ top: 0, bottom: 0 }} | |
| onDrag={handleDrag} | |
| onDragEnd={handleDragEnd} | |
| animate={{ height: sheetHeight, opacity: isHidden ? 0 : 1 }} | |
| transition={{ type: 'spring', stiffness: 170, damping: 30, duration: 0.3 }} | |
| onPointerDown={(e) => !isInteractionDisabled && dragControls.start(e)} | |
| exit={{ height: 0, opacity: 0 }} | |
| > | |
| <div className="w-20 h-1.5 bg-primary mx-auto rounded-full" /> | |
| <Content>{children}</Content> | |
| </motion.div> | |
| )} | |
| </AnimatePresence> | |
| </> | |
| ); | |
| }); | |
| export const BottomSheet = memo(({ | |
| children, | |
| resetTrigger = 0, | |
| stiffness = 170, | |
| damping = 30, | |
| duration = 0.3, | |
| backdropOpacity = 0.7 | |
| }: Props) => { | |
| const dragControls = useDragControls(); | |
| const { sheetHeight, isHidden, isInteractionDisabled, handleDrag, handleDragEnd, resetSheet } = | |
| useBottomSheet(resetTrigger); | |
| return ( | |
| <> | |
| <div | |
| className={`absolute top-0 left-0 w-full h-full transition-opacity duration-300 ${ | |
| isHidden ? 'opacity-0 pointer-events-none' : `opacity-${backdropOpacity * 100} pointer-events-auto` | |
| }`} | |
| onClick={() => { | |
| if (!isInteractionDisabled) { | |
| resetSheet(); | |
| } | |
| }} | |
| /> | |
| <AnimatePresence> | |
| {!isHidden && ( | |
| <motion.div | |
| role="dialog" | |
| aria-modal="true" | |
| aria-label="바텀 시트" | |
| className="absolute bottom-0 left-0 w-full bg-white shadow-[0px_-4px_10px_0px_rgba(0,0,0,0.1)] rounded-t-3xl p-4 overflow-hidden" | |
| style={{ height: sheetHeight }} | |
| drag="y" | |
| dragControls={dragControls} | |
| dragElastic={0} | |
| dragConstraints={{ top: 0, bottom: 0 }} | |
| onDrag={handleDrag} | |
| onDragEnd={handleDragEnd} | |
| animate={{ height: sheetHeight, opacity: isHidden ? 0 : 1 }} | |
| transition={{ type: 'spring', stiffness, damping, duration }} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Escape' && !isInteractionDisabled) { | |
| resetSheet(); | |
| } | |
| }} | |
| tabIndex={0} | |
| onPointerDown={(e) => !isInteractionDisabled && dragControls.start(e)} | |
| exit={{ height: 0, opacity: 0 }} | |
| > | |
| <div className="w-20 h-1.5 bg-primary mx-auto rounded-full" /> | |
| <Content>{children}</Content> | |
| </motion.div> | |
| )} | |
| </AnimatePresence> | |
| </> | |
| ); | |
| }); |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (9)
src/components/common/BottomSheet/BottomSheet.stories.tsx (2)
1-1: eslint-disable 주석에 대한 설명이 필요합니다eslint 규칙을 비활성화하는 이유에 대한 설명을 주석으로 추가해 주세요. 이는 다른 개발자들의 이해를 돕고 코드 유지보수성을 향상시킬 수 있습니다.
-/* eslint-disable no-restricted-exports */ +/* eslint-disable no-restricted-exports */ +// default export가 필요한 이유: Storybook 설정 파일은 default export를 요구함
30-32: 스토리북 예제 컨텐츠 개선이 필요합니다현재 하드코딩된 "Contents ..." 대신, BottomSheet의 실제 사용 사례를 보여주는 의미 있는 예제 컨텐츠를 추가해 주세요.
- <div className="flex flex-col gap-6 items-center"> - <div>Contents ...</div> - </div> + <div className="flex flex-col gap-6 items-center p-4"> + <h2 className="text-lg font-bold">즐겨찾기 추가</h2> + <div className="w-full"> + <label className="block mb-2">카테고리 선택</label> + <select className="w-full p-2 border rounded"> + <option>학습</option> + <option>운동</option> + <option>취미</option> + </select> + </div> + <button className="w-full p-2 bg-primary text-white rounded"> + 추가하기 + </button> + </div>src/components/common/BottomSheet/index.tsx (4)
11-13: Content 컴포넌트의 타입 정의 개선 필요현재 React.PropsWithChildren을 사용하고 있지만, 더 구체적인 타입 정의가 필요해 보입니다. 스타일링 관련 props나 추가적인 커스터마이징을 위한 props를 고려해보세요.
-const Content = ({ children }: React.PropsWithChildren) => ( +type ContentProps = { + className?: string; + style?: React.CSSProperties; +} & React.PropsWithChildren; + +const Content = ({ children, className = '', style }: ContentProps) => ( - <div className="w-full text-black p-6">{children}</div> + <div className={`w-full text-black p-6 ${className}`} style={style}>{children}</div>
15-20: 커스텀 훅 매개변수 최적화 필요
useBottomSheet훅이 컴포넌트 내부에서만 사용되는데resetTrigger를 매개변수로 받고 있습니다. 내부 상태로 관리하는 것이 더 적절해 보입니다.-export const BottomSheet = memo(({ children, resetTrigger = false }: Props) => { +export const BottomSheet = memo(({ children }: Omit<Props, 'resetTrigger'>) => { const dragControls = useDragControls(); - const { sheetHeight, isHidden, isInteractionDisabled, handleDrag, handleDragEnd, resetSheet } = - useBottomSheet(resetTrigger); + const { sheetHeight, isHidden, isInteractionDisabled, handleDrag, handleDragEnd, resetSheet } = + useBottomSheet();
41-42: 애니메이션 설정값 상수화 필요애니메이션 관련 설정값들이 하드코딩되어 있습니다. 이러한 값들은 상수로 분리하여 관리하는 것이 좋습니다.
+const ANIMATION_CONFIG = { + stiffness: 170, + damping: 30, + duration: 0.3, +} as const; -transition={{ type: 'spring', stiffness: 170, damping: 30, duration: 0.3 }} +transition={{ type: 'spring', ...ANIMATION_CONFIG }}
55-72: 데모 컴포넌트 개선 필요현재 데모 구현이 매우 기본적입니다. 다음과 같은 개선사항을 고려해보세요:
- 다양한 사용 사례 추가
- 에러 상황 처리
- 로딩 상태 표시
export const BottomSheetDemo = () => { const [isOpen, setIsOpen] = useState(false); + const [isLoading, setIsLoading] = useState(false); + const [error, setError] = useState<Error | null>(null); - const handleClickBottomSheet = () => setIsOpen(!isOpen); + const handleClickBottomSheet = async () => { + try { + setIsLoading(true); + setError(null); + // 실제 데이터 로딩 로직 추가 + setIsOpen(!isOpen); + } catch (err) { + setError(err instanceof Error ? err : new Error('알 수 없는 에러가 발생했습니다.')); + } finally { + setIsLoading(false); + } + }; return ( <> <Button variant="primary" size="medium" + disabled={isLoading} onClick={handleClickBottomSheet} > - Open BottomSheet + {isLoading ? '로딩 중...' : 'Open BottomSheet'} </Button> + {error && <div className="text-red-500 mt-2">{error.message}</div>} <BottomSheet resetTrigger={isOpen}> <div className="flex flex-col gap-6 items-center"> - <div>BottomSheet Content</div> + {isLoading ? ( + <div>데이터를 불러오는 중...</div> + ) : ( + <div>BottomSheet Content</div> + )} </div> </BottomSheet> </> ); };src/hooks/common/useBottomSheet.tsx (3)
7-15: 타입 정의 추가 필요상태 변수들의 타입 안정성을 높이기 위해 명시적인 타입 정의를 추가하는 것이 좋습니다.
- const [sheetHeight, setSheetHeight] = useState(INITIAL_HEIGHT); - const [isHidden, setIsHidden] = useState(false); - const [isInteractionDisabled, setIsInteractionDisabled] = useState(false); + const [sheetHeight, setSheetHeight] = useState<number>(INITIAL_HEIGHT); + const [isHidden, setIsHidden] = useState<boolean>(false); + const [isInteractionDisabled, setIsInteractionDisabled] = useState<boolean>(false);
17-26: useEffect 정리(cleanup) 함수 추가 고려컴포넌트가 언마운트될 때 상태를 정리하는 cleanup 함수를 추가하면 좋을 것 같습니다.
useEffect(() => { if (resetTrigger) { setSheetHeight(INITIAL_HEIGHT); setIsHidden(false); setIsInteractionDisabled(false); dragOffsetRef.current = 0; initialPositionRef.current = INITIAL_HEIGHT; dragStartTimeRef.current = null; } + return () => { + dragOffsetRef.current = 0; + initialPositionRef.current = INITIAL_HEIGHT; + dragStartTimeRef.current = null; + }; }, [resetTrigger]);
56-76: 매직 넘버 상수화 필요velocity 계산에 사용되는 100과 같은 매직 넘버를 의미 있는 상수로 추출하면 좋을 것 같습니다.
+const VELOCITY_MULTIPLIER = 100; +const ANIMATION_DURATION = 0.3; const handleDragEnd = useCallback(() => { const velocity = lastVelocityRef.current; - const projectedHeight = sheetHeight + velocity * 100; + const projectedHeight = sheetHeight + velocity * VELOCITY_MULTIPLIER; // ... snap points logic ... animate(sheetHeight, nearestSnapPoint, { - duration: 0.3, + duration: ANIMATION_DURATION, ease: 'easeOut', // ... rest of the animation config }); }, [sheetHeight]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
src/components/common/BottomSheet/BottomSheet.stories.tsx(1 hunks)src/components/common/BottomSheet/BottomSheet.types.ts(1 hunks)src/components/common/BottomSheet/index.tsx(1 hunks)src/hooks/common/useBottomSheet.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/common/BottomSheet/BottomSheet.types.ts
🔇 Additional comments (2)
src/hooks/common/useBottomSheet.tsx (2)
1-5: import 구문이 깔끔하게 구성되어 있습니다!
필요한 모든 의존성이 명확하게 정의되어 있으며, 타입과 상수가 적절한 위치에서 임포트되어 있습니다.
87-88: 반환값이 잘 구성되어 있습니다!
필요한 모든 상태값과 핸들러가 명확하게 반환되고 있습니다.
| {isOpen && ( | ||
| <BottomSheet resetTrigger={isOpen}> | ||
| <div className="flex flex-col gap-6 items-center"> | ||
| <div>Contents ...</div> | ||
| </div> | ||
| </BottomSheet> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
BottomSheet 렌더링 로직 개선이 필요합니다
-
조건부 렌더링(
{isOpen && ...})은 애니메이션 처리에 문제를 일으킬 수 있습니다. BottomSheet가 닫힐 때 애니메이션이 완료되기 전에 컴포넌트가 제거될 수 있습니다. -
resetTriggerprop의 용도와 네이밍이 명확하지 않습니다. 이전 리뷰에서 논의된 대로 boolean 타입으로 변경되었지만, 더 명확한 prop 이름이 필요해 보입니다.
- {isOpen && (
- <BottomSheet resetTrigger={isOpen}>
+ <BottomSheet
+ isOpen={isOpen}
+ onClose={() => setIsOpen(false)}
+ >
<div className="flex flex-col gap-6 items-center">
<div>Contents ...</div>
</div>
- </BottomSheet>
- )}
+ </BottomSheet>Committable suggestion skipped: line range outside the PR's diff.
src/hooks/common/useBottomSheet.tsx
Outdated
| const handleDrag = useCallback( | ||
| (_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => { | ||
| if (isInteractionDisabled) return; | ||
|
|
||
| if (!dragStartTimeRef.current) { | ||
| dragStartTimeRef.current = Date.now(); | ||
| lastDragPositionRef.current = info.delta.y; | ||
| } | ||
|
|
||
| requestAnimationFrame(() => { | ||
| const dragAmount = -info.delta.y; | ||
| dragOffsetRef.current += dragAmount; | ||
|
|
||
| const velocity = | ||
| (lastDragPositionRef.current - info.delta.y) / | ||
| (Date.now() - (dragStartTimeRef.current || Date.now())); | ||
| lastVelocityRef.current = velocity; | ||
|
|
||
| const newHeight = Math.min( | ||
| Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT), | ||
| MAX_HEIGHT | ||
| ); | ||
| setSheetHeight(newHeight); | ||
| }); | ||
| }, | ||
| [isInteractionDisabled] | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
에러 처리 로직 추가 필요
드래그 핸들러에서 발생할 수 있는 예외 상황에 대한 처리가 필요합니다.
const handleDrag = useCallback(
(_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => {
if (isInteractionDisabled) return;
+
+ try {
if (!dragStartTimeRef.current) {
dragStartTimeRef.current = Date.now();
lastDragPositionRef.current = info.delta.y;
}
requestAnimationFrame(() => {
const dragAmount = -info.delta.y;
dragOffsetRef.current += dragAmount;
const velocity =
(lastDragPositionRef.current - info.delta.y) /
(Date.now() - (dragStartTimeRef.current || Date.now()));
lastVelocityRef.current = velocity;
const newHeight = Math.min(
Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT),
MAX_HEIGHT
);
setSheetHeight(newHeight);
});
+ } catch (error) {
+ console.error('드래그 처리 중 오류 발생:', error);
+ resetSheet();
+ }
},
[isInteractionDisabled]
);Committable suggestion skipped: line range outside the PR's diff.
src/hooks/common/useBottomSheet.tsx
Outdated
| const resetSheet = useCallback(() => { | ||
| setSheetHeight(INITIAL_HEIGHT); | ||
| setIsHidden(false); | ||
| setIsInteractionDisabled(false); | ||
| dragOffsetRef.current = 0; | ||
| initialPositionRef.current = INITIAL_HEIGHT; | ||
| dragStartTimeRef.current = null; | ||
| }, []); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
코드 중복 제거 필요
resetSheet 함수의 로직이 useEffect의 로직과 중복됩니다. 공통 함수로 추출하면 좋을 것 같습니다.
+const resetSheetState = () => {
+ setSheetHeight(INITIAL_HEIGHT);
+ setIsHidden(false);
+ setIsInteractionDisabled(false);
+ dragOffsetRef.current = 0;
+ initialPositionRef.current = INITIAL_HEIGHT;
+ dragStartTimeRef.current = null;
+};
useEffect(() => {
- if (resetTrigger) {
- setSheetHeight(INITIAL_HEIGHT);
- setIsHidden(false);
- setIsInteractionDisabled(false);
- dragOffsetRef.current = 0;
- initialPositionRef.current = INITIAL_HEIGHT;
- dragStartTimeRef.current = null;
- }
+ if (resetTrigger) resetSheetState();
}, [resetTrigger]);
-const resetSheet = useCallback(() => {
- setSheetHeight(INITIAL_HEIGHT);
- setIsHidden(false);
- setIsInteractionDisabled(false);
- dragOffsetRef.current = 0;
- initialPositionRef.current = INITIAL_HEIGHT;
- dragStartTimeRef.current = null;
-}, []);
+const resetSheet = useCallback(resetSheetState, []);📝 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.
| const resetSheet = useCallback(() => { | |
| setSheetHeight(INITIAL_HEIGHT); | |
| setIsHidden(false); | |
| setIsInteractionDisabled(false); | |
| dragOffsetRef.current = 0; | |
| initialPositionRef.current = INITIAL_HEIGHT; | |
| dragStartTimeRef.current = null; | |
| }, []); | |
| const resetSheetState = () => { | |
| setSheetHeight(INITIAL_HEIGHT); | |
| setIsHidden(false); | |
| setIsInteractionDisabled(false); | |
| dragOffsetRef.current = 0; | |
| initialPositionRef.current = INITIAL_HEIGHT; | |
| dragStartTimeRef.current = null; | |
| }; | |
| useEffect(() => { | |
| if (resetTrigger) resetSheetState(); | |
| }, [resetTrigger]); | |
| const resetSheet = useCallback(resetSheetState, []); |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
src/components/common/BottomSheet/BottomSheet.types.ts (2)
3-17: Props 타입에 대한 개선 제안Props 타입이 잘 정의되어 있지만, 몇 가지 개선사항을 제안드립니다:
- children의 타입을 더 구체적으로 제한하는 것이 좋습니다.
- BottomSheet의 높이 제약조건을 타입으로 정의하면 좋을 것 같습니다.
다음과 같이 개선해보세요:
export type Props = ComponentPropsWithoutRef<'div'> & { isOpen: boolean; onClose?: VoidFunction; - children?: ReactNode; + children?: ReactNode | JSX.Element; + minHeight?: number; + maxHeight?: number; };
19-21: DragInfo 타입에 대한 유효성 검사 추가 제안드래그 좌표값에 대한 제약조건이 없어 예상치 못한 동작이 발생할 수 있습니다.
다음과 같이 개선해보세요:
export type DragInfo = { delta: { x: number; y: number; /** * 드래그 방향이 유효한지 확인하는 메서드 */ isValidDrag?: () => boolean; }; };src/hooks/common/useBottomSheet.tsx (2)
7-15: 훅 사용법에 대한 문서화가 필요합니다.이 훅의 사용 방법과 각 상태값의 의미를 설명하는 JSDoc 문서를 추가하면 좋을 것 같습니다.
예시:
+/** + * BottomSheet의 상태와 동작을 관리하는 훅 + * @param isOpen - BottomSheet의 초기 표시 여부 + * @returns {Object} BottomSheet 제어에 필요한 상태와 핸들러들 + */ export const useBottomSheet = (isOpen: boolean) => {
85-88: 반환 타입 정의가 필요합니다.반환값의 타입을 명시적으로 정의하면 타입 안정성과 문서화에 도움이 될 것 같습니다.
다음과 같은 인터페이스 정의를 제안합니다:
interface UseBottomSheetReturn { sheetHeight: number; isHidden: boolean; isInteractionDisabled: boolean; handleDrag: (event: PointerEvent | TouchEvent, info: DragInfo) => void; handleDragEnd: () => void; resetSheet: () => void; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
src/components/common/BottomSheet/BottomSheet.stories.tsx(1 hunks)src/components/common/BottomSheet/BottomSheet.types.ts(1 hunks)src/components/common/BottomSheet/index.tsx(1 hunks)src/hooks/common/useBottomSheet.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/common/BottomSheet/BottomSheet.stories.tsx
- src/components/common/BottomSheet/index.tsx
🔇 Additional comments (3)
src/components/common/BottomSheet/BottomSheet.types.ts (1)
1-21: 전반적인 타입 정의 검토
타입 정의가 전반적으로 잘 되어있고 JSDoc 문서화도 잘 되어있습니다. 다만, 이전 리뷰에서 언급된 resetTrigger가 현재 코드에서 제거된 것으로 보이는데, 이 변경사항이 의도된 것인지 확인이 필요합니다.
관련 코드를 확인하기 위해 다음 스크립트를 실행해보세요:
✅ Verification successful
resetTrigger 제거는 의도된 리팩토링입니다
resetTrigger 속성이 isOpen 속성으로 대체되었고, 관련 로직이 더 명확하게 개선되었습니다. 현재 구현에서는:
isOpen속성을 통해 바텀시트의 상태를 직접적으로 제어useBottomSheet훅에서isOpen값의 변경을 감지하여 상태를 초기화- 불필요한
resetTrigger로직이 제거되어 코드가 더 간결해짐
이는 컴포넌트의 사용성과 가독성을 향상시키는 긍정적인 변경사항입니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# resetTrigger 관련 코드 검색
rg "resetTrigger" --type ts --type tsx
Length of output: 66
Script:
#!/bin/bash
# resetTrigger 관련 코드 검색 (수정된 버전)
rg "resetTrigger"
# BottomSheet 컴포넌트 관련 파일 검색
fd "BottomSheet" --type f
# BottomSheet 컴포넌트 구현 코드 확인
ast-grep --pattern 'const BottomSheet = $_'
Length of output: 4025
Script:
#!/bin/bash
# useBottomSheet 훅 구현 확인
cat src/hooks/common/useBottomSheet.tsx
# 이전 커밋에서 resetTrigger 관련 코드 검색
git log -p | rg "resetTrigger"
Length of output: 5395
src/hooks/common/useBottomSheet.tsx (2)
1-6: 의존성 관리가 잘 되어있습니다!
상수와 타입을 별도의 파일로 분리하여 관리하는 것이 좋은 구조입니다.
17-28: 상태 초기화 로직이 잘 구현되어 있습니다!
resetSheetState 함수를 재사용하여 코드 중복을 방지한 점이 좋습니다.
src/hooks/common/useBottomSheet.tsx
Outdated
| const handleDrag = useCallback( | ||
| (_: PointerEvent, info: DragInfo) => { | ||
| if (isInteractionDisabled) return; | ||
|
|
||
| try { | ||
| if (!dragStartTimeRef.current) { | ||
| dragStartTimeRef.current = Date.now(); | ||
| lastDragPositionRef.current = info.delta.y; | ||
| } | ||
|
|
||
| requestAnimationFrame(() => { | ||
| const dragAmount = -info.delta.y; | ||
| dragOffsetRef.current += dragAmount; | ||
|
|
||
| const velocity = | ||
| (lastDragPositionRef.current - info.delta.y) / | ||
| (Date.now() - (dragStartTimeRef.current || Date.now())); | ||
| lastVelocityRef.current = velocity; | ||
|
|
||
| const newHeight = Math.min( | ||
| Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT), | ||
| MAX_HEIGHT | ||
| ); | ||
| setSheetHeight(newHeight); | ||
| }); | ||
| } catch (error) { | ||
| console.error('드래그 중 에러 발생:', error); | ||
| resetSheetState(); | ||
| } | ||
| }, | ||
| [isInteractionDisabled] | ||
| ); | ||
|
|
||
| const handleDragEnd = useCallback(() => { | ||
| const velocity = lastVelocityRef.current; | ||
| const projectedHeight = sheetHeight + velocity * 100; | ||
|
|
||
| const snapPoints = [MIN_VISIBLE_HEIGHT, INITIAL_HEIGHT, MAX_HEIGHT]; | ||
| const nearestSnapPoint = snapPoints.reduce((prev, curr) => | ||
| Math.abs(curr - projectedHeight) < Math.abs(prev - projectedHeight) ? curr : prev | ||
| ); | ||
|
|
||
| animate(sheetHeight, nearestSnapPoint, { | ||
| duration: 0.3, | ||
| ease: 'easeOut', | ||
| onUpdate: (value) => setSheetHeight(value), | ||
| onComplete: () => { | ||
| if (nearestSnapPoint === MIN_VISIBLE_HEIGHT) { | ||
| setIsHidden(true); | ||
| setIsInteractionDisabled(true); | ||
| } | ||
| }, | ||
| }); | ||
| }, [sheetHeight]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
모바일 디바이스 지원 개선이 필요합니다.
현재 PointerEvent만 처리하고 있는데, 일부 모바일 브라우저에서는 Touch 이벤트도 지원해야 할 수 있습니다.
다음과 같이 수정을 제안합니다:
- (_: PointerEvent, info: DragInfo) => {
+ (_: PointerEvent | TouchEvent, info: DragInfo) => {또한 터치 이벤트에 대한 passive 이벤트 리스너 설정을 고려해보세요:
// BottomSheet 컴포넌트에서 사용 시
useEffect(() => {
document.addEventListener('touchmove', preventDefault, { passive: false });
return () => document.removeEventListener('touchmove', preventDefault);
}, []);
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
src/hooks/common/useBottomSheet.tsx (5)
1-4: 이벤트 타입 정의 개선 필요이벤트 타입을 유니온 타입으로 사용하는 것보다 제네릭을 활용하여 타입 안전성을 높일 수 있습니다.
-import { DragInfo } from '@/components/common/BottomSheet/BottomSheet.types'; +import { DragEvent, DragInfo } from '@/components/common/BottomSheet/BottomSheet.types'; +type DragEventType = MouseEvent | TouchEvent | PointerEvent; +type DragHandlerType = (event: DragEventType, info: DragInfo) => void;
6-11: 상태와 ref 타입 명시 필요상태와 ref의 타입을 명시적으로 선언하면 타입 안전성이 향상됩니다.
-export const useBottomSheet = (resetTrigger: boolean) => { +interface BottomSheetState { + sheetHeight: number; + isHidden: boolean; + isInteractionDisabled: boolean; +} +export const useBottomSheet = (resetTrigger: boolean) => { const [sheetHeight, setSheetHeight] = useState<number>(INITIAL_HEIGHT); const [isHidden, setIsHidden] = useState<boolean>(false); const [isInteractionDisabled, setIsInteractionDisabled] = useState<boolean>(false); - const dragOffsetRef = useRef(0); - const initialPositionRef = useRef(INITIAL_HEIGHT); + const dragOffsetRef = useRef<number>(0); + const initialPositionRef = useRef<number>(INITIAL_HEIGHT);
23-37: 드래그 핸들러 개선 제안현재 구현은 간단하고 직관적이지만, Framer Motion의
dragElastic속성을 활용하여 사용자 경험을 개선할 수 있습니다.// BottomSheet 컴포넌트에서 사용 시: <motion.div drag="y" dragElastic={0.2} onDrag={handleDrag} onDragEnd={handleDragEnd} dragConstraints={{ top: 0, bottom: MAX_HEIGHT - MIN_VISIBLE_HEIGHT }} > {children} </motion.div>
39-46: 에러 바운더리 추가 고려드래그 종료 시 예상치 못한 상태 변경이 발생할 수 있습니다. 컴포넌트 레벨에서 에러 바운더리를 추가하는 것이 좋습니다.
// ErrorBoundary 컴포넌트 구현 예시: class BottomSheetErrorBoundary extends React.Component { static getDerivedStateFromError(error) { return { hasError: true }; } componentDidCatch(error, errorInfo) { // 에러 로깅 console.error('BottomSheet 에러:', error, errorInfo); } render() { if (this.state.hasError) { return <div>바텀시트를 불러오는데 문제가 발생했습니다.</div>; } return this.props.children; } }
56-57: 반환 타입 명시 필요훅의 반환 타입을 명시적으로 정의하면 타입 안전성과 문서화가 개선됩니다.
+interface UseBottomSheetReturn { + sheetHeight: number; + isHidden: boolean; + isInteractionDisabled: boolean; + handleDrag: DragHandlerType; + handleDragEnd: () => void; + resetSheet: () => void; +} -return { sheetHeight, isHidden, isInteractionDisabled, handleDrag, handleDragEnd, resetSheet }; +return { + sheetHeight, + isHidden, + isInteractionDisabled, + handleDrag, + handleDragEnd, + resetSheet +} as UseBottomSheetReturn;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
src/components/common/BottomSheet/index.tsx(1 hunks)src/hooks/common/useBottomSheet.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/common/BottomSheet/index.tsx
src/hooks/common/useBottomSheet.tsx
Outdated
| useEffect(() => { | ||
| if (resetTrigger) { | ||
| setSheetHeight(INITIAL_HEIGHT); | ||
| setIsHidden(false); | ||
| setIsInteractionDisabled(false); | ||
| dragOffsetRef.current = 0; | ||
| initialPositionRef.current = INITIAL_HEIGHT; | ||
| } | ||
| }, [resetTrigger]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
초기화 로직 중복 제거 필요
useEffect와 resetSheet 함수에서 초기화 로직이 중복되어 있습니다. 공통 함수로 추출하여 재사용성을 높이는 것이 좋습니다.
+const resetSheetState = () => ({
+ sheetHeight: INITIAL_HEIGHT,
+ isHidden: false,
+ isInteractionDisabled: false,
+ dragOffset: 0,
+ initialPosition: INITIAL_HEIGHT,
+});
useEffect(() => {
if (resetTrigger) {
- setSheetHeight(INITIAL_HEIGHT);
- setIsHidden(false);
- setIsInteractionDisabled(false);
- dragOffsetRef.current = 0;
- initialPositionRef.current = INITIAL_HEIGHT;
+ const { sheetHeight, isHidden, isInteractionDisabled, dragOffset, initialPosition } = resetSheetState();
+ setSheetHeight(sheetHeight);
+ setIsHidden(isHidden);
+ setIsInteractionDisabled(isInteractionDisabled);
+ dragOffsetRef.current = dragOffset;
+ initialPositionRef.current = initialPosition;
}
}, [resetTrigger]);📝 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.
| useEffect(() => { | |
| if (resetTrigger) { | |
| setSheetHeight(INITIAL_HEIGHT); | |
| setIsHidden(false); | |
| setIsInteractionDisabled(false); | |
| dragOffsetRef.current = 0; | |
| initialPositionRef.current = INITIAL_HEIGHT; | |
| } | |
| }, [resetTrigger]); | |
| const resetSheetState = () => ({ | |
| sheetHeight: INITIAL_HEIGHT, | |
| isHidden: false, | |
| isInteractionDisabled: false, | |
| dragOffset: 0, | |
| initialPosition: INITIAL_HEIGHT, | |
| }); | |
| useEffect(() => { | |
| if (resetTrigger) { | |
| const { sheetHeight, isHidden, isInteractionDisabled, dragOffset, initialPosition } = resetSheetState(); | |
| setSheetHeight(sheetHeight); | |
| setIsHidden(isHidden); | |
| setIsInteractionDisabled(isInteractionDisabled); | |
| dragOffsetRef.current = dragOffset; | |
| initialPositionRef.current = initialPosition; | |
| } | |
| }, [resetTrigger]); |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
src/hooks/common/useBottomSheet.tsx (3)
7-11: 상태 변수들의 타입 정의가 필요합니다.상태 변수들의 목적은 명확하지만, 타입 안정성을 위해 명시적인 타입 정의를 추가하면 좋을 것 같습니다.
- const [sheetHeight, setSheetHeight] = useState(INITIAL_HEIGHT); - const [isHidden, setIsHidden] = useState(false); - const [isInteractionDisabled, setIsInteractionDisabled] = useState(false); - const dragOffsetRef = useRef(0); - const initialPositionRef = useRef(INITIAL_HEIGHT); + const [sheetHeight, setSheetHeight] = useState<number>(INITIAL_HEIGHT); + const [isHidden, setIsHidden] = useState<boolean>(false); + const [isInteractionDisabled, setIsInteractionDisabled] = useState<boolean>(false); + const dragOffsetRef = useRef<number>(0); + const initialPositionRef = useRef<number>(INITIAL_HEIGHT);
39-49: 부드러운 전환 효과 추가가 필요합니다.현재는 상태 변경이 즉각적으로 일어나는데, 사용자 경험 향상을 위해 애니메이션을 추가하면 좋을 것 같습니다.
const handleDragEnd = useCallback(() => { if (sheetHeight <= MIN_VISIBLE_HEIGHT) { - setIsHidden(true); - setIsInteractionDisabled(true); - if (onClose) { - onClose(); - } + // framer-motion의 animate 함수를 사용하여 부드러운 전환 효과 추가 + animate(sheetHeight, 0, { + duration: 0.3, + onComplete: () => { + setIsHidden(true); + setIsInteractionDisabled(true); + onClose?.(); + } + }); } else if (sheetHeight > MAX_HEIGHT) { - setSheetHeight(MAX_HEIGHT); + animate(sheetHeight, MAX_HEIGHT, { + duration: 0.3 + }); } }, [sheetHeight, onClose]);
51-63: 클린업 로직 추가가 필요합니다.상태 초기화 시 진행 중인 애니메이션이나 이벤트 리스너를 정리하는 로직이 필요합니다.
const resetSheet = useCallback(() => { + // 진행 중인 애니메이션이 있다면 중단 + if (animationRef.current) { + animationRef.current.stop(); + } + setSheetHeight(INITIAL_HEIGHT); setIsHidden(false); setIsInteractionDisabled(false); dragOffsetRef.current = 0; initialPositionRef.current = INITIAL_HEIGHT; }, []);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
src/components/common/BottomSheet/index.tsx(1 hunks)src/hooks/common/useBottomSheet.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/common/BottomSheet/index.tsx
🔇 Additional comments (3)
src/hooks/common/useBottomSheet.tsx (3)
1-6: LGTM! 깔끔한 import문과 훅 선언입니다.
필요한 의존성들이 잘 정리되어 있고, 훅의 인터페이스가 명확합니다.
13-21: 이전 리뷰 의견과 동일합니다.
초기화 로직의 중복에 대한 이슈가 이전 리뷰에서 이미 지적되었습니다.
65-74: 반환값이 잘 구성되어 있습니다.
필요한 모든 상태와 핸들러가 명확하게 정의되어 있습니다.
src/hooks/common/useBottomSheet.tsx
Outdated
| const handleDrag = useCallback( | ||
| (_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => { | ||
| if (isInteractionDisabled) return; | ||
|
|
||
| const dragAmount = -info.delta.y; | ||
| dragOffsetRef.current += dragAmount; | ||
|
|
||
| const newHeight = Math.min( | ||
| Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT), | ||
| MAX_HEIGHT | ||
| ); | ||
| setSheetHeight(newHeight); | ||
| }, | ||
| [isInteractionDisabled] | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
드래그 핸들러의 성능 최적화가 필요합니다.
잦은 상태 업데이트로 인한 성능 저하를 방지하기 위해 다음과 같은 개선을 제안합니다:
const handleDrag = useCallback(
(_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => {
if (isInteractionDisabled) return;
+
+ // requestAnimationFrame을 사용하여 렌더링 최적화
+ requestAnimationFrame(() => {
const dragAmount = -info.delta.y;
dragOffsetRef.current += dragAmount;
const newHeight = Math.min(
Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT),
MAX_HEIGHT
);
setSheetHeight(newHeight);
+ });
},
- [isInteractionDisabled]
+ [isInteractionDisabled, setSheetHeight]
);📝 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.
| const handleDrag = useCallback( | |
| (_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => { | |
| if (isInteractionDisabled) return; | |
| const dragAmount = -info.delta.y; | |
| dragOffsetRef.current += dragAmount; | |
| const newHeight = Math.min( | |
| Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT), | |
| MAX_HEIGHT | |
| ); | |
| setSheetHeight(newHeight); | |
| }, | |
| [isInteractionDisabled] | |
| ); | |
| const handleDrag = useCallback( | |
| (_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => { | |
| if (isInteractionDisabled) return; | |
| // requestAnimationFrame을 사용하여 렌더링 최적화 | |
| requestAnimationFrame(() => { | |
| const dragAmount = -info.delta.y; | |
| dragOffsetRef.current += dragAmount; | |
| const newHeight = Math.min( | |
| Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT), | |
| MAX_HEIGHT | |
| ); | |
| setSheetHeight(newHeight); | |
| }); | |
| }, | |
| [isInteractionDisabled, setSheetHeight] | |
| ); |
| /** | ||
| * Callback function that is called when the BottomSheet should close. | ||
| */ | ||
| onClose?: VoidFunction; |
There was a problem hiding this comment.
isOpen 으로 close도 관리하면 될 것 같은데 onClose함수가 필요한 이유가 따로 있나요?!
There was a problem hiding this comment.
혹시 나중에 바텀시트가 닫히는 시점에 부모 컴포넌트에서 액션이 필요한 부분이 있을까봐 부모 컴포넌트로 바텀시트 상태를 전달하려고 따로 작성했는데, 생각해보니 바텀시트가 닫혀도 다른 상태는 바뀔게 없을 것 같아서 isOpen으로 통합하겠습니다!
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (7)
src/components/common/BottomSheet/index.tsx (4)
10-10: Content 컴포넌트의 역할과 스타일링 개선 필요현재 Content 컴포넌트는 단순한 div 래퍼에 불과합니다. 컴포넌트의 목적을 명확히 하고 필요한 스타일링을 추가하거나, 불필요하다면 제거하는 것이 좋겠습니다.
12-12: 메모이제이션 사용에 대한 문서화 필요
memo를 사용하여 성능을 최적화하고 있습니다. 이 최적화가 필요한 이유와 언제 리렌더링이 발생하는지 주석으로 문서화하면 좋겠습니다.
30-30: z-index 관리 방식 개선 필요하드코딩된
z-[1000]값은 유지보수를 어렵게 만들 수 있습니다. z-index 값을 전역 상수나 테마 설정으로 관리하는 것이 좋겠습니다.
43-43: 핸들바 커스터마이징 옵션 추가 필요핸들바의 크기와 색상이 하드코딩되어 있습니다. 이를 props로 받아 커스터마이징할 수 있도록 개선하면 좋겠습니다.
- <div className="w-20 h-1.5 bg-primary mx-auto rounded-full" /> + <div + className={`${handleWidth ?? 'w-20'} ${handleHeight ?? 'h-1.5'} ${ + handleColor ?? 'bg-primary' + } mx-auto rounded-full`} + />src/hooks/common/useBottomSheet.tsx (3)
3-4: 타입과 상수 import 구조 개선 필요타입과 상수 파일의 경로가 너무 깊습니다. 접근성과 유지보수성을 높이기 위해 경로 별칭을 더 효율적으로 사용하는 것이 좋습니다.
다음과 같이 수정을 제안합니다:
-import { DragInfo } from '@/components/common/BottomSheet/BottomSheet.types'; -import { INITIAL_HEIGHT, MIN_VISIBLE_HEIGHT, MAX_HEIGHT } from '@/constants/bottomSheetOptions'; +import { DragInfo } from '@/types/bottomSheet'; +import { BOTTOM_SHEET_OPTIONS } from '@/constants';
6-12: 초기 상태 관리 개선 필요상태 변수들이 개별적으로 선언되어 있어 관리가 복잡할 수 있습니다. 또한 ref 값들의 초기화 로직이 분산되어 있습니다.
상태를 객체로 통합하고, ref 초기화를 한 곳에서 관리하는 것을 제안합니다:
+interface BottomSheetState { + height: number; + isHidden: boolean; + isInteractionDisabled: boolean; +} + +const initialState: BottomSheetState = { + height: INITIAL_HEIGHT, + isHidden: false, + isInteractionDisabled: false, +}; export const useBottomSheet = (isOpen: boolean) => { - const [sheetHeight, setSheetHeight] = useState(INITIAL_HEIGHT); - const [isHidden, setIsHidden] = useState(false); - const [isInteractionDisabled, setIsInteractionDisabled] = useState(false); + const [state, setState] = useState<BottomSheetState>(initialState); const dragOffsetRef = useRef(0); const initialPositionRef = useRef(INITIAL_HEIGHT);
50-62: 상태 초기화 로직 중복 제거 필요
resetSheet와useEffect내의 초기화 로직이 중복되어 있습니다. 공통 함수로 추출하는 것이 좋습니다.+const getInitialState = () => ({ + height: INITIAL_HEIGHT, + isHidden: false, + isInteractionDisabled: false, +}); + +const resetRefs = (refs: { + dragOffset: React.MutableRefObject<number>, + initialPosition: React.MutableRefObject<number> +}) => { + refs.dragOffset.current = 0; + refs.initialPosition.current = INITIAL_HEIGHT; +}; const resetSheet = useCallback(() => { - setSheetHeight(INITIAL_HEIGHT); - setIsHidden(false); - setIsInteractionDisabled(false); - dragOffsetRef.current = 0; - initialPositionRef.current = INITIAL_HEIGHT; + setState(getInitialState()); + resetRefs({ dragOffset: dragOffsetRef, initialPosition: initialPositionRef }); }, []);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
src/components/common/BottomSheet/BottomSheet.stories.tsx(1 hunks)src/components/common/BottomSheet/index.tsx(1 hunks)src/hooks/common/useBottomSheet.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/common/BottomSheet/BottomSheet.stories.tsx
| style={{ height: sheetHeight }} | ||
| drag="y" | ||
| dragControls={dragControls} | ||
| dragElastic={1} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
dragElastic 값 조정 필요
dragElastic={1}은 드래그 시 너무 많은 탄성을 허용할 수 있습니다. 보통 0~0.5 사이의 값을 사용하는 것이 권장됩니다. 현재 설정은 사용자 경험을 저해할 수 있습니다.
- dragElastic={1}
+ dragElastic={0.3}📝 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.
| dragElastic={1} | |
| dragElastic={0.3} |
| <div | ||
| className={`absolute top-0 left-0 w-full h-full transition-opacity duration-300 ${ | ||
| isHidden ? 'pointer-events-none' : 'pointer-events-auto' | ||
| }`} | ||
| onClick={handleClose} | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
백드롭 구현 개선 필요
현재 백드롭에 다음과 같은 개선이 필요합니다:
- 배경색이 지정되어 있지 않습니다
- 불투명도 전환만 있고 실제 불투명도 값이 설정되어 있지 않습니다
- className={`absolute top-0 left-0 w-full h-full transition-opacity duration-300 ${
- isHidden ? 'pointer-events-none' : 'pointer-events-auto'
- }`}
+ className={`absolute top-0 left-0 w-full h-full bg-black transition-opacity duration-300 ${
+ isHidden ? 'opacity-0 pointer-events-none' : 'opacity-70 pointer-events-auto'
+ }`}📝 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.
| <div | |
| className={`absolute top-0 left-0 w-full h-full transition-opacity duration-300 ${ | |
| isHidden ? 'pointer-events-none' : 'pointer-events-auto' | |
| }`} | |
| onClick={handleClose} | |
| /> | |
| <div | |
| className={`absolute top-0 left-0 w-full h-full bg-black transition-opacity duration-300 ${ | |
| isHidden ? 'opacity-0 pointer-events-none' : 'opacity-70 pointer-events-auto' | |
| }`} | |
| onClick={handleClose} | |
| /> |
src/hooks/common/useBottomSheet.tsx
Outdated
| const handleDragEnd = useCallback(() => { | ||
| if (sheetHeight <= MIN_VISIBLE_HEIGHT) { | ||
| setIsHidden(true); | ||
| setIsInteractionDisabled(true); | ||
| } else if (sheetHeight > MAX_HEIGHT) { | ||
| setSheetHeight(MAX_HEIGHT); | ||
| } | ||
| }, [sheetHeight]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
드래그 종료 처리 로직 개선 필요
이전 리뷰어의 제안대로 early return 패턴을 적용하고, 스냅 포인트 기능을 추가하면 좋을 것 같습니다.
const handleDragEnd = useCallback(() => {
if (sheetHeight <= MIN_VISIBLE_HEIGHT) {
setIsHidden(true);
setIsInteractionDisabled(true);
+ return;
}
- } else if (sheetHeight > MAX_HEIGHT) {
+ if (sheetHeight > MAX_HEIGHT) {
setSheetHeight(MAX_HEIGHT);
+ return;
}
+
+ // 스냅 포인트 구현
+ const snapPoints = [MIN_VISIBLE_HEIGHT, INITIAL_HEIGHT, MAX_HEIGHT];
+ const nearestPoint = snapPoints.reduce((prev, curr) =>
+ Math.abs(curr - sheetHeight) < Math.abs(prev - sheetHeight) ? curr : prev
+ );
+ setSheetHeight(nearestPoint);
}, [sheetHeight]);📝 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.
| const handleDragEnd = useCallback(() => { | |
| if (sheetHeight <= MIN_VISIBLE_HEIGHT) { | |
| setIsHidden(true); | |
| setIsInteractionDisabled(true); | |
| } else if (sheetHeight > MAX_HEIGHT) { | |
| setSheetHeight(MAX_HEIGHT); | |
| } | |
| }, [sheetHeight]); | |
| const handleDragEnd = useCallback(() => { | |
| if (sheetHeight <= MIN_VISIBLE_HEIGHT) { | |
| setIsHidden(true); | |
| setIsInteractionDisabled(true); | |
| return; | |
| } | |
| if (sheetHeight > MAX_HEIGHT) { | |
| setSheetHeight(MAX_HEIGHT); | |
| return; | |
| } | |
| // 스냅 포인트 구현 | |
| const snapPoints = [MIN_VISIBLE_HEIGHT, INITIAL_HEIGHT, MAX_HEIGHT]; | |
| const nearestPoint = snapPoints.reduce((prev, curr) => | |
| Math.abs(curr - sheetHeight) < Math.abs(prev - sheetHeight) ? curr : prev | |
| ); | |
| setSheetHeight(nearestPoint); | |
| }, [sheetHeight]); |
src/hooks/common/useBottomSheet.tsx
Outdated
| const handleDrag = useCallback( | ||
| (_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => { | ||
| if (isInteractionDisabled) return; | ||
|
|
||
| requestAnimationFrame(() => { | ||
| const dragAmount = -info.delta.y; | ||
| dragOffsetRef.current += dragAmount; | ||
|
|
||
| const newHeight = Math.min( | ||
| Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT), | ||
| MAX_HEIGHT | ||
| ); | ||
| setSheetHeight(newHeight); | ||
| }); | ||
| }, | ||
| [isInteractionDisabled, setSheetHeight] | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
드래그 핸들러 성능 및 사용성 개선 필요
현재 구현은 기본적인 드래그 기능만 제공합니다. 사용자 경험 향상을 위해 다음 기능들의 추가가 필요합니다.
- 드래그 방향 제한
- 드래그 저항 추가
- 성능 최적화
const handleDrag = useCallback(
(_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => {
if (isInteractionDisabled) return;
+
+ // 수직 방향으로만 드래그 허용
+ if (Math.abs(info.delta.x) > Math.abs(info.delta.y)) return;
requestAnimationFrame(() => {
const dragAmount = -info.delta.y;
- dragOffsetRef.current += dragAmount;
+ // 드래그 저항 추가
+ const resistance = 0.5;
+ const resistedDrag = dragAmount * resistance;
+ dragOffsetRef.current += resistedDrag;
const newHeight = Math.min(
Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT),
MAX_HEIGHT
);
- setSheetHeight(newHeight);
+ // 불필요한 리렌더링 방지
+ if (Math.abs(newHeight - sheetHeight) > 1) {
+ setSheetHeight(newHeight);
+ }
});
},
[isInteractionDisabled, setSheetHeight]
);📝 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.
| const handleDrag = useCallback( | |
| (_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => { | |
| if (isInteractionDisabled) return; | |
| requestAnimationFrame(() => { | |
| const dragAmount = -info.delta.y; | |
| dragOffsetRef.current += dragAmount; | |
| const newHeight = Math.min( | |
| Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT), | |
| MAX_HEIGHT | |
| ); | |
| setSheetHeight(newHeight); | |
| }); | |
| }, | |
| [isInteractionDisabled, setSheetHeight] | |
| ); | |
| const handleDrag = useCallback( | |
| (_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => { | |
| if (isInteractionDisabled) return; | |
| // 수직 방향으로만 드래그 허용 | |
| if (Math.abs(info.delta.x) > Math.abs(info.delta.y)) return; | |
| requestAnimationFrame(() => { | |
| const dragAmount = -info.delta.y; | |
| // 드래그 저항 추가 | |
| const resistance = 0.5; | |
| const resistedDrag = dragAmount * resistance; | |
| dragOffsetRef.current += resistedDrag; | |
| const newHeight = Math.min( | |
| Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT), | |
| MAX_HEIGHT | |
| ); | |
| // 불필요한 리렌더링 방지 | |
| if (Math.abs(newHeight - sheetHeight) > 1) { | |
| setSheetHeight(newHeight); | |
| } | |
| }); | |
| }, | |
| [isInteractionDisabled, setSheetHeight] | |
| ); |
| animate={{ height: sheetHeight, opacity: isHidden ? 0 : 1 }} | ||
| transition={{ type: 'spring', stiffness: 170, damping: 30, duration: 0.3 }} | ||
| onPointerDown={(e) => !isInteractionDisabled && dragControls.start(e)} | ||
| exit={{ height: 0, opacity: 0 }} |
There was a problem hiding this comment.
framer 관련 객체들 저는 constants의 motion.ts 에서 관리하는데, 따로 빼도 깔끔할것 같네요!
There was a problem hiding this comment.
아하 확인했습니다! 수정해서 올리겠습니당
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/constants/motions.ts (2)
36-39: 매개변수 유효성 검사 추가 필요sheetHeight가 음수이거나 유효하지 않은 값일 경우 예상치 못한 동작이 발생할 수 있습니다.
다음과 같이 매개변수 검증을 추가하는 것을 고려해보세요:
animate: (sheetHeight: number, isHidden: boolean) => ({ + height: Math.max(0, sheetHeight), // 음수 방지 opacity: isHidden ? 0 : 1, }),
44-49: 애니메이션 타이밍 최적화 제안현재 스프링 애니메이션 설정이 약간 급격할 수 있습니다. 더 부드러운 사용자 경험을 위해 damping 값을 조정하는 것을 고려해보세요.
다음과 같은 설정을 제안드립니다:
transition: { type: 'spring', stiffness: 170, - damping: 30, + damping: 26, duration: 0.3, },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
src/components/common/BottomSheet/index.tsx(1 hunks)src/constants/motions.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/common/BottomSheet/index.tsx
🔇 Additional comments (1)
src/constants/motions.ts (1)
35-50: 전반적인 구현이 잘 되었습니다
BOTTOM_SHEET_ANIMATION 상수가 기존 애니메이션 상수들과 일관된 패턴을 따르고 있으며, BottomSheet 컴포넌트의 요구사항을 잘 충족하고 있습니다. 특히 isHidden 플래그를 통한 투명도 제어가 효과적으로 구현되었습니다.
…date handleDragEnd
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (3)
src/components/common/BottomSheet/index.tsx (1)
37-40: 애니메이션 설정을 더 유연하게 만들어보세요현재 하드코딩된 애니메이션 값들을 props를 통해 커스터마이징할 수 있도록 개선하면 좋겠습니다.
export interface Props { children: ReactNode; isOpen: boolean; + animationConfig?: { + stiffness?: number; + damping?: number; + duration?: number; + }; } // BOTTOM_SHEET_ANIMATION 상수를 props로 받은 설정값으로 오버라이드 const animationConfig = { ...BOTTOM_SHEET_ANIMATION.transition, ...props.animationConfig, };src/hooks/common/useBottomSheet.tsx (2)
15-23: 초기화 로직을 별도 함수로 분리하면 좋을 것 같습니다.여러 곳에서 사용되는 초기화 로직을 하나의 함수로 분리하여 재사용성을 높이는 것이 좋습니다.
+const resetSheetState = () => { + setSheetHeight(INITIAL_HEIGHT); + setIsHidden(false); + setIsInteractionDisabled(false); + dragOffsetRef.current = 0; + initialPositionRef.current = INITIAL_HEIGHT; +}; useEffect(() => { - if (isOpen) { - setSheetHeight(INITIAL_HEIGHT); - setIsHidden(false); - setIsInteractionDisabled(false); - dragOffsetRef.current = 0; - initialPositionRef.current = INITIAL_HEIGHT; - } + if (isOpen) resetSheetState(); }, [isOpen]);
25-41: 터치 이벤트 최적화가 필요합니다.터치 이벤트에 passive 옵션을 추가하여 스크롤 성능을 개선할 수 있습니다.
useEffect(() => { const handleClickOutside = (event: Event) => { if (bottomSheetRef.current && !bottomSheetRef.current.contains(event.target as Node)) { setSheetHeight(INITIAL_HEIGHT); } }; if (isOpen) { document.addEventListener('mousedown', handleClickOutside); - document.addEventListener('touchstart', handleClickOutside); + document.addEventListener('touchstart', handleClickOutside, { passive: true }); } return () => { document.removeEventListener('mousedown', handleClickOutside); - document.removeEventListener('touchstart', handleClickOutside); + document.removeEventListener('touchstart', handleClickOutside); }; }, [isOpen]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
src/components/common/BottomSheet/BottomSheet.types.ts(1 hunks)src/components/common/BottomSheet/BottomSheetHeader.tsx(1 hunks)src/components/common/BottomSheet/index.tsx(1 hunks)src/hooks/common/useBottomSheet.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/common/BottomSheet/BottomSheetHeader.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/common/BottomSheet/BottomSheet.types.ts
🔇 Additional comments (2)
src/components/common/BottomSheet/index.tsx (1)
1-11: 모듈 구조가 잘 구성되어 있습니다!
상수와 타입을 별도 파일로 분리하고, Portal 컴포넌트를 활용한 점이 좋습니다.
src/hooks/common/useBottomSheet.tsx (1)
1-14: 상태 관리와 초기화가 잘 구현되어 있습니다!
필요한 훅들과 타입들이 잘 임포트되어 있으며, 바텀 시트의 상태 관리를 위한 state와 ref들이 적절하게 초기화되어 있습니다.
| return ( | ||
| <Portal isOpen={!isHidden}> | ||
| <AnimatePresence> | ||
| {!isHidden && ( | ||
| <motion.div | ||
| ref={bottomSheetRef} | ||
| className="absolute bottom-0 left-0 w-full bg-white shadow-[0px_-4px_10px_0px_rgba(0,0,0,0.1)] rounded-t-3xl p-4 overflow-hidden z-[1000] h-${sheetHeight}" | ||
| drag="y" | ||
| dragControls={dragControls} | ||
| dragElastic={0} | ||
| dragConstraints={{ top: 0, bottom: 0 }} | ||
| onDrag={handleDrag} | ||
| onDragEnd={handleDragEnd} | ||
| animate={BOTTOM_SHEET_ANIMATION.animate(sheetHeight, isHidden)} | ||
| transition={BOTTOM_SHEET_ANIMATION.transition} | ||
| onPointerDown={(e) => !isInteractionDisabled && dragControls.start(e)} | ||
| exit={BOTTOM_SHEET_ANIMATION.exit} | ||
| > | ||
| <BottomSheetHeader /> | ||
| {children} | ||
| </motion.div> | ||
| )} | ||
| </AnimatePresence> | ||
| </Portal> | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
백드롭 구현과 z-index 관리가 필요합니다
현재 구현에 다음 사항들이 누락되어 있습니다:
- 백드롭 (배경 오버레이)
- z-index 관리 상수화
<Portal isOpen={!isHidden}>
+ <div
+ className={`fixed inset-0 bg-black transition-opacity duration-300 ${
+ isHidden ? 'opacity-0 pointer-events-none' : 'opacity-70 pointer-events-auto'
+ }`}
+ onClick={() => !isInteractionDisabled && onClose?.()}
+ />
<AnimatePresence>
{!isHidden && (
<motion.div
ref={bottomSheetRef}
- className="absolute bottom-0 left-0 w-full bg-white shadow-[0px_-4px_10px_0px_rgba(0,0,0,0.1)] rounded-t-3xl p-4 overflow-hidden z-[1000] h-${sheetHeight}"
+ className="fixed bottom-0 left-0 w-full bg-white shadow-[0px_-4px_10px_0px_rgba(0,0,0,0.1)] rounded-t-3xl p-4 overflow-hidden z-[var(--z-bottomsheet)] h-${sheetHeight}"z-index 값은 상수로 관리하는 것을 추천드립니다:
// src/constants/zIndex.ts
export const Z_INDEX = {
backdrop: 999,
bottomSheet: 1000,
} as const;| export const BottomSheet = memo(({ children, isOpen }: Props) => { | ||
| const dragControls = useDragControls(); | ||
|
|
||
| const { | ||
| sheetHeight, | ||
| isHidden, | ||
| isInteractionDisabled, | ||
| handleDrag, | ||
| handleDragEnd, | ||
| bottomSheetRef, | ||
| } = useBottomSheet(isOpen); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
접근성 개선이 필요합니다
다음과 같은 접근성 관련 속성들을 추가하는 것이 좋겠습니다:
export const BottomSheet = memo(({
- children,
- isOpen
+ children,
+ isOpen,
+ ariaLabel = '바텀 시트',
+ onClose
}: Props) => {
// ...
return (
<Portal isOpen={!isHidden}>
<motion.div
+ role="dialog"
+ aria-modal="true"
+ aria-label={ariaLabel}
+ onKeyDown={(e) => {
+ if (e.key === 'Escape' && !isInteractionDisabled && onClose) {
+ onClose();
+ }
+ }}
+ tabIndex={0}Committable suggestion skipped: line range outside the PR's diff.
src/hooks/common/useBottomSheet.tsx
Outdated
| const handleDragEnd = useCallback(() => { | ||
| if (sheetHeight <= MIN_VISIBLE_HEIGHT) { | ||
| setSheetHeight(MIN_VISIBLE_HEIGHT); | ||
| setTimeout(() => { | ||
| setIsHidden(true); | ||
| setIsInteractionDisabled(true); | ||
| }, BOTTOM_SHEET_ANIMATION.transition.duration * 1000); | ||
| return; | ||
| } | ||
|
|
||
| if (sheetHeight > MAX_HEIGHT) { | ||
| setSheetHeight(MAX_HEIGHT); | ||
| return; | ||
| } | ||
| }, [sheetHeight]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
드래그 종료 처리 개선이 필요합니다.
- setTimeout 대신 AnimationEvent를 사용하면 더 안정적인 상태 업데이트가 가능합니다.
- 스냅 포인트를 추가하여 사용자 경험을 개선할 수 있습니다.
+const SNAP_POINTS = [MIN_VISIBLE_HEIGHT, INITIAL_HEIGHT, MAX_HEIGHT];
+
const handleDragEnd = useCallback(() => {
if (sheetHeight <= MIN_VISIBLE_HEIGHT) {
setSheetHeight(MIN_VISIBLE_HEIGHT);
- setTimeout(() => {
+ const handleAnimationEnd = () => {
setIsHidden(true);
setIsInteractionDisabled(true);
- }, BOTTOM_SHEET_ANIMATION.transition.duration * 1000);
+ bottomSheetRef.current?.removeEventListener('transitionend', handleAnimationEnd);
+ };
+ bottomSheetRef.current?.addEventListener('transitionend', handleAnimationEnd);
return;
}
if (sheetHeight > MAX_HEIGHT) {
setSheetHeight(MAX_HEIGHT);
return;
}
+
+ // 가장 가까운 스냅 포인트로 이동
+ const nearestPoint = SNAP_POINTS.reduce((prev, curr) =>
+ Math.abs(curr - sheetHeight) < Math.abs(prev - sheetHeight) ? curr : prev
+ );
+ setSheetHeight(nearestPoint);
}, [sheetHeight]);Committable suggestion skipped: line range outside the PR's diff.
| const handleDrag = useCallback( | ||
| (_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => { | ||
| if (isInteractionDisabled) return; | ||
|
|
||
| requestAnimationFrame(() => { | ||
| const dragAmount = -info.delta.y; | ||
| dragOffsetRef.current += dragAmount * 5; | ||
|
|
||
| const newHeight = Math.min( | ||
| Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT), | ||
| MAX_HEIGHT | ||
| ); | ||
| setSheetHeight(newHeight); | ||
| }); | ||
| }, | ||
| [isInteractionDisabled, setSheetHeight] | ||
| ); |
There was a problem hiding this comment.
드래그 동작 개선이 필요합니다.
현재 구현에서 발견된 문제점들:
- dragAmount에 임의의 값(5)을 곱하는 것은 예측하기 어려운 동작을 야기할 수 있습니다.
- 드래그 저항이 없어 사용자 경험이 부자연스러울 수 있습니다.
const handleDrag = useCallback(
(_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => {
if (isInteractionDisabled) return;
+ // 수직 방향으로만 드래그 허용
+ if (Math.abs(info.delta.x) > Math.abs(info.delta.y)) return;
+
requestAnimationFrame(() => {
const dragAmount = -info.delta.y;
- dragOffsetRef.current += dragAmount * 5;
+ // 드래그 저항 추가
+ const resistance = 0.5;
+ dragOffsetRef.current += dragAmount * resistance;
const newHeight = Math.min(
Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT),
MAX_HEIGHT
);
setSheetHeight(newHeight);
});
},
[isInteractionDisabled, setSheetHeight]
);📝 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.
| const handleDrag = useCallback( | |
| (_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => { | |
| if (isInteractionDisabled) return; | |
| requestAnimationFrame(() => { | |
| const dragAmount = -info.delta.y; | |
| dragOffsetRef.current += dragAmount * 5; | |
| const newHeight = Math.min( | |
| Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT), | |
| MAX_HEIGHT | |
| ); | |
| setSheetHeight(newHeight); | |
| }); | |
| }, | |
| [isInteractionDisabled, setSheetHeight] | |
| ); | |
| const handleDrag = useCallback( | |
| (_: MouseEvent | TouchEvent | PointerEvent, info: DragInfo) => { | |
| if (isInteractionDisabled) return; | |
| // 수직 방향으로만 드래그 허용 | |
| if (Math.abs(info.delta.x) > Math.abs(info.delta.y)) return; | |
| requestAnimationFrame(() => { | |
| const dragAmount = -info.delta.y; | |
| // 드래그 저항 추가 | |
| const resistance = 0.5; | |
| dragOffsetRef.current += dragAmount * resistance; | |
| const newHeight = Math.min( | |
| Math.max(initialPositionRef.current + dragOffsetRef.current, MIN_VISIBLE_HEIGHT), | |
| MAX_HEIGHT | |
| ); | |
| setSheetHeight(newHeight); | |
| }); | |
| }, | |
| [isInteractionDisabled, setSheetHeight] | |
| ); |
관련 이슈
close #39
📑 작업 내용
💬 리뷰 중점 사항/기타 참고 사항
Summary by CodeRabbit
새로운 기능
BottomSheet컴포넌트 및BottomSheetHeader서브 컴포넌트 추가BottomSheet의 기본 스토리 추가 및 상호작용 가능성 제공상수 추가
INITIAL_HEIGHT,MIN_VISIBLE_HEIGHT,MAX_HEIGHT상수 추가BOTTOM_SHEET_ANIMATION애니메이션 설정 추가타입 정의
Props및DragInfo타입 정의 추가문서화
BottomSheet컴포넌트 사용 예시 제공