Feat: overlay-kit 패키지 적용 및 모든 Popup 컴포넌트 구현#79
Conversation
|
""" Walkthrough이번 변경은 공통 Popup UI 컴포넌트와 다양한 팝업(경고, 확인, 신고 등) 컴포넌트 추가, 관련 스타일 모듈 및 Storybook 스토리 작성, OverlayProvider 적용, z-index 토큰 확장, Webpack alias 설정, 그리고 일부 환경설정 및 의존성 추가로 구성되어 있습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant OverlayProvider
participant Popup
participant PopupVariant
User->>App: 팝업 트리거(예: 버튼 클릭)
App->>OverlayProvider: 팝업 오픈 요청
OverlayProvider->>Popup: 팝업 렌더링(open=true)
Popup->>PopupVariant: 특정 팝업(경고/확인/신고 등) 렌더링
User->>PopupVariant: 닫기/확인 등 액션
PopupVariant->>OverlayProvider: 팝업 닫기 요청
OverlayProvider->>Popup: 팝업 언마운트
Estimated code review effort4 (60–120분)
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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
Documentation and Community
|
|
This pull request (commit
|
🚀 Storybook 배포📖 Storybook: https://683d91ab23651aa0b399e435-splfxwmzie.chromatic.com/ |
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (9)
app/providers.tsx (1)
12-16: SSR 처리 방식 검토 권장현재 구현은 작동하지만, 초기 렌더링 시 빈 화면이 잠깐 보일 수 있습니다. 더 나은 사용자 경험을 위해 다음을 고려해보세요:
- const [isClient, setIsClient] = useState(false); - useEffect(() => setIsClient(true), []); - if (!isClient) return null; + const [isMounted, setIsMounted] = useState(false); + useEffect(() => setIsMounted(true), []); + if (!isMounted) return <QueryProvider>{children}</QueryProvider>;또는
dynamicimport를 사용하는 방법도 있습니다.shared/ui/popup/popup-report/index.tsx (1)
29-29: 이메일 주소를 props로 받도록 개선 권장하드코딩된 이메일 주소를 props로 받아 재사용성을 높이는 것이 좋겠습니다.
interface PopupReportProps { isOpen: boolean; close: () => void; + email?: string; } -const PopupReport = ({ isOpen, close }: PopupReportProps) => { +const PopupReport = ({ isOpen, close, email = "lettie@gmail.com" }: PopupReportProps) => { // ... - <p>lettie@gmail.com</p> + <p>{email}</p>shared/ui/popup/popup-confirm-letter/index.tsx (1)
27-27: Next.js Image 컴포넌트 사용 권장정적 이미지에 대해 Next.js의 Image 컴포넌트를 사용하면 성능 최적화를 얻을 수 있습니다.
+import Image from "next/image"; // ... -<img src={LettieCharacter.src} alt="Lettie character" /> +<Image src={LettieCharacter} alt="Lettie character" width={120} height={120} />shared/ui/popup/popup-report/popup-report.css.ts (1)
22-38: 중복된 정렬 속성 정리
captionContainer에서 중복된 정렬 속성들이 있습니다.export const captionContainer = style({ display: "flex", flexDirection: "row", justifyContent: "center", - alignContent: "center", alignItems: "center", - textAlign: "center", gap: "1rem", // ... 나머지 속성들 });
alignContent는 여러 행이 있을 때 사용되고,textAlign은 flexbox에서 중앙 정렬에는 불필요합니다.shared/ui/popup/popup-exit-lettie/popup-exit-lettie.css.ts (1)
4-7: 고정 크기 대신 반응형 디자인 고려고정된 width와 height 값이 다양한 화면 크기에서 문제가 될 수 있습니다. max-width나 clamp() 함수 사용을 검토해보세요.
shared/ui/popup/popup-intro/index.tsx (1)
21-21: 이미지 최적화 개선 고려Next.js Image 컴포넌트 사용을 고려해보세요. 성능과 접근성 면에서 더 나은 선택입니다.
+ import Image from 'next/image'; - <img src={LettieCharacter.src} alt="Lettie character" /> + <Image src={LettieCharacter} alt="레티 캐릭터" width={120} height={120} />shared/ui/popup/popup.stories.tsx (2)
77-124: 반복되는 스타일 코드 추상화 고려각 스토리에서 버튼 스타일이 반복됩니다. 공통 스타일을 추출하거나 컴포넌트로 만드는 것을 고려해보세요.
+ const StoryButton: React.FC<{ children: React.ReactNode; onClick: () => void }> = ({ children, onClick }) => ( + <button + style={{ + padding: "10px 20px", + borderRadius: "12px", + }} + onClick={onClick} + > + {children} + </button> + ); export const Default: Story = { render: () => ( - <button - style={{ - padding: "10px 20px", - borderRadius: "12px", - }} - onClick={() => { + <StoryButton onClick={() => { overlay.open(({ isOpen, close }) => ( <Popup open={isOpen}> <Popup.Title>Title</Popup.Title> <Popup.Content>description</Popup.Content> <Popup.Actions> <button onClick={close} style={{ width: "100%", color: themeVars.color.white[40], }} > Back </button> <button style={{ width: "100%", color: themeVars.color.purple[10], }} onClick={close} > Continue </button> </Popup.Actions> </Popup> )); - }} - > - Open Popup - </button> + }}> + Open Popup + </StoryButton> ),
246-252: 스토리 설명 불일치ConfirmLetter 스토리의 설명이 "편지 관련 경고를 표시하는 팝업"으로 되어있는데, 실제로는 편지 확인 팝업입니다. 정확한 설명으로 수정해주세요.
docs: { description: { - story: "편지 관련 경고를 표시하는 팝업입니다.", + story: "편지 확인을 위한 팝업입니다.", }, },shared/ui/popup/popup-intro/popup-intro.css.ts (1)
47-52: 클릭 가능한 요소에cursor: pointer추가를 고려해주세요
closeButton은 사용자가 클릭하여 팝업을 닫는 인터랙티브 요소입니다. 시각적 힌트(마우스 커서 변화)가 없으면 버튼임을 인지하기 어렵습니다.export const closeButton = style({ position: "absolute", top: "2.2rem", right: "2.2rem", color: themeVars.color.white[85], + cursor: "pointer", });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlshared/assets/character/X3.pngis excluded by!**/*.pngshared/assets/icon/close.svgis excluded by!**/*.svgshared/assets/icon/copy.svgis excluded by!**/*.svgshared/assets/pop-up/X2.pngis excluded by!**/*.png
📒 Files selected for processing (26)
.storybook/fonts.css(1 hunks).storybook/main.ts(2 hunks).storybook/preview.tsx(2 hunks)app/page.tsx(1 hunks)app/providers.tsx(1 hunks)biome.json(1 hunks)package.json(1 hunks)shared/styles/base/reset.css.ts(1 hunks)shared/styles/tokens/z-index.ts(1 hunks)shared/ui/popup/index.tsx(1 hunks)shared/ui/popup/popup-cancel-creation/index.tsx(1 hunks)shared/ui/popup/popup-cancel-creation/popup-cancel-creation.css.ts(1 hunks)shared/ui/popup/popup-confirm-letter/index.tsx(1 hunks)shared/ui/popup/popup-confirm-letter/popup-confirm-letter.css.ts(1 hunks)shared/ui/popup/popup-exit-lettie/index.tsx(1 hunks)shared/ui/popup/popup-exit-lettie/popup-exit-lettie.css.ts(1 hunks)shared/ui/popup/popup-intro/index.tsx(1 hunks)shared/ui/popup/popup-intro/popup-intro.css.ts(1 hunks)shared/ui/popup/popup-report/index.tsx(1 hunks)shared/ui/popup/popup-report/popup-report.css.ts(1 hunks)shared/ui/popup/popup-warning-capsule/index.tsx(1 hunks)shared/ui/popup/popup-warning-capsule/popup-warning-capsule.css.ts(1 hunks)shared/ui/popup/popup-warning-letter/index.tsx(1 hunks)shared/ui/popup/popup-warning-letter/popup-warning-letter.css.ts(1 hunks)shared/ui/popup/popup.css.ts(1 hunks)shared/ui/popup/popup.stories.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (11)
shared/ui/popup/popup-confirm-letter/popup-confirm-letter.css.ts (2)
shared/ui/popup/popup-cancel-creation/popup-cancel-creation.css.ts (3)
layout(4-7)title(9-13)continueButton(20-23)shared/styles/base/theme.css.ts (1)
themeVars(14-14)
app/providers.tsx (1)
shared/providers/query-provider.tsx (1)
QueryProvider(38-51)
shared/ui/popup/popup-warning-letter/popup-warning-letter.css.ts (1)
shared/styles/base/theme.css.ts (1)
themeVars(14-14)
shared/ui/popup/popup-report/popup-report.css.ts (1)
shared/styles/base/theme.css.ts (1)
themeVars(14-14)
shared/ui/popup/popup-cancel-creation/popup-cancel-creation.css.ts (2)
shared/ui/popup/popup-confirm-letter/popup-confirm-letter.css.ts (3)
layout(4-7)title(9-15)continueButton(17-20)shared/styles/base/theme.css.ts (1)
themeVars(14-14)
shared/ui/popup/popup-warning-capsule/popup-warning-capsule.css.ts (1)
shared/styles/base/theme.css.ts (1)
themeVars(14-14)
shared/ui/popup/popup.css.ts (1)
shared/styles/base/theme.css.ts (1)
themeVars(14-14)
shared/ui/popup/index.tsx (2)
shared/utils/cn.ts (1)
cn(3-5)shared/styles/base/theme.css.ts (1)
themeClass(14-14)
shared/ui/popup/popup-exit-lettie/popup-exit-lettie.css.ts (1)
shared/styles/base/theme.css.ts (1)
themeVars(14-14)
shared/ui/popup/popup.stories.tsx (1)
shared/styles/base/theme.css.ts (1)
themeVars(14-14)
shared/ui/popup/popup-intro/popup-intro.css.ts (1)
shared/styles/base/theme.css.ts (1)
themeVars(14-14)
🔇 Additional comments (20)
package.json (1)
28-28: overlay-kit 보안 감사 위해 lockfile 생성 후 재검증 필요
overlay-kit@1.8.4는 현재 NPM 상 최신 버전입니다.- 현 상태에서는
package-lock.json이 없어npm audit가 실행되지 않았습니다.- 로컬에서 아래 절차를 따라 보안 취약점 검토를 진행해주세요:
npm i --package-lock-only로package-lock.json생성npm audit실행하여 취약점 보고서 확인.storybook/fonts.css (1)
6-7: 파일 끝 개행 문자 추가 - 좋습니다!코딩 표준에 맞는 파일 포맷팅 개선입니다.
shared/styles/tokens/z-index.ts (1)
5-8: 팝업 z-index 토큰 구조가 잘 설계되었습니다.헤더(99) < 팝업 배경(100) < 팝업 콘텐츠(101)로 논리적인 레이어링 구조를 제공합니다. 향후 확장성도 고려된 좋은 설계입니다.
shared/styles/base/reset.css.ts (1)
25-25: 문법 오류 수정 완료객체 리터럴 문법을 올바르게 수정했습니다. 이 변경으로 CSS-in-JS 선언이 정상적으로 작동할 것입니다.
.storybook/preview.tsx (2)
2-2: overlay-kit 패키지 통합Storybook 환경에서 팝업 컴포넌트가 올바르게 작동하도록 OverlayProvider를 적절히 추가했습니다.
21-31: OverlayProvider 래핑 구현기존 스토리북 데코레이터를 OverlayProvider로 감싸서 팝업 컴포넌트들이 스토리북 환경에서 정상적으로 렌더링될 수 있도록 구성했습니다.
.storybook/main.ts (2)
1-1: Node.js path 모듈 import웹팩 alias 설정을 위한 path 모듈을 올바르게 import했습니다.
45-49: @shared alias 설정스토리북 환경에서 shared 컴포넌트들을 일관된 경로로 import할 수 있도록 alias를 적절히 구성했습니다. null 체크도 포함되어 안전합니다.
app/providers.tsx (2)
1-1: 클라이언트 컴포넌트 지시어 추가overlay-kit과 상태 관리를 위해 클라이언트 컴포넌트로 변경한 것이 적절합니다.
18-22: Provider 구조 구현OverlayProvider를 QueryProvider 내부에 중첩하여 팝업 컴포넌트들이 필요한 컨텍스트를 모두 사용할 수 있도록 올바르게 구성했습니다.
shared/ui/popup/popup-confirm-letter/popup-confirm-letter.css.ts (3)
4-7: 팝업 레이아웃 크기 검토높이가 38.95rem으로 상당히 큽니다. 다양한 화면 크기에서 적절한 크기인지 확인해보세요.
9-15: title 스타일 구현flexbox를 사용한 세로 배치와 gap 설정이 적절합니다. 일관된 간격 관리를 위한 좋은 접근입니다.
17-25: 버튼 색상 대비 확인 필요
현재continueButton(white[30])과putButton(purple[10])에 사용된 색상 값이 WCAG 2.1 AA 버튼 텍스트 대비비(최소 4.5:1)를 충족하는지 검증해 주세요.
- 확인 위치:
shared/styles/tokens/colors.ts(또는 유사 파일)에서white[30],purple[10]헥스 코드 값 조회- WCAG 컨트라스트 계산기(https://webaim.org/resources/contrastchecker/) 등에 헥스 코드를 입력해 대비비 확인
shared/ui/popup/popup-warning-letter/popup-warning-letter.css.ts (1)
1-30: 잘 구현된 CSS 모듈입니다.vanilla-extract와 테마 변수를 활용한 일관된 스타일링이 좋습니다. 팝업 컴포넌트들 간의 통일성도 잘 유지되어 있네요.
shared/ui/popup/popup-warning-capsule/popup-warning-capsule.css.ts (1)
1-22: 일관된 스타일링이 잘 적용되었습니다.다른 팝업 컴포넌트들과 동일한 패턴으로 테마 변수를 활용한 깔끔한 구현입니다.
shared/ui/popup/popup-cancel-creation/popup-cancel-creation.css.ts (1)
1-24: CSS 구현 양호vanilla-extract와 테마 시스템을 올바르게 사용하고 있으며, 다른 팝업 컴포넌트들과 일관된 스타일링 패턴을 따르고 있습니다.
shared/ui/popup/popup.css.ts (2)
4-19: 기본 팝업 레이아웃 구현이 우수함중앙 정렬, z-index 관리, 테마 변수 사용이 잘 구현되었습니다. 모달 팝업의 표준적인 패턴을 잘 따르고 있습니다.
45-53: 딤 오버레이 구현 확인반투명 배경과 적절한 z-index 설정이 잘 되어있습니다. 모달의 접근성과 사용성을 높이는 좋은 구현입니다.
shared/ui/popup/popup.stories.tsx (1)
14-71: 포괄적인 Storybook 설정과 문서화컴포넌트 사용법, 다양한 예시, 한국어 문서화가 매우 잘 되어있습니다. 개발자 경험을 크게 향상시킬 것입니다.
shared/ui/popup/popup-intro/popup-intro.css.ts (1)
66-80: 커스텀 라디오 접근성 강화 (focus-visible, 대비 확보)
appearance: none으로 기본 스타일을 제거하면 키보드 포커스 링도 사라집니다. 시각장애·키보드 사용자 모두를 위해 포커스 및 체크 상태를 명확히 표시해주세요.export const radio = style({ appearance: "none", width: "2rem", height: "2rem", border: `3px solid ${themeVars.color.white[5]}`, borderRadius: "10px", + cursor: "pointer", transition: "all 0.3s ease", selectors: { "&:checked": { border: `5px solid ${themeVars.color.purple[50]}`, borderRadius: "1rem", }, + "&:focus-visible": { + outline: `2px solid ${themeVars.color.purple[50]}`, + outlineOffset: "2px", + }, }, });이렇게 하면
- 키보드
Tab이동 시 포커스 링이 나타나고- 체크 상태 대비가 강화되어 WCAG 기준 충족에 도움됩니다.
[ suggest_essential_refactor ]
|
This pull request (commit
|
|
This pull request (commit
|
|
This pull request (commit
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
shared/ui/popup/popup-confirm-letter/popup-confirm-letter.css.ts (1)
9-15: 타이틀 스타일에서 타이포그래피 일관성을 고려해보세요.다른 팝업 컴포넌트들은
themeVars.text.F17또는themeVars.text.F20을 사용하여 폰트 스타일을 정의하고 있습니다. 현재 구조가 특별한 레이아웃 요구사항 때문이라면 괜찮지만, 가능하다면 타이포그래피 테마 변수 사용을 고려해보세요.export const title = style({ + ...themeVars.text.F17, // 또는 F20 display: "flex", flexDirection: "column", gap: "1.3rem", marginTop: "0.4rem", marginBottom: "1rem", });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
app/page.tsx(1 hunks)shared/ui/popup/index.tsx(1 hunks)shared/ui/popup/popup-cancel-creation/index.tsx(1 hunks)shared/ui/popup/popup-cancel-creation/popup-cancel-creation.css.ts(1 hunks)shared/ui/popup/popup-confirm-letter/index.tsx(1 hunks)shared/ui/popup/popup-confirm-letter/popup-confirm-letter.css.ts(1 hunks)shared/ui/popup/popup-exit-lettie/index.tsx(1 hunks)shared/ui/popup/popup-exit-lettie/popup-exit-lettie.css.ts(1 hunks)shared/ui/popup/popup-report/index.tsx(1 hunks)shared/ui/popup/popup-report/popup-report.css.ts(1 hunks)shared/ui/popup/popup-warning-capsule/index.tsx(1 hunks)shared/ui/popup/popup-warning-capsule/popup-warning-capsule.css.ts(1 hunks)shared/ui/popup/popup-warning-letter/index.tsx(1 hunks)shared/ui/popup/popup-warning-letter/popup-warning-letter.css.ts(1 hunks)shared/ui/popup/popup.css.ts(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- shared/ui/popup/popup-warning-letter/popup-warning-letter.css.ts
- shared/ui/popup/popup-exit-lettie/index.tsx
- shared/ui/popup/popup-report/popup-report.css.ts
- app/page.tsx
🚧 Files skipped from review as they are similar to previous changes (9)
- shared/ui/popup/popup-cancel-creation/index.tsx
- shared/ui/popup/popup-warning-letter/index.tsx
- shared/ui/popup/popup-cancel-creation/popup-cancel-creation.css.ts
- shared/ui/popup/popup-warning-capsule/index.tsx
- shared/ui/popup/index.tsx
- shared/ui/popup/popup-report/index.tsx
- shared/ui/popup/popup-exit-lettie/popup-exit-lettie.css.ts
- shared/ui/popup/popup-confirm-letter/index.tsx
- shared/ui/popup/popup.css.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
shared/ui/popup/popup-warning-capsule/popup-warning-capsule.css.ts (6)
shared/ui/popup/popup-cancel-creation/popup-cancel-creation.css.ts (2)
layout(4-7)continueButton(15-17)shared/ui/popup/popup-confirm-letter/popup-confirm-letter.css.ts (1)
layout(4-7)shared/ui/popup/popup-exit-lettie/popup-exit-lettie.css.ts (1)
layout(4-7)shared/ui/popup/popup-report/popup-report.css.ts (1)
layout(4-7)shared/ui/popup/popup-warning-letter/popup-warning-letter.css.ts (3)
layout(4-7)iconWrapper(9-11)continueButton(13-15)shared/styles/base/theme.css.ts (1)
themeVars(14-14)
shared/ui/popup/popup-confirm-letter/popup-confirm-letter.css.ts (7)
shared/ui/popup/popup-cancel-creation/popup-cancel-creation.css.ts (2)
layout(4-7)title(9-13)shared/ui/popup/popup-exit-lettie/popup-exit-lettie.css.ts (2)
layout(4-7)title(9-11)shared/ui/popup/popup-warning-capsule/popup-warning-capsule.css.ts (1)
layout(4-7)shared/ui/popup/popup-report/popup-report.css.ts (2)
layout(4-7)title(9-11)shared/ui/popup/popup-warning-letter/popup-warning-letter.css.ts (1)
layout(4-7)shared/ui/popup/popup.css.ts (1)
title(21-25)shared/styles/base/theme.css.ts (1)
themeVars(14-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: comment-pr
- GitHub Check: test
🔇 Additional comments (5)
shared/ui/popup/popup-warning-capsule/popup-warning-capsule.css.ts (3)
4-7: 일관된 팝업 레이아웃 패턴을 잘 따르고 있습니다.다른 팝업 컴포넌트들과 동일한 너비(32.4rem)를 사용하여 시각적 일관성을 유지하고 있습니다.
9-11: 아이콘 여백 처리가 적절합니다.다른 팝업 컴포넌트들과 일관된 여백값(1.8rem)을 사용하여 시각적 균형을 맞추고 있습니다.
13-15: 테마 시스템을 올바르게 활용하고 있습니다.
themeVars.color.purple[10]을 사용하여 디자인 시스템의 일관성을 유지하고 있으며, 다른 팝업 컴포넌트들과 동일한 패턴을 따르고 있습니다.shared/ui/popup/popup-confirm-letter/popup-confirm-letter.css.ts (2)
4-7: 레이아웃 스타일이 잘 구현되었습니다.다른 팝업 컴포넌트들과 일관된 너비(32.4rem)를 사용하고 있으며, 높이는 해당 팝업의 콘텐츠에 맞게 적절히 설정되어 있습니다.
17-19: 버튼 스타일이 테마 변수를 올바르게 사용하고 있습니다.
themeVars.color.purple[10]을 사용하여 일관된 색상 시스템을 유지하고 있어 좋습니다.
|
This pull request (commit
|
|
This pull request (commit
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
shared/ui/popup/popup-confirm-letter/popup-confirm-letter.css.ts (2)
4-10: title 스타일이 다른 팝업 컴포넌트와 다른 패턴을 사용합니다.다른 팝업 컴포넌트들의 title 스타일은 주로 타이포그래피(
themeVars.text.F17,themeVars.text.F20등)를 적용하는 반면, 이 스타일은 flexbox 레이아웃에 집중하고 있습니다. 이는 아마도 여러 요소를 담는 컨테이너 역할을 하는 것으로 보입니다.만약 텍스트 스타일링이 필요하다면 일관성을 위해
themeVars.text속성 추가를 고려해보세요.
12-14: putButton 스타일이 올바르게 구현되었습니다.테마 시스템을 적절히 활용하여 일관된 색상을 적용했습니다. 다만
putButton이라는 네이밍이 다소 모호할 수 있으니, 향후 더 명확한 이름(submitButton,confirmButton등)으로 변경하는 것을 고려해보세요.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
shared/ui/popup/popup-cancel-creation/popup-cancel-creation.css.ts(1 hunks)shared/ui/popup/popup-confirm-letter/popup-confirm-letter.css.ts(1 hunks)shared/ui/popup/popup-exit-lettie/popup-exit-lettie.css.ts(1 hunks)shared/ui/popup/popup-report/popup-report.css.ts(1 hunks)shared/ui/popup/popup-warning-capsule/popup-warning-capsule.css.ts(1 hunks)shared/ui/popup/popup-warning-letter/popup-warning-letter.css.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- shared/ui/popup/popup-exit-lettie/popup-exit-lettie.css.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- shared/ui/popup/popup-warning-capsule/popup-warning-capsule.css.ts
- shared/ui/popup/popup-warning-letter/popup-warning-letter.css.ts
- shared/ui/popup/popup-report/popup-report.css.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
shared/ui/popup/popup-confirm-letter/popup-confirm-letter.css.ts (5)
shared/ui/popup/popup-exit-lettie/popup-exit-lettie.css.ts (1)
title(4-6)shared/ui/popup/popup-report/popup-report.css.ts (1)
title(4-6)shared/ui/popup/popup-cancel-creation/popup-cancel-creation.css.ts (1)
title(4-8)shared/ui/popup/popup.css.ts (1)
title(21-25)shared/styles/base/theme.css.ts (1)
themeVars(14-14)
shared/ui/popup/popup-cancel-creation/popup-cancel-creation.css.ts (4)
shared/ui/popup/popup-exit-lettie/popup-exit-lettie.css.ts (1)
title(4-6)shared/styles/base/theme.css.ts (1)
themeVars(14-14)shared/ui/popup/popup-warning-capsule/popup-warning-capsule.css.ts (1)
continueButton(8-10)shared/ui/popup/popup-warning-letter/popup-warning-letter.css.ts (1)
continueButton(8-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (4)
shared/ui/popup/popup-cancel-creation/popup-cancel-creation.css.ts (3)
1-2: import 구문이 올바르게 작성되었습니다.테마 변수와 vanilla-extract 스타일 함수를 적절히 가져오고 있으며, 프로젝트의 다른 팝업 컴포넌트들과 일관된 패턴을 따르고 있습니다.
4-8: title 스타일이 적절하게 정의되었습니다.테마의 F20 텍스트 스타일을 재사용하고 적절한 여백을 설정하여 시각적 계층구조를 잘 구성했습니다. 다른 팝업 컴포넌트들과 일관된 패턴을 따르고 있습니다.
10-12: continueButton 스타일이 일관성 있게 구현되었습니다.다른 팝업 컴포넌트들(
popup-warning-capsule,popup-warning-letter)과 동일한purple[10]색상 토큰을 사용하여 UI 일관성을 유지하고 있습니다.shared/ui/popup/popup-confirm-letter/popup-confirm-letter.css.ts (1)
1-2: 임포트 구문이 올바릅니다.테마 시스템과 vanilla-extract의 표준 임포트 패턴을 잘 따르고 있습니다.
|
혹시 Popup이 |
넵 추가했습니다! |
|
This pull request (commit
|
|
This pull request (commit
|
seung365
left a comment
There was a problem hiding this comment.
Popup 구현 너무 고생하셨습니다~! pending 되었던 댓글 확인해주시고 괜찮다 싶으시면 반영해주심 감사합니당
어푸는 했습니다~! :)
|
|
||
| declare module "*.svg" { | ||
| import type { SVGProps } from "react"; | ||
| const ReactComponent: FunctionComponent< |
There was a problem hiding this comment.
여기 FunctionComponent를 import 해줘야할거 같습니닷
| open: boolean; | ||
| } | ||
|
|
||
| const PopupRoot = ({ children, className, open, ...props }: PopupRootProps) => { |
There was a problem hiding this comment.
여기 ...props를 뿌리는데 현재 PopupRootProps를 보았을땐 children, className, open이 세개말곤 더 없어보입니다
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
shared/hooks/useInView.ts (2)
8-12: 타입 일관성 개선 필요JSDoc에서는
React.RefObject<HTMLElement>로 명시되어 있지만, 실제 구현에서는HTMLDivElement로 제한되어 있습니다. 더 범용적인 사용을 위해 제네릭 타입을 고려해보세요.- * @returns {{ - * ref: React.RefObject<HTMLElement>; - * isIntersecting: boolean; - * }} ref: 감지할 요소에 할당할 ref, isIntersecting: 뷰포트에 보이는지 여부 + * @returns {{ + * ref: React.RefObject<T>; + * isIntersecting: boolean; + * }} ref: 감지할 요소에 할당할 ref, isIntersecting: 뷰포트에 보이는지 여부
19-21: 제네릭 타입으로 확장성 개선현재
HTMLDivElement로 고정되어 있어 다른 HTML 요소에서 사용하기 어렵습니다. 제네릭 타입을 사용하여 재사용성을 높여보세요.- export const useInView = (threshold = 0.2) => { - const ref = useRef<HTMLDivElement>(null); + export const useInView = <T extends HTMLElement = HTMLDivElement>(threshold = 0.2) => { + const ref = useRef<T>(null);shared/ui/add-capsule-button/index.tsx (2)
8-10: threshold 값을 props로 받는 것을 고려해보세요현재 하드코딩된 0.2 값을 컴포넌트 props로 받으면 재사용성이 높아질 수 있습니다.
- const AddCapsuleButton = () => { - const { ref, isIntersecting } = useInView(0.2); + interface AddCapsuleButtonProps { + threshold?: number; + } + + const AddCapsuleButton = ({ threshold = 0.2 }: AddCapsuleButtonProps) => { + const { ref, isIntersecting } = useInView(threshold);
13-13: ref div의 목적이 명확하지 않습니다감지용 요소와 실제 버튼이 분리된 구조의 의도를 코멘트로 설명하면 유지보수성이 향상됩니다.
+ {/* 뷰포트 감지용 기준점 요소 */} <div ref={ref} className={styles.refDiv} />
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
shared/assets/icon/heart.svgis excluded by!**/*.svg
📒 Files selected for processing (10)
shared/hooks/useInView.ts(1 hunks)shared/styles/tokens/color.ts(1 hunks)shared/types/assets.d.ts(1 hunks)shared/ui/add-capsule-button/add-capsule-button.css.ts(1 hunks)shared/ui/add-capsule-button/add-capsule-button.stories.tsx(1 hunks)shared/ui/add-capsule-button/index.tsx(1 hunks)shared/ui/like-button/index.tsx(1 hunks)shared/ui/like-button/like-button.css.ts(1 hunks)shared/ui/like-button/like-button.stories.tsx(1 hunks)shared/ui/popup/index.tsx(1 hunks)
🧬 Code Graph Analysis (1)
shared/ui/add-capsule-button/index.tsx (2)
shared/hooks/useInView.ts (1)
useInView(19-42)shared/utils/cn.ts (1)
cn(3-5)
✅ Files skipped from review due to trivial changes (5)
- shared/styles/tokens/color.ts
- shared/ui/like-button/like-button.css.ts
- shared/ui/add-capsule-button/add-capsule-button.stories.tsx
- shared/ui/add-capsule-button/add-capsule-button.css.ts
- shared/ui/like-button/like-button.stories.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- shared/types/assets.d.ts
- shared/ui/popup/index.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
shared/ui/add-capsule-button/index.tsx (2)
shared/hooks/useInView.ts (1)
useInView(19-42)shared/utils/cn.ts (1)
cn(3-5)
🔇 Additional comments (4)
shared/ui/like-button/index.tsx (2)
5-7: 타입 정의가 깔끔합니다!
ComponentProps<"button">을 확장하여 모든 표준 버튼 속성을 지원하면서도 필요한isLiked속성을 추가한 것이 좋습니다.
9-15: 구현이 깔끔합니다!props 스프레딩 순서가 올바르고, 동적 스타일링이 잘 적용되었습니다. onClick 핸들러를 내부에서 처리하지 않아 재사용성이 높은 것도 좋은 설계입니다.
shared/hooks/useInView.ts (1)
23-39: IntersectionObserver 구현이 우수합니다올바른 cleanup 로직과 dependency 관리로 메모리 누수를 방지하고 있습니다. threshold 변경 시 observer 재생성도 적절히 처리되었습니다.
shared/ui/add-capsule-button/index.tsx (1)
1-1: "use client" 지시어 사용이 적절합니다IntersectionObserver와 상태를 사용하는 인터랙티브 컴포넌트에 클라이언트 사이드 렌더링이 필요한 상황입니다.
| <button | ||
| className={cn( | ||
| styles.buttonStyle, | ||
| isIntersecting ? styles.expanded : styles.collapsed, | ||
| )} | ||
| > | ||
| <PlusIcon /> | ||
| {isIntersecting && "만들기"} | ||
| </button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
접근성 개선이 필요합니다
버튼에 적절한 레이블과 role이 필요합니다. 시각적으로 텍스트가 보이지 않을 때 스크린 리더 사용자를 고려해야 합니다.
<button
+ aria-label="캡슐 만들기"
className={cn(
styles.buttonStyle,
isIntersecting ? styles.expanded : styles.collapsed,
)}
>
<PlusIcon />
- {isIntersecting && "만들기"}
+ <span className={isIntersecting ? undefined : "sr-only"}>만들기</span>
</button>🤖 Prompt for AI Agents
In shared/ui/add-capsule-button/index.tsx around lines 14 to 22, the button
lacks an accessible label and role for screen reader users, especially when the
text is visually hidden. Add an aria-label attribute to the button with a
descriptive label that conveys its purpose, and ensure the button element has
the correct role if needed. This will improve accessibility for users relying on
assistive technologies.
| <button className={styles.likeButton({ liked: isLiked })} {...props}> | ||
| <HeartIcon width={24} /> | ||
| </button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
접근성 개선이 필요합니다.
스크린 리더 사용자를 위해 접근성 속성 추가를 권장합니다.
- <button className={styles.likeButton({ liked: isLiked })} {...props}>
+ <button
+ className={styles.likeButton({ liked: isLiked })}
+ aria-label={isLiked ? "좋아요 취소" : "좋아요"}
+ aria-pressed={isLiked}
+ {...props}
+ >
<HeartIcon width={24} />
</button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button className={styles.likeButton({ liked: isLiked })} {...props}> | |
| <HeartIcon width={24} /> | |
| </button> | |
| <button | |
| className={styles.likeButton({ liked: isLiked })} | |
| aria-label={isLiked ? "좋아요 취소" : "좋아요"} | |
| aria-pressed={isLiked} | |
| {...props} | |
| > | |
| <HeartIcon width={24} /> | |
| </button> |
🤖 Prompt for AI Agents
In shared/ui/like-button/index.tsx around lines 11 to 13, the button element
lacks accessibility attributes for screen reader users. Add appropriate ARIA
attributes such as aria-pressed to indicate the toggle state of the like button,
and include an accessible label or aria-label describing the button's action.
This will improve accessibility for users relying on screen readers.
|
This pull request (commit
|
* feat: view-type-tabs 구현 및 스토리 작성 * refactor: 코드래빗 리뷰 반영 * chore: 폴더 구조 수정
|
This pull request (commit
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
shared/ui/view-type-tabs/index.tsx (1)
15-44: 접근성 개선을 고려해보세요컴포넌트 구현이 깔끔하고 잘 구성되어 있습니다. 다만 접근성 측면에서 개선할 점이 있습니다:
- aria-label="Layers" + aria-label={viewType === ViewType.LAYERS ? `${ViewType.LAYERS} 선택됨` : ViewType.LAYERS}- aria-label="Grid" + aria-label={viewType === ViewType.GRID ? `${ViewType.GRID} 선택됨` : ViewType.GRID}현재 선택 상태를 스크린 리더 사용자에게 전달할 수 있습니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
shared/assets/icon/grid.svgis excluded by!**/*.svgshared/assets/icon/layers.svgis excluded by!**/*.svg
📒 Files selected for processing (4)
shared/types/types.ts(1 hunks)shared/ui/view-type-tabs/index.tsx(1 hunks)shared/ui/view-type-tabs/view-type-tabs.css.ts(1 hunks)shared/ui/view-type-tabs/view-type-tabs.stories.tsx(1 hunks)
🧬 Code Graph Analysis (1)
shared/ui/view-type-tabs/index.tsx (1)
shared/utils/cn.ts (1)
cn(3-5)
✅ Files skipped from review due to trivial changes (3)
- shared/types/types.ts
- shared/ui/view-type-tabs/view-type-tabs.css.ts
- shared/ui/view-type-tabs/view-type-tabs.stories.tsx
🧰 Additional context used
🧬 Code Graph Analysis (1)
shared/ui/view-type-tabs/index.tsx (1)
shared/utils/cn.ts (1)
cn(3-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: storybook-deploy
🔇 Additional comments (3)
shared/ui/view-type-tabs/index.tsx (3)
1-8: Import 구조가 깔끔합니다절대 경로를 일관되게 사용하고 필요한 의존성만 import한 깔끔한 구조입니다.
10-13: 타입 정의가 명확합니다ViewType enum을 사용한 타입 안전한 인터페이스 정의가 잘 되어 있습니다.
46-46: LGTM적절한 default export입니다.
📌 Summary
📚 Tasks
👀 To Reviewer
팝업 열고닫는 로직을 추상화하여 일괄적으로 관리하고, 팝업을 선언적으로 띄울 방법을 고민하다 아래 패키지를 설치했습니다.
사용 방법은 스토리북에 문서화해두었습니다.
Popup 컴포넌트를 구현하고, 이를 활용해 필요한 커스텀 팝업을 모두 구현해놓았습니다. 네이밍 제안 환영입니다,,,,,,
참고로 PopupConfirmLetter는 아직 .apng 적용 전입니다! 적용 후 머지 예정입니다.-> 적용 완료!
📸 Screenshot
스토리북 참고참고!!!!
Summary by CodeRabbit
신규 기능
버그 수정
개선 및 기타