Conversation
|
""" WalkthroughBottomSheet UI 컴포넌트가 새롭게 추가되었습니다. 컴포넌트의 스타일, 구현, Storybook 스토리, 그리고 관련 의존성(vaul)이 반영되었습니다. index 파일을 통해 컴포넌트가 외부로 공개됩니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TriggerButton
participant BottomSheet
participant VaulDrawer
User->>TriggerButton: 클릭
TriggerButton->>BottomSheet: open 상태 변경
BottomSheet->>VaulDrawer: open 상태 전달
VaulDrawer->>BottomSheet: onOpenChange 콜백
BottomSheet->>User: 하위 children, title, footer 렌더링
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested reviewers
Poem
""" Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/components/ui/BottomSheet/BottomSheet.stories.tsxOops! Something went wrong! :( ESLint: 9.27.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/eslint-config-next@15.3.2_eslint@9.27.0_jiti@2.4.2__typescript@5.8.3/node_modules/eslint-config-next/index.js". If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting. .storybook/main.tsOops! Something went wrong! :( ESLint: 9.27.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/eslint-config-next@15.3.2_eslint@9.27.0_jiti@2.4.2__typescript@5.8.3/node_modules/eslint-config-next/index.js". If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting. .storybook/vitest.setup.tsOops! Something went wrong! :( ESLint: 9.27.0 ESLint couldn't find the plugin "eslint-plugin-react-hooks". (The package "eslint-plugin-react-hooks" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react-hooks" was referenced from the config file in " » eslint-config-next/core-web-vitals » /node_modules/.pnpm/eslint-config-next@15.3.2_eslint@9.27.0_jiti@2.4.2__typescript@5.8.3/node_modules/eslint-config-next/index.js". If you still can't figure out the problem, please see https://eslint.org/docs/latest/use/troubleshooting.
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (10)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
|
🎨 Storybook Preview: https://685a32a1c0bbd269fdb67af4-jlivhkkzuz.chromatic.com/ |
|
This pull request (commit
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/components/ui/BottomSheet/BottomSheet.css.ts (1)
26-29: TODO 주석 해결 필요minHeight 값에 대한 디자이너 논의가 필요하다는 TODO 주석이 있습니다. 디자인 시스템과 일치하도록 이 값을 확정해주세요.
디자이너와의 논의 후 이 값을 업데이트하는 이슈를 생성하거나 PR을 작성해드릴까요?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
package.json(1 hunks)src/components/ui/BottomSheet/BottomSheet.css.ts(1 hunks)src/components/ui/BottomSheet/BottomSheet.stories.tsx(1 hunks)src/components/ui/BottomSheet/BottomSheet.tsx(1 hunks)src/components/ui/BottomSheet/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`{src/app/**/_components/*.{ts,tsx},src/components/**/*.{ts,tsx}}`: 컴포넌트 파일은 PascalCase로 네이밍해야 한다 (예: `Button.tsx`, `DomainLayout.tsx`).
{src/app/**/_components/*.{ts,tsx},src/components/**/*.{ts,tsx}}: 컴포넌트 파일은 PascalCase로 네이밍해야 한다 (예:Button.tsx,DomainLayout.tsx).
📄 Source: CodeRabbit Inference Engine (.cursor/rules/nextjs-folder-structure.mdc)
List of files the instruction was applied to:
src/components/ui/BottomSheet/index.tssrc/components/ui/BottomSheet/BottomSheet.tsxsrc/components/ui/BottomSheet/BottomSheet.css.tssrc/components/ui/BottomSheet/BottomSheet.stories.tsx
`**/*.css.ts`: vanilla-extract 스타일 파일은 camelCase로, `.css.ts` 확장자를 사용해야 하며, 해당 컴포넌트와 같은 폴더에 배치해야 한다.
**/*.css.ts: vanilla-extract 스타일 파일은 camelCase로,.css.ts확장자를 사용해야 하며, 해당 컴포넌트와 같은 폴더에 배치해야 한다.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/nextjs-folder-structure.mdc)
List of files the instruction was applied to:
src/components/ui/BottomSheet/BottomSheet.css.ts
🧠 Learnings (1)
src/components/ui/BottomSheet/BottomSheet.css.ts (2)
Learnt from: CR
PR: YAPP-Github/26th-Web-Team-1-FE#0
File: .cursor/rules/nextjs-folder-structure.mdc:0-0
Timestamp: 2025-06-30T08:25:30.684Z
Learning: Applies to **/*.css.ts : vanilla-extract 스타일 파일은 camelCase로, `.css.ts` 확장자를 사용해야 하며, 해당 컴포넌트와 같은 폴더에 배치해야 한다.
Learnt from: CR
PR: YAPP-Github/26th-Web-Team-1-FE#0
File: .cursor/rules/nextjs-folder-structure.mdc:0-0
Timestamp: 2025-06-30T08:25:30.684Z
Learning: 스타일 근접성: vanilla-extract 스타일 파일은 해당 컴포넌트와 같은 폴더에 배치해야 한다.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (8)
package.json (1)
28-28: vaul 패키지 최신 버전 확인됨, 보안 감사 추가 진행 필요vaul 버전 ^1.1.2는 NPM의 최신(1.1.2)과 일치합니다. 다만 보안 취약점 검사를 위해선 lockfile이 필요하므로, 아래 절차를 따라 추가 검증을 진행해주세요.
- package-lock.json 생성
npm i --package-lock-only- 보안 감사 실행
npm audit --audit-level moderate- 감사 결과 이상 여부 확인 후 병합 여부 결정
src/components/ui/BottomSheet/index.ts (1)
1-1: 올바른 barrel export 패턴 사용BottomSheet 컴포넌트의 re-export가 표준 패턴에 따라 올바르게 구현되었습니다.
src/components/ui/BottomSheet/BottomSheet.tsx (2)
29-33: 접근성 속성 적절히 구현됨aria-modal과 role="dialog" 속성이 올바르게 적용되어 접근성이 잘 고려되었습니다.
6-13: Props 타입 정의가 명확함BottomSheetProps 타입이 명확하고 적절하게 정의되었습니다. 모든 필수/선택 속성이 올바르게 표현되었습니다.
src/components/ui/BottomSheet/BottomSheet.css.ts (1)
1-4: 파일명 및 구조 규칙 준수vanilla-extract 스타일 파일이 코딩 가이드라인에 따라 camelCase와 .css.ts 확장자를 사용하며 컴포넌트와 같은 폴더에 올바르게 배치되었습니다.
src/components/ui/BottomSheet/BottomSheet.stories.tsx (3)
23-45: BottomSheetWrapper 구현이 적절함상태 관리를 위한 래퍼 컴포넌트가 Storybook 패턴에 따라 올바르게 구현되었습니다. useState를 사용한 상태 관리와 props 전달이 적절합니다.
7-18: Storybook 메타데이터 설정이 적절함컴포넌트의 Storybook 설정이 올바르게 구성되었습니다. argTypes 설정으로 적절한 컨트롤을 제공하고 있습니다.
72-77: viewMode 기반 조건부 렌더링이 적절함Docs 모드에서는 닫힌 상태로, Canvas 모드에서는 열린 상태로 표시하는 로직이 사용자 경험을 고려하여 잘 구현되었습니다.
|
This pull request (commit
|
|
This pull request (commit
|
Seojunhwan
left a comment
Seojunhwan
left a comment
There was a problem hiding this comment.
디자인이 바뀌지 않는다! 라고 하면 지금 구현을 그대로 사용해도 좋을 것 같아요!
만약 디자인이 변경될 수도 있고, 여러 상황에 대처해야 한다면 compound 패턴으로 만들어봐도 좋을 것 같아요! ex) vaul 컴포넌트 인터페이스처럼요!
|
This pull request (commit
|
|
@wkdtnqls0506 수빈님, 고생하셨습니다! conflict만 처리해주실 수 있나요?!
|
wkdtnqls0506
force-pushed
the
feature/PRODUCT-112
branch
from
0810669 to
c2ae013
Compare
2 participants
✅ 이슈 번호
close #54
🪄 작업 내용 (변경 사항)
src/components/ui/BottomSheet📸 스크린샷
2025-07-01.1.52.50.mov
💡 설명
openonOpenChangechildrentitletriggerfooter🗣️ 리뷰어에게 전달 사항
BottomSheet 버튼 관련
바텀시트 하단에 버튼이 항상 true 값이긴 한데,
버튼의 텍스트나 클릭 시 동작까지 BottomSheet가 직접 책임지는 게 맞을까?하는 고민이 들었습니다..🤔BottomSheet는
레이아웃 + 열고 닫는 동작까지만 책임지고, 버튼의 실제 로직은 사용하는 쪽에서 유연하게 받는 게 맞겠다고 판단했습니다..!그래서 하단 버튼은 footer props로 분리해서 버튼 props의 값을 외부에서 자유롭게 제어할 수 있는 구조로 구성했습니다!
BottomSheet min-height 관련
피그마 상에 정해진 값이 없는 거 같아서 일단 코멘트 남겨둔 상태입니다!
값이 정해진다면 다시 수정해서 올리겠습니다 ㅎㅎ
📍 트러블 슈팅
Summary by CodeRabbit
신규 기능
스타일
문서화
Chores