Conversation
Walkthrough디자인 시스템에 Popup 및 PopupContainer 컴포넌트를 추가하고 관련 스토리북 파일을 작성했으며, components index에 두 컴포넌트를 재수출로 노출했습니다. 또한 Input 컴포넌트의 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant App as Consumer App
participant PC as PopupContainer
participant P as Popup
participant DOM as document.body (Portal)
U->>App: "Open Popup" 클릭
App->>PC: isOpen=true, onClose, popupProps
PC-->>DOM: createPortal(Overlay + Popup)
PC->>P: 렌더링(popupProps, onLeftClick=onClose)
Note over P: type에 따라\n- input: Input 렌더링\n- subtext: 보조문구 표시\n- default: 콘텐츠 없음
alt 왼쪽 버튼 클릭
U->>P: Left 버튼 클릭
P->>PC: onLeftClick()
PC->>App: onClose()
else ESC 입력
U->>PC: ESC 키 입력
PC->>App: onClose()
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
✅ Storybook chromatic 배포 확인: |
![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 (9)
packages/design-system/src/components/input/Input.tsx (1)
34-41: 접근성: helperText를 스크린리더에 연결하세요(aria-describedby + useId)에러 도움말이 시각적으로만 보이고 스크린리더에 연결되어 있지 않습니다.
helperText가 있을 때input에aria-describedby를 부여하고, 도움말 노드에id를 부여하면 접근성이 개선됩니다.다음 diff를 적용해 주세요:
-import { InputHTMLAttributes, Ref } from 'react'; +import { InputHTMLAttributes, Ref, useId } from 'react'; @@ }: InputProps) => { + const helperId = useId(); return ( @@ <input ref={ref} className="placeholder-font-gray-3 w-full focus:outline-none" - aria-invalid={isError} + aria-invalid={!!isError} + aria-describedby={isError && helperText ? helperId : undefined} {...props} /> @@ - {isError && helperText && ( + {isError && helperText && ( <div className="flex items-center"> - <p className="text-error body4-r">{helperText}</p> + <p id={helperId} className="text-error body4-r">{helperText}</p> </div> )}Also applies to: 44-49
packages/design-system/src/components/popup/Popup.tsx (6)
13-22: 구조분해 파라미터에 신규 프롭 누락위 인터페이스 확장과 맞추려면 구조분해에도 포함돼야 합니다.
const Popup = ({ type, subtext, placeholder, title, left, right, helperText, isError, + onLeftClick, + onRightClick, + inputProps, }: PopupProps) => {
24-26: 대화상자 역할/라벨링 추가 및 커서 스타일 조정팝업이 모달/대화 상자로 동작한다면
role="dialog",aria-modal,aria-labelledby/aria-describedby등을 부여해 접근성을 확보하는 것이 바람직합니다. 또한cursor-pointer는 입력 필드/버튼과 상충되므로 제거를 권장합니다.- <div className="flex w-[26rem] cursor-pointer flex-col items-center justify-center rounded-[1.2rem] bg-white px-[1.6rem] py-[2.4rem] shadow-[0_0_32px_0_rgba(0,0,0,0.10)]"> - <div className="sub2-sb text-font-black-1 pb-[0.8rem]">{title}</div> + <div + role="dialog" + aria-modal="true" + aria-labelledby={titleId} + aria-describedby={type === 'subtext' ? descId : undefined} + className="flex w-[26rem] flex-col items-center justify-center rounded-[1.2rem] bg-white px-[1.6rem] py-[2.4rem] shadow-[0_0_32px_0_rgba(0,0,0,0.10)]" + > + <h2 id={titleId} className="sub2-sb text-font-black-1 pb-[0.8rem]">{title}</h2>컴포넌트 상단에 다음 보일러플레이트를 추가해 주세요(삽입 코드):
// 파일 상단 import에 추가 import { useId } from 'react'; // 컴포넌트 내부(반환문 위) const titleId = useId(); const descId = useId();
27-33: Input 프롭 패스스루로 실사용성 강화외부에서
value/onChange/name등 입력 제어가 필요합니다.inputProps를 스프레드하세요.<Input placeholder={placeholder} helperText={helperText} isError={isError} + {...inputProps} />
35-39:subtext를 대화상자 설명으로 연결
aria-describedby가 참조할 수 있도록 id를 부여하세요.- {type === 'subtext' && ( - <div className="body3-r text-font-gray-2 w-full py-[0.8rem] text-center"> + {type === 'subtext' && ( + <div id={descId} className="body3-r text-font-gray-2 w-full py-[0.8rem] text-center"> {subtext} </div> )}
40-40: 주석 오탈자
때가 맞습니다:type===default일 때는 아무것도 없음.- {/* type===default일 떄는 아무것도 없음 */} + {/* type===default일 때는 아무것도 없음 */}
1-11: 설계 방향에 대한 의견(텍스트/에러를 prop으로 받을지 내부에서 파생할지)현 단계에선 “표현 컴포넌트”로서 텍스트/에러 메시지를 prop으로 받는 쪽이 확장과 재사용에 유리합니다. 다만 다음을 함께 고려하면 유연성과 안전성이 올라갑니다.
- 타입 분기별로 요구 프롭을 강제하는 식별 합집합(discriminated union) 적용
- 예:
type: 'subtext'일 때subtext: ReactNode필수 등- 버튼 라벨/액션은 슬롯/프롭 병행 지원
- 기본:
left/right + onLeftClick/onRightClick- 고급:
actions?: { label: ReactNode; onClick?: () => void; tone?: 'primary'|'neutral'|'danger' }[]- 오버레이/포털/포커스 트랩은 후속 PR에서 모달 컨테이너로 분리(접근성/포커스 관리 전담)
packages/design-system/src/components/popup/Popup.stories.tsx (2)
9-12: argTypes control 표기 방식 일관화 제안
control: 'radio'도 동작하지만, Storybook 7/8 가이드에 맞춰control: { type: 'radio' }로 통일하면 도구 호환성이 좋아집니다.- type: { - control: 'radio', + type: { + control: { type: 'radio' }, options: ['input', 'subtext', 'default'], description: '팝업의 내용 타입', },
34-42: 스토리 상호작용 노출: 버튼 액션을 action 패널에 연결Popup에
onLeftClick/onRightClick을 추가하는 경우, 스토리에서 action을 연결하면 DX가 좋아집니다.helperText: { control: 'text', description: '에러 상태일 때 표시할 도움말', }, + onLeftClick: { action: 'left click' }, + onRightClick: { action: 'right click' }, },추가로,
parameters: { layout: 'centered' }를meta에 지정하면 캔버스 중앙 정렬이 보장됩니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/client/src/App.tsx(2 hunks)packages/design-system/src/components/index.ts(1 hunks)packages/design-system/src/components/input/Input.tsx(1 hunks)packages/design-system/src/components/popup/Popup.stories.tsx(1 hunks)packages/design-system/src/components/popup/Popup.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: constantly-dev
PR: Pinback-Team/pinback-client#80
File: apps/client/src/shared/components/ui/modalPop/ModalPop.tsx:36-41
Timestamp: 2025-07-15T20:00:13.756Z
Learning: In apps/client/src/shared/components/ui/modalPop/ModalPop.tsx, the InfoBox component uses hardcoded values for title, location, and icon URL as temporary test data. These should be replaced with dynamic data from props when implementing actual functionality and should be marked with TODO comments for future changes.
📚 Learning: 2025-08-23T18:37:59.985Z
Learnt from: jjangminii
PR: Pinback-Team/pinback-client#22
File: packages/design-system/src/components/input/Input.tsx:5-9
Timestamp: 2025-08-23T18:37:59.985Z
Learning: React 19부터 함수 컴포넌트에서 ref를 일반 prop으로 받을 수 있어 forwardRef가 더 이상 필요하지 않다. InputProps interface에서 ref: Ref<HTMLInputElement>로 정의하고 함수 컴포넌트에서 ({ ref, ...props }) => 형태로 받을 수 있다.
Applied to files:
packages/design-system/src/components/input/Input.tsx
📚 Learning: 2025-08-23T18:37:59.985Z
Learnt from: jjangminii
PR: Pinback-Team/pinback-client#22
File: packages/design-system/src/components/input/Input.tsx:5-9
Timestamp: 2025-08-23T18:37:59.985Z
Learning: React 19에서는 함수 컴포넌트가 ref를 일반 prop으로 받을 수 있다. interface에서 ref: Ref<HTMLInputElement>로 정의하고 함수 컴포넌트에서 ({ ref, ...props }) => 형태로 받을 수 있어 forwardRef가 불필요하다. React 19.1.1+ 프로젝트에서는 이 방식을 사용해야 한다.
Applied to files:
packages/design-system/src/components/input/Input.tsx
🔇 Additional comments (4)
apps/client/src/App.tsx (2)
19-19: 불필요한 백슬래시 제거로 JSX 구문 오류 예방 — 잘 처리됐습니다
<h1>Vite + React</h1>라인 끝의 백슬래시 제거로 빌드/런타임 에러 가능성을 제거했습니다.
5-5: import 블록 가독성 개선 OK의미 있는 빈 줄 추가로 import 섹션 가독성이 좋아졌습니다.
packages/design-system/src/components/index.ts (1)
9-9: Popup 공개 export 추가 — 문제 없습니다디자인 시스템 표면(API)에 새 컴포넌트를 노출하는 올바른 변경입니다.
packages/design-system/src/components/popup/Popup.stories.tsx (1)
1-1: Storybook 타입 import 경로 일관성 확인됨워크스페이스 전체의
*.stories.*파일을 확인한 결과, 모든 Story 파일이@storybook/react-vite에서Meta및StoryObj를 import하고 있어 일관성이 유지되고 있습니다. 변경이 불필요합니다.
|
|
||
| interface InputProps extends InputHTMLAttributes<HTMLInputElement> { | ||
| ref: Ref<HTMLInputElement>; | ||
| ref?: Ref<HTMLInputElement>; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
ref를 선택적(prop)으로 전환 — React 19 채택 전제에서는 합리적
이 변경은 React 19의 “함수 컴포넌트가 ref를 일반 prop으로 받는다”는 팀 컨벤션/러닝과 일치합니다. 하위 호환성(React 18 이하)에서는 깨질 수 있으므로, 워크스페이스 전반이 React 19.x 타입을 사용 중인지 한 번 확인을 부탁드립니다.
다음 스크립트로 워크스페이스의 React/타입 버전을 점검해 주세요.
🏁 Script executed:
#!/bin/bash
# 리포지토리 내 package.json에서 react / @types/react 버전 확인
fd -a 'package.json' | while read -r f; do
echo "==> $f"
jq -r '.dependencies.react,.devDependencies.react,.peerDependencies.react' "$f" 2>/dev/null | awk 'NF{print "react:",$0}'
jq -r '.dependencies["@types/react"],.devDependencies["@types/react"],.peerDependencies["@types/react"]' "$f" 2>/dev/null | awk 'NF{print "@types/react:",$0}'
done | sed '/null$/d'Length of output: 3971
React 19 도입 기반 검증 — @types/react 버전 일관성 확보 필요
이 PR에서 ref를 선택적 prop으로 전환한 것은 React 19의 컨벤션에 부합하나, 워크스페이스 전반의 타입 일관성을 위해 다음 위치의 @types/react 버전을 React 19.x와 맞춰주세요.
• apps/extension/package.json
– 현재: @types/react: ^18.3.5
– 수정: React 19.x 타입(^19.1.10 등)으로 업데이트
• 루트 package.json
– 현재: @types/react: 19
– 수정: 명확한 SemVer(^19.1.x) 형식으로 변경
• 필요 시, 다른 패키지(특히 devDependencies)에 @types/react가 있다면 동일하게 점검 및 업데이트
위 조치 후, React 19 기준의 ref prop 사용이 전체 코드베이스에서 안전하게 작동함을 보장할 수 있습니다.
🤖 Prompt for AI Agents
packages/design-system/src/components/input/Input.tsx around line 6: the ref
prop was made optional to align with React 19 conventions but workspace
@types/react versions are inconsistent; update apps/extension/package.json to
use a React 19.x types range (e.g. "@types/react": "^19.1.10"), change the root
package.json entry from "19" to a clear semver range like "^19.1.x", and audit
other package.json/devDependencies across the monorepo to ensure all
@types/react entries are aligned to the same React 19.x range so the optional
ref typing is safe throughout the codebase.
| interface PopupProps { | ||
| type: 'input' | 'subtext' | 'default'; | ||
| subtext?: string; | ||
| placeholder?: string; | ||
| title: string; | ||
| left: string; | ||
| right: string; | ||
| isError?: boolean; | ||
| helperText?: string; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
API 보완 제안: 클릭 핸들러/ReactNode/입력 프롭 패스스루 추가
현재는 버튼 텍스트만 받고 클릭 동작을 노출하지 않아 실사용이 어렵습니다. 또 title/텍스트들이 string으로 제한되어 아이콘/강조 텍스트 등을 넣기 어렵습니다. Input에도 value/onChange 등을 전달할 수 있도록 패스스루가 필요합니다.
아래처럼 인터페이스 확장을 제안합니다:
-interface PopupProps {
- type: 'input' | 'subtext' | 'default';
- subtext?: string;
- placeholder?: string;
- title: string;
- left: string;
- right: string;
- isError?: boolean;
- helperText?: string;
-}
+import type { ComponentProps, ReactNode } from 'react';
+
+interface PopupProps {
+ type: 'input' | 'subtext' | 'default';
+ title: ReactNode;
+ left: ReactNode;
+ right: ReactNode;
+ subtext?: ReactNode;
+ placeholder?: string; // 유지(placeholder만 간단 전달 시)
+ isError?: boolean;
+ helperText?: string;
+ // 동작 핸들러
+ onLeftClick?: () => void;
+ onRightClick?: () => void;
+ // Input 컴포넌트로 프롭 패스스루
+ inputProps?: ComponentProps<typeof Input>;
+}그리고 아래 두 군데도 함께 반영해 주세요:
- 라인 13~22의 파라미터 구조분해에
onLeftClick,onRightClick,inputProps추가 - 라인 28~33의
<Input ... />에{...inputProps}스프레드 추가
📝 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.
| interface PopupProps { | |
| type: 'input' | 'subtext' | 'default'; | |
| subtext?: string; | |
| placeholder?: string; | |
| title: string; | |
| left: string; | |
| right: string; | |
| isError?: boolean; | |
| helperText?: string; | |
| } | |
| // Add at the top of the file if not already present | |
| import type { ComponentProps, ReactNode } from 'react'; | |
| interface PopupProps { | |
| type: 'input' | 'subtext' | 'default'; | |
| - title: string; | |
| - left: string; | |
| - right: string; | |
| title: ReactNode; | |
| left: ReactNode; | |
| right: ReactNode; | |
| subtext?: ReactNode; | |
| placeholder?: string; // 유지(placeholder만 간단 전달 시) | |
| isError?: boolean; | |
| helperText?: string; | |
| // 클릭 핸들러 | |
| onLeftClick?: () => void; | |
| onRightClick?: () => void; | |
| // Input 컴포넌트로 프롭 패스스루 | |
| inputProps?: ComponentProps<typeof Input>; | |
| } |
| <div className="flex flex-row items-center justify-center gap-[1.2rem] pt-[0.8rem]"> | ||
| <button className="border-gray200 sub5-sb bg-white-bg text-font-black-1 w-[10.8rem] rounded-[0.4rem] border py-[0.85rem]"> | ||
| {left} | ||
| </button> | ||
| <button className="sub5-sb bg-gray900 text-white-bg w-[10.8rem] rounded-[0.4rem] py-[0.85rem]"> | ||
| {right} | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
버튼: onClick 노출 및 type="button" 지정
폼 내부에서 의도치 않은 submit을 막고, 액션을 외부에 전달하세요. (가능하면 디자인 시스템의 Button 컴포넌트 사용도 고려해 주세요.)
- <button className="border-gray200 sub5-sb bg-white-bg text-font-black-1 w-[10.8rem] rounded-[0.4rem] border py-[0.85rem]">
+ <button
+ type="button"
+ onClick={onLeftClick}
+ className="border-gray200 sub5-sb bg-white-bg text-font-black-1 w-[10.8rem] rounded-[0.4rem] border py-[0.85rem]"
+ >
{left}
</button>
- <button className="sub5-sb bg-gray900 text-white-bg w-[10.8rem] rounded-[0.4rem] py-[0.85rem]">
+ <button
+ type="button"
+ onClick={onRightClick}
+ className="sub5-sb bg-gray900 text-white-bg w-[10.8rem] rounded-[0.4rem] py-[0.85rem]"
+ >
{right}
</button>추가로, 내부에서 직접 스타일링하는 대신 packages/design-system/src/components/button/Button을 활용하면 스타일 일관성이 좋아집니다. Button API가 다르면 해당 API에 맞춰 래핑해도 좋습니다.
📝 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="flex flex-row items-center justify-center gap-[1.2rem] pt-[0.8rem]"> | |
| <button className="border-gray200 sub5-sb bg-white-bg text-font-black-1 w-[10.8rem] rounded-[0.4rem] border py-[0.85rem]"> | |
| {left} | |
| </button> | |
| <button className="sub5-sb bg-gray900 text-white-bg w-[10.8rem] rounded-[0.4rem] py-[0.85rem]"> | |
| {right} | |
| </button> | |
| </div> | |
| <div className="flex flex-row items-center justify-center gap-[1.2rem] pt-[0.8rem]"> | |
| <button | |
| type="button" | |
| onClick={onLeftClick} | |
| className="border-gray200 sub5-sb bg-white-bg text-font-black-1 w-[10.8rem] rounded-[0.4rem] border py-[0.85rem]" | |
| > | |
| {left} | |
| </button> | |
| <button | |
| type="button" | |
| onClick={onRightClick} | |
| className="sub5-sb bg-gray900 text-white-bg w-[10.8rem] rounded-[0.4rem] py-[0.85rem]" | |
| > | |
| {right} | |
| </button> | |
| </div> |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
packages/design-system/src/components/popup/types.ts (3)
1-14: 변형별 필수/선택 props가 섞여 타입 안정성이 떨어집니다 — 판별 가능한 유니온으로 재구성 제안현재
BasePopupProps에subtext,placeholder,isError,helperText등이 모두 선택으로 포함되어 있어,type에 따른 필수성 보장이 되지 않습니다. 변형(input|subtext|default)에 따라 요구되는 필드를 강제하기 위해 판별 가능한 유니온으로 재구성하는 것을 권합니다. 런타임 변경 없이 타입 안전성만 크게 개선됩니다.아래처럼 전체를 유니온으로 변경하면, 스토리/사용처에서 변형별 오용을 컴파일 타임에 차단할 수 있습니다.
-export type PopupType = 'input' | 'subtext' | 'default'; - -export interface BasePopupProps { - type: PopupType; - title: string; - left: string; - right: string; - subtext?: string; - placeholder?: string; - isError?: boolean; - helperText?: string; - onLeftClick?: () => void; - onRightClick?: () => void; -} - -export interface PopupContainerProps extends BasePopupProps { - isOpen: boolean; - onClose: () => void; -} +export type PopupType = 'input' | 'subtext' | 'default'; + +interface PopupCommonProps { + type: PopupType; + title: string; + left: string; + right: string; + onLeftClick?: () => void; + onRightClick?: () => void; +} + +type PopupInputProps = PopupCommonProps & { + type: 'input'; + placeholder?: string; + isError?: boolean; + helperText?: string; +}; + +type PopupSubtextProps = PopupCommonProps & { + type: 'subtext'; + subtext: string; +}; + +type PopupDefaultProps = PopupCommonProps & { + type: 'default'; +}; + +export type BasePopupProps = + | PopupInputProps + | PopupSubtextProps + | PopupDefaultProps; + +export interface PopupContainerProps extends BasePopupProps { + isOpen: boolean; + onClose: () => void; +}필요하시면 기존 사용처(스토리/컴포넌트)에서의 타입 에러를 한 번에 정리하는 작은 PR 패치도 만들어드릴 수 있습니다.
6-7: 버튼 라벨의 표현력을 높이기 위해 string → ReactNode 확장 고려아이콘+텍스트 조합 등 디자인 확장을 위해
left,right타입을ReactNode로 넓히는 것을 추천합니다. 디자인 시스템 관점에서 유연성이 커집니다.-export interface BasePopupProps { +export interface BasePopupProps { ... - left: string; - right: string; + left: React.ReactNode; + right: React.ReactNode; ... }또한 네이밍을
left/right대신cancelLabel/confirmLabel(혹은secondaryLabel/primaryLabel)로 바꾸면 의미가 더 분명해집니다. 네이밍 변경은 영향 범위가 크니 시점만 합의되면 추후 적용해도 됩니다.타입 확장 시
Popup.tsx내부 렌더링이string전제 로직이 없는지 확인 부탁드립니다.
8-11: Input 변형의 값 제어/검증을 상위로 올릴 수 있는 훅 포인트 추가 제안현재 에러(
isError,helperText)는 외부 제어를 가정하지만 입력값 자체를 외부로 전달할 통상적인 채널(value/defaultValue/onChange)은 정의돼 있지 않습니다. 실사용 시 “저장” 클릭 시점에 입력값 전달/검증이 필요할 가능성이 높습니다. 다음과 같은 속성 추가를 고려해주세요.type PopupInputProps = PopupCommonProps & { type: 'input'; placeholder?: string; isError?: boolean; helperText?: string; + value?: string; + defaultValue?: string; + onChange?: (value: string) => void; };이렇게 하면 “에러 메시지는 상위 결정”이라는 현재 설계 의도와도 자연스럽게 맞물립니다.
작성해주신 고민에 대해: 기본 라벨/에러 메시지는 “변형별 합리적인 디폴트 제공 + 필요 시 오버라이드”가 유지보수/일관성 측면에서 가장 무난합니다. 즉, 컴포넌트 내부에서 기본값을 제공하되, props가 오면 그것을 우선하는 형태가 바람직합니다.
packages/design-system/src/components/popup/PopupContainer.tsx (2)
12-19: 배경 스크롤 잠금(optional)과 포커스 진입 보완모바일 환경에서 백그라운드 스크롤이 계속되는 UX를 방지하려면 열림 동안
body스크롤을 잠그는 것을 권장합니다. 또한 열릴 때 다이얼로그에 초점이 이동하도록 해주세요.useEffect(() => { if (!isOpen) return; const handleEsc = (e: KeyboardEvent) => { if (e.key === 'Escape') onClose(); }; window.addEventListener('keydown', handleEsc); return () => window.removeEventListener('keydown', handleEsc); }, [isOpen, onClose]); + + // 배경 스크롤 잠금 + 포커스 진입 + useEffect(() => { + if (!isOpen) return; + if (typeof document !== 'undefined') { + const prev = document.body.style.overflow; + document.body.style.overflow = 'hidden'; + // 다이얼로그에 포커스 이동 + setTimeout(() => dialogRef.current?.focus(), 0); + return () => { + document.body.style.overflow = prev; + }; + } + }, [isOpen]);
25-25: 색상 토큰/유틸 사용 권장
bg-[#00000099]대신 Tailwind 유틸(bg-black/60) 또는 디자인 토큰 변수를 사용하면 일관성과 테마 대응이 좋아집니다. 위의 메인 수정안에 포함해 두었습니다.packages/design-system/src/components/popup/PopupContainer.stories.tsx (1)
34-42: 인터랙션 스토리에서 기본 동작 완결성 보강현재 스토리는 "저장"(
right) 버튼 클릭 시 아무 동작이 없습니다. 데모 관점에서onRightClick에 닫기 동작(또는 토스트/로그)을 연결해 기본 플로우를 보여주면 더 명확합니다.<PopupContainer isOpen={open} onClose={() => setOpen(false)} type="input" title="카테고리 입력" left="취소" right="저장" + onRightClick={() => setOpen(false)} placeholder="카테고리 제목을 입력해주세요" />
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/client/src/App.tsx(2 hunks)packages/design-system/src/components/index.ts(1 hunks)packages/design-system/src/components/popup/Popup.tsx(1 hunks)packages/design-system/src/components/popup/PopupContainer.stories.tsx(1 hunks)packages/design-system/src/components/popup/PopupContainer.tsx(1 hunks)packages/design-system/src/components/popup/types.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/client/src/App.tsx
- packages/design-system/src/components/popup/Popup.tsx
- packages/design-system/src/components/index.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/design-system/src/components/popup/PopupContainer.tsx (1)
packages/design-system/src/components/popup/types.ts (1)
PopupContainerProps(16-19)
🔇 Additional comments (1)
packages/design-system/src/components/popup/PopupContainer.stories.tsx (1)
1-1: Storybook 타입 import 일관성 확인 완료레포 내 모든
.stories.*파일에서@storybook/react-vite만 사용되고 있으며,@storybook/react와의 혼용이 전혀 발견되지 않았습니다. 별도의 수정은 필요하지 않습니다.
| type Story = StoryObj<typeof PopupContainer>; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Story 제네릭이 meta의 component와 불일치
meta는 PopupContainerWrapper를 component로 사용하지만, Story 타입은 typeof PopupContainer를 제네릭으로 사용하고 있습니다. 타입 안전성과 자동 추론을 위해 일치시키는 편이 좋습니다.
-type Story = StoryObj<typeof PopupContainer>;
+type Story = StoryObj<typeof PopupContainerWrapper>;📝 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.
| type Story = StoryObj<typeof PopupContainer>; | |
| type Story = StoryObj<typeof PopupContainerWrapper>; |
🤖 Prompt for AI Agents
In packages/design-system/src/components/popup/PopupContainer.stories.tsx around
lines 19-20, the Story generic uses typeof PopupContainer while the meta uses
PopupContainerWrapper as the component; change the Story type to use typeof
PopupContainerWrapper so the generic matches the meta component, and ensure
PopupContainerWrapper is imported/available in this file to preserve type safety
and correct prop inference.
| return createPortal( | ||
| <div className="fixed inset-0 z-10 flex items-center justify-center"> | ||
| <div className="absolute inset-0 bg-[#00000099]" /> | ||
| <div className="relative"> | ||
| <Popup {...popupProps} onLeftClick={onClose} /> | ||
| </div> | ||
| </div>, | ||
| document.body // body 위에서 렌더링 되게 함! | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
SSR 안전성 보강 및 포털 대상 가드 필요
isOpen === true 상태로 SSR 렌더가 발생하면 document가 없어 크래시가 납니다. 마운트 여부 가드와 document 존재 확인 후 포털을 생성하도록 방어 코드를 추가해주세요.
또한 현재 onLeftClick를 무조건 onClose로 덮어써 상위가 전달한 핸들러가 무시됩니다. “취소 후 닫기”라는 기본 UX를 유지하면서도 상위 핸들러와 합성하도록 바꾸는 편이 안전합니다.
-import { useEffect } from 'react';
+import { useEffect, useRef, useState } from 'react';
...
-const PopupContainer = ({
+const PopupContainer = ({
isOpen,
onClose,
...popupProps
}: PopupContainerProps) => {
+ const [mounted, setMounted] = useState(false);
+ const dialogRef = useRef<HTMLDivElement>(null);
+ const { onLeftClick, ...restPopupProps } = popupProps;
+
+ // 클라이언트 마운트 확인
+ useEffect(() => {
+ setMounted(true);
+ return () => setMounted(false);
+ }, []);
// ESC 키로 닫는 것 정도 (외부 클릭은 안되게! : 어차피 x박스나 취소버튼이 있음)
useEffect(() => {
if (!isOpen) return;
const handleEsc = (e: KeyboardEvent) => {
if (e.key === 'Escape') onClose();
};
window.addEventListener('keydown', handleEsc);
return () => window.removeEventListener('keydown', handleEsc);
}, [isOpen, onClose]);
- if (!isOpen) return null;
+ if (!isOpen || !mounted) return null;
- return createPortal(
+ const portalTarget =
+ typeof document !== 'undefined' ? document.body : null;
+
+ return portalTarget ? createPortal(
<div className="fixed inset-0 z-10 flex items-center justify-center">
- <div className="absolute inset-0 bg-[#00000099]" />
- <div className="relative">
- <Popup {...popupProps} onLeftClick={onClose} />
+ <div className="absolute inset-0 bg-black/60" />
+ <div
+ className="relative outline-none"
+ ref={dialogRef}
+ role="dialog"
+ aria-modal="true"
+ aria-label={popupProps.title}
+ tabIndex={-1}
+ >
+ <Popup
+ {...restPopupProps}
+ onLeftClick={() => {
+ onLeftClick?.();
+ onClose();
+ }}
+ />
</div>
</div>,
- document.body // body 위에서 렌더링 되게 함!
- );
+ portalTarget // body 위에서 렌더링 되게 함!
+ ) : null;- SSR 크래시 방지:
mounted+portalTarget가드 - a11y 기본값:
role="dialog",aria-modal,aria-label,tabIndex={-1}및 포커스 가능한 컨테이너 - 핸들러 합성: 상위
onLeftClick→ 실행 후onClose호출 - 시각적: 투명도는 Tailwind 표기(
bg-black/60) 사용
추가로, isOpen이 true가 될 때 dialogRef.current?.focus() 하는 포커스 진입 로직을 넣으면 키보드 접근성이 더 좋아집니다.
📝 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.
| return createPortal( | |
| <div className="fixed inset-0 z-10 flex items-center justify-center"> | |
| <div className="absolute inset-0 bg-[#00000099]" /> | |
| <div className="relative"> | |
| <Popup {...popupProps} onLeftClick={onClose} /> | |
| </div> | |
| </div>, | |
| document.body // body 위에서 렌더링 되게 함! | |
| ); | |
| // don't render on server or when closed | |
| if (!isOpen || !mounted) return null; | |
| const portalTarget = | |
| typeof document !== 'undefined' ? document.body : null; | |
| return portalTarget ? createPortal( | |
| <div className="fixed inset-0 z-10 flex items-center justify-center"> | |
| <div className="absolute inset-0 bg-black/60" /> | |
| <div | |
| className="relative outline-none" | |
| ref={dialogRef} | |
| role="dialog" | |
| aria-modal="true" | |
| aria-label={popupProps.title} | |
| tabIndex={-1} | |
| > | |
| <Popup | |
| {...restPopupProps} | |
| onLeftClick={() => { | |
| onLeftClick?.(); | |
| onClose(); | |
| }} | |
| /> | |
| </div> | |
| </div>, | |
| portalTarget // body 위에서 렌더링 되게 함! | |
| ) : null; |
🤖 Prompt for AI Agents
packages/design-system/src/components/popup/PopupContainer.tsx around lines 23
to 31: guard portal creation with a mounted flag and a portalTarget check (only
call createPortal when mounted && typeof document !== "undefined" &&
document.body exists), change backdrop class to Tailwind shorthand bg-black/60,
make the popup container focusable (tabIndex={-1}) and add accessibility
attributes role="dialog" aria-modal="true" and an aria-label prop fallback, on
open call dialogRef.current?.focus() to move focus into the dialog, and compose
the onLeftClick handler so it first calls any provided popupProps.onLeftClick
and then calls onClose (ensuring both run) instead of unconditionally overriding
it.
constantly-dev
left a comment
constantly-dev
left a comment
There was a problem hiding this comment.
수고하셨습니다! 현재 createPortal로 잘 설계해주셨는데 이후에
- 인터페이스가 더 많아진다면 설계를 어떻게 수정해볼지
- overlay(요소 외부 영역) 부분이 다른 곳에서도 쓰일텐데 공통으로 어떻게 뺄 수 있을지
- 현재의 Popup보다 더 선언적이고 좋은 설계는 없을지 (feat. toss overlay kit 등)
등을 더 생각해보시면 좋을 것 같아요 👍
| const meta: Meta<typeof Popup> = { | ||
| title: 'Components/Popup', | ||
| component: Popup, | ||
| tags: ['autodocs'], | ||
| argTypes: { | ||
| type: { | ||
| control: 'radio', | ||
| options: ['input', 'subtext', 'default'], | ||
| description: '팝업의 내용 타입', |
| const Popup = ({ | ||
| type, | ||
| subtext, | ||
| placeholder, | ||
| title, | ||
| left, | ||
| right, | ||
| helperText, | ||
| isError, | ||
| onLeftClick, | ||
| onRightClick, | ||
| }: BasePopupProps) => { |
There was a problem hiding this comment.
디자인을 보니 설계하기가 어려우셨을 것 같습니다! 꽤 많은 타입의 popup이 있어서 힘드셨을텐데 잘 해주셨네요.
다만 확실히 인터페이스가 많아지는 것은 사실이어서 이후에 더 좋은 설계가 있다면 리팩토링하는 것도 좋을 것 같아요 👍
There was a problem hiding this comment.
추후 계속 리팩토링 요소 보이는대로 작업해보겠습니다! 감사합니다
| @@ -0,0 +1,52 @@ | |||
| import Input from '../input/Input'; | |||
| import { BasePopupProps } from './types'; | |||
There was a problem hiding this comment.
Props 타입은 따로 분리하지 않고 컴포넌트 위에 바로 배치하기로 컨벤션 정했어서 수정해주세요!
There was a problem hiding this comment.
헉! 내부로 배치 수정했습니다 :)
| onClose, | ||
| ...popupProps | ||
| }: PopupContainerProps) => { | ||
| // ESC 키로 닫는 것 정도 (외부 클릭은 안되게! : 어차피 x박스나 취소버튼이 있음) |
There was a problem hiding this comment.
그렇다면 모달/팝업은 요소 외부 클릭 시 닫히는 경우가 없나요? 기/디 분들과 상의는 필요 없는지 궁금합니다!
There was a problem hiding this comment.
확인해보니! 닫기나 x버튼 이외의 외부요소 클릭 제어는 필요없다고 하십니다!
| useEffect(() => { | ||
| if (!isOpen) return; | ||
| const handleEsc = (e: KeyboardEvent) => { | ||
| if (e.key === 'Escape') onClose(); | ||
| }; | ||
| window.addEventListener('keydown', handleEsc); | ||
| return () => window.removeEventListener('keydown', handleEsc); | ||
| }, [isOpen, onClose]); |
There was a problem hiding this comment.
재사용이 다른 곳에서도 된다면 Hook으로 분리해도 좋겠네요 👍
| @@ -0,0 +1,19 @@ | |||
| export type PopupType = 'input' | 'subtext' | 'default'; | |||
|
|
|||
| export interface BasePopupProps { | |||
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
package.json (1)
24-26: 브라우저/커버리지 테스트 스택 추가 좋습니다. 실행 스크립트와 CI 훅을 함께 넣어 운영 가능성을 높이세요.패키지 추가만으로는 활용이 어려우니, 루트 스크립트를 보강해 바로 사용할 수 있게 해두면 좋습니다.
- 제안 스크립트(루트 package.json의 "scripts" 보강):
"scripts": { "build": "turbo run build", "dev": "turbo run dev", "lint": "turbo run lint", "format": "prettier --write \"**/*.{ts,tsx,md}\"", "check-types": "turbo run check-types", - "test": "turbo run test", + "test": "turbo run test", + "test:unit": "turbo run test:unit --filter=...design-system", + "test:browser": "vitest --browser chromium", + "test:cov": "vitest run --coverage", + "test:e2e": "playwright test", "storybook": "storybook dev -p 6006", "build-storybook": "storybook build" },
- CI 팁:
- Playwright 브라우저 설치: CI에서
npx playwright install --with-deps chromium실행.- 캐시: Playwright 다운로드 디렉토리와 Vitest 캐시를 캐싱하면 속도가 개선됩니다.
- Storybook 기반 시각 회귀나 상호작용 테스트를 엮을 경우,
storybook build && npx http-server storybook-static & playwright test와 같은 순서를 추천.Also applies to: 33-33
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
package.json(2 hunks)packages/design-system/src/components/popup/PopupContainer.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/design-system/src/components/popup/PopupContainer.tsx
| "dependencies": { | ||
| "@types/react-router-dom": "^5.3.3", | ||
| "react-router-dom": "^7.8.2" |
There was a problem hiding this comment.
react-router-dom v7는 자체 타입을 포함합니다 — @types/react-router-dom 추가는 불필요하며 타입 충돌 위험이 큽니다.
react-router-dom은 v6부터 타입 정의를 내장합니다. 현재 deps 조합(react-router-dom@^7 + @types/react-router-dom@^5)은 API/타입이 상이해 오류를 유발할 수 있습니다. 루트에 불필요한 타입 패키지는 제거해 주세요.
아래 최소 수정안을 제안합니다.
"dependencies": {
- "@types/react-router-dom": "^5.3.3",
"react-router-dom": "^7.8.2"
}📝 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.
| "dependencies": { | |
| "@types/react-router-dom": "^5.3.3", | |
| "react-router-dom": "^7.8.2" | |
| "dependencies": { | |
| "react-router-dom": "^7.8.2" |
🤖 Prompt for AI Agents
In package.json around lines 44-46, the project lists "@types/react-router-dom"
alongside "react-router-dom@^7", which causes type conflicts because
react-router-dom v7 already includes its own TypeScript definitions; remove the
"@types/react-router-dom" entry from dependencies (or devDependencies if
present), run yarn/npm install to update lockfile, search codebase for any
direct references to the removed package and update CI/containers to use the
refreshed lockfile and rebuild types to ensure no remaining type errors.
💡 Verification agent
🧩 Analysis chain
루트 의존성 배치는 PNPM 워크스페이스 격리 원칙과 어긋날 수 있습니다. 실제 사용 패키지로 옮기는 것을 권장합니다.
design-system의 팝업 작업과 직접 관련 없는 라우터를 루트에 두면, 하위 패키지가 암묵적으로 루트에 의존하는(hoist 전제) 안티패턴이 생길 수 있습니다. react-router-dom을 사용하는 앱(예: packages/web-app)이나 스토리북을 실행하는 특정 워크스페이스의 package.json으로 이동해 주세요. 사용 위치가 불명확하면 아래 스크립트로 참조 지점을 확인해 보세요.
🏁 Script executed:
#!/bin/bash
# repo 루트에서 실행: react-router-dom 사용/선언 위치를 확인
set -euo pipefail
echo "== import/require 사용 위치 조회 =="
rg -nP -C2 --type=ts --type=tsx '\bfrom\s+[\'"]react-router-dom[\'"]|\brequire\([\'"]react-router-dom[\'"]\)' || true
echo "== 워크스페이스들의 package.json 의존성 선언 조회 =="
fd package.json -t f | xargs -I{} jq -r '."name" as $n | .dependencies["react-router-dom"]? as $v | select($v!=null) | "\($n): \($v)"' {} \
| sort || trueLength of output: 198
🏁 Script executed:
#!/bin/bash
# repo 루트에서 실행: react-router-dom 사용/선언 위치를 확인
set -euo pipefail
echo "== import/require 사용 위치 조회 =="
rg -n -C2 --type=ts --type=tsx 'from.*react-router-dom|require.*react-router-dom' || echo "No matches found"
echo "== 워크스페이스들의 package.json 의존성 선언 조회 =="
fd package.json -t f | while read file; do
if jq -e '.dependencies["react-router-dom"]' "$file" >/dev/null 2>&1; then
name=$(jq -r '.name // "unknown"' "$file")
version=$(jq -r '.dependencies["react-router-dom"]' "$file")
echo "$name: $version (in $file)"
fi
done
echo "== devDependencies도 확인 =="
fd package.json -t f | while read file; do
if jq -e '.devDependencies["react-router-dom"]' "$file" >/dev/null 2>&1; then
name=$(jq -r '.name // "unknown"' "$file")
version=$(jq -r '.devDependencies["react-router-dom"]' "$file")
echo "$name: $version (devDep in $file)"
fi
doneLength of output: 2353
루트 package.json에서 react-router-dom 제거
루트(pinback-client) 패키지는 해당 의존성을 직접 사용하지 않으므로 삭제하고, 실제 사용하는 워크스페이스(apps/client 등)에만 선언하세요.
🤖 Prompt for AI Agents
In package.json around lines 44-46, the root package declares react-router-dom
and @types/react-router-dom even though pinback-client doesn't use them; remove
these entries from the root "dependencies" block and instead add/ensure
react-router-dom (and @types/react-router-dom if needed for TypeScript) are
declared in the workspace package(s) that actually use them (e.g., apps/client)
and run the workspace package manager install (npm/yarn/pnpm) in those
workspaces to update lockfiles; also verify no imports in the root reference
react-router-dom before committing.
2 participants
📌 Related Issues
✅ 체크 리스트
📄 Tasks
⭐ PR Point (To Reviewer)
props의 범위
짜면서 고민이였던 게, 이 팝업에 넘기는 props가 너무 많나..?에 대한 고민이 있었습니당
근데 그렇다고 팝업 밖에서 전달될 props 줄이기 위해, 팝업 자체 내에서 에러 검출이나 버튼 명 분기를 할까 고민도 했습니다.
ex. 서브텍스트가 있는 팝업일 경우 : "확인"/"닫기" 등으로 지정 / 인풋이 있는 팝업일 경우 : "수정"/"닫기"로 지정?
하지만 서비스에서의 팝업을 유연하게 변형하고 확장성 입장에서는 그냥 버튼 텍스트나 에러메시지도 밖에서 직접 props로 받는게 나을 것 같았습니다! 좋은 의견이 있다면 공유해주세요!
createPortal 활용
Important
createPortal의 도입 이유
이렇게 쓰면, 전체 body 돔 요소의 위에서 팝업이 뜨게 된다라는 의미!
if (!isOpen) return null;앞에서 isOpen팝업이 켜져있지 않으면 return 내보내고, 켜져있으면 팝업 노출이 isOpen 불리안으로 끄고 키고 전체를 제어해요!
📷 Screenshot
Summary by CodeRabbit