Merged
Conversation
📝 WalkthroughWalkthrough스토리북용으로 여러 공통 UI 컴포넌트(Alert, Badge, DropdownMenu)를 추가하고, Button/Input/Modal에 대한 스토리를 작성했으며 글로벌 CSS에 상태 색상 커스텀 프로퍼티 두 개를 추가했습니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
src/components/common/alert/Alert.tsx (1)
29-42: Alert 컴포넌트에 ARIArole속성 추가를 권장합니다.스크린 리더 사용자를 위해 Alert 컴포넌트에 적절한 ARIA role을 설정해 주세요. variant에 따라
role="alert"(danger, warning) 또는role="status"(info, success)를 적용하면 접근성이 크게 향상됩니다.🔧 variant별 role 매핑 예시
+ const roleMap: Record<TAlertVariant, "alert" | "status"> = { + info: "status", + success: "status", + warning: "alert", + danger: "alert", + }; + return ( <div + role={roleMap[variant]} className={twMerge(base, variantClasses[variant], className)} {...rest} >As per coding guidelines,
src/**: "7. 접근성: 시맨틱 HTML, ARIA 속성 사용 확인."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/alert/Alert.tsx` around lines 29 - 42, The Alert component should set an ARIA role on the outer div based on the variant prop to improve screen-reader accessibility: in the Alert function/component, compute a role string (e.g., if variant is "danger" or "warning" use "alert", if "info" or "success" use "status", default to undefined or "status") and add it to the returned root div alongside className/ {...rest}; reference the component return (the outer <div> that uses twMerge(base, variantClasses[variant], className)) and the prop name variant to locate where to implement this mapping.src/components/common/modal/Modal.stories.tsx (1)
32-78: 다양한 사이즈/패딩 조합에 대한 스토리 추가를 고려해 주세요.현재
Default스토리 하나만 있는데, Modal의IModalProps인터페이스(src/components/common/modal/Modal.tsx:13-23)에 정의된size(sm|md|lg|xl),padding,hideCloseButton,disableOverlayClick등 다양한 props 조합을 보여주는 스토리가 있으면 컴포넌트 문서화에 더 효과적입니다. 초기 세팅이니 추후 추가해도 무방합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/modal/Modal.stories.tsx` around lines 32 - 78, Add additional Storybook stories in Modal.stories.tsx that demonstrate the different IModalProps combinations: create separate exported stories (e.g., Small, Large, XLarge, NoPadding, HiddenCloseButton, NoOverlayClose) that reuse the existing Default render pattern (EnsureModalRoot, button to open, Modal component) but pass size="sm"|"lg"|"xl", different padding values, hideCloseButton={true}, and disableOverlayClick={true} to the Modal component so users can see each variant; keep each story self-contained and named to reflect the prop combination.src/components/common/dropdownmenu/DropdownMenu.tsx (2)
33-70: 접근성: 드롭다운 메뉴에 WAI-ARIA 속성이 누락되어 있습니다.코딩 가이드라인에 따라 시맨틱 HTML과 ARIA 속성 사용을 확인해야 합니다. 현재 드롭다운 패턴에 필요한 핵심 접근성 속성이 빠져 있습니다:
- 트리거 버튼에
aria-expanded,aria-haspopup="menu"누락- 메뉴 패널에
role="menu"누락- 각 메뉴 아이템에
role="menuitem"누락Escape키로 메뉴 닫기, 화살표 키 탐색 등 키보드 지원 부재🔧 최소한의 ARIA 속성 추가 예시
- <button type="button" onClick={() => setOpen((v) => !v)}> + <button + type="button" + aria-haspopup="menu" + aria-expanded={open} + onClick={() => setOpen((v) => !v)} + > {trigger} </button> {open && ( - <div className="absolute left-0 mt-2 w-72 rounding-15 bg-brand-200 py-3 px-1 shadow-Medium"> - <div className="space-y-1"> + <div role="menu" className="absolute left-0 mt-2 w-72 rounding-15 bg-brand-200 py-3 px-1 shadow-Medium"> + <div className="space-y-1"> {items.map((it, idx) => ( <div key={idx} className="px-2"> <button type="button" + role="menuitem" onClick={() => {As per coding guidelines,
src/**: "7. 접근성: 시맨틱 HTML, ARIA 속성 사용 확인."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/dropdownmenu/DropdownMenu.tsx` around lines 33 - 70, The DropdownMenu component is missing accessibility semantics and keyboard support; update the trigger button (the element rendering {trigger}) to include aria-haspopup="menu", aria-expanded={open}, and aria-controls pointing to the menu id, add role="menu" and an id to the menu panel (the div rendered when open) and set role="menuitem" on each menu item button (the elements inside items.map), implement keyboard handlers on the trigger and menu to close on Escape and navigate items with ArrowUp/ArrowDown (manage focus via refs for each item and ensure setOpen(false) still runs on item activation), and ensure focus is moved into the menu when opened and returned to the trigger when closed.
22-31:useEffect리스너를open상태에 따라 조건부로 등록하면 불필요한 이벤트 처리를 줄일 수 있습니다.메뉴가 닫혀 있을 때도
mousedown리스너가 항상 등록되어 있습니다.open을 의존성으로 추가하고 열린 상태에서만 리스너를 등록하면 성능상 약간 이점이 있습니다.♻️ 조건부 리스너 등록
useEffect(() => { + if (!open) return; const onDocClick = (e: MouseEvent) => { if (!ref.current) return; if (!ref.current.contains(e.target as Node)) setOpen(false); }; document.addEventListener("mousedown", onDocClick); return () => { document.removeEventListener("mousedown", onDocClick); }; - }, []); + }, [open]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/dropdownmenu/DropdownMenu.tsx` around lines 22 - 31, The effect currently always registers the document "mousedown" listener; update the useEffect that defines onDocClick to depend on the open state and only attach the listener when open is true: add open to the dependency array, wrap the document.addEventListener call in a conditional (if (open) { ... }), and ensure the cleanup still removes the listener (or is a no-op when not attached). This change touches the useEffect block that references ref, onDocClick, and setOpen so the listener is only active while the dropdown is open.src/components/common/badge/Badge.tsx (2)
4-12:TBadgeVariant타입 export 권장
TBadgeVariant와TBadgeSize를 export하지 않으면, 사용 측에서 API 상태 값을 badge variant로 매핑할 때 직접 string 리터럴을 하드코딩하게 됩니다. 외부에서 타입 안전하게 참조할 수 있도록 export를 추가하는 것을 권장합니다.♻️ 제안
-type TBadgeVariant = "syncing" | "success" | "inactive" | "running" | "stopped"; +export type TBadgeVariant = "syncing" | "success" | "inactive" | "running" | "stopped"; -type TBadgeSize = "sm" | "md"; +export type TBadgeSize = "sm" | "md"; -interface IBadgeProps extends HTMLAttributes<HTMLSpanElement> { +export interface IBadgeProps extends HTMLAttributes<HTMLSpanElement> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/badge/Badge.tsx` around lines 4 - 12, Export the badge type aliases so consumers can reference them for type-safe mappings: add export to TBadgeVariant and TBadgeSize (e.g., export type TBadgeVariant = ... and export type TBadgeSize = ...); optionally export IBadgeProps if external components need the prop shape. Update the declarations for TBadgeVariant, TBadgeSize (and IBadgeProps if desired) in Badge.tsx so they are exported for external use.
22-34:sizeClasses,variantClasses를 컴포넌트 외부로 이동 권장두 객체는 props나 state에 의존하지 않는 순수 상수인데 함수 본문 내에 정의되어 있어, 렌더링마다 새 객체가 생성됩니다. 컴포넌트 외부로 이동하면 불필요한 메모리 할당을 피할 수 있습니다.
♻️ 제안
+const SIZE_CLASSES: Record<TBadgeSize, string> = { + sm: "h-7 px-3 py-1 font-caption", + md: "h-9 px-4 py-1 font-body2", +}; + +const VARIANT_CLASSES: Record<TBadgeVariant, string> = { + syncing: "bg-status-yellow/30 text-status-yellow border border-status-yellow", + success: "bg-status-green/30 text-status-green border border-status-green", + inactive: "bg-status-red/30 text-status-red border border-status-red/40", + running: "bg-status-blue/30 text-status-blue border border-status-blue/40", + stopped: "bg-brand-300 text-text-sub border border-text-placeholder/40", +}; + export default function Badge({ variant = "success", size = "md", leftIcon, className, children, ...rest }: IBadgeProps) { - const sizeClasses: Record<TBadgeSize, string> = { - sm: "h-7 px-3 py-1 font-caption", - md: "h-9 px-4 py-1 font-body2", - }; - - const variantClasses: Record<TBadgeVariant, string> = { - syncing: "bg-status-yellow/30 text-status-yellow border border-status-yellow", - success: "bg-status-green/30 text-status-green border border-status-green", - inactive: "bg-status-red/30 text-status-red border border-status-red/40", - running: "bg-status-blue/30 text-status-blue border border-status-blue/40", - stopped: "bg-brand-300 text-text-sub border border-text-placeholder/40", - }; return ( <span className={twMerge( "inline-flex items-center gap-2 rounded-full whitespace-nowrap", - sizeClasses[size], - variantClasses[variant], + SIZE_CLASSES[size], + VARIANT_CLASSES[variant], className, )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/badge/Badge.tsx` around lines 22 - 34, Move the pure constant maps sizeClasses and variantClasses out of the Badge component so they are defined once at module scope (keeping their types Record<TBadgeSize, string> and Record<TBadgeVariant, string>), then update the component to reference these top-level constants instead of recreating them on each render; locate the definitions of sizeClasses and variantClasses in Badge.tsx and cut them to the file top-level (above the Badge component) and ensure any exported type names TBadgeSize and TBadgeVariant remain available for the record typings.src/components/common/badge/Badge.stories.tsx (2)
1-11:import from "@storybook/react-vite"임포트 방식 확인
@storybook/react-vite에서Meta와StoryObj를 임포트하는 것은 Storybook 8의 공식 마이그레이션 가이드에서 권장하는 방식입니다. 현재 코드는 올바른 임포트를 사용하고 있습니다.다만, meta 선언 시
satisfies연산자를 활용하면 타입 추론이 더 엄격해집니다. Storybook 공식 문서에서는satisfies Meta<typeof Component>패턴과StoryObj<typeof meta>를 함께 사용하는 것을 권장합니다.♻️ 제안 (타입 안전성 강화)
-const meta: Meta<typeof Badge> = { +const meta = { title: "Common/Badge", component: Badge, parameters: { layout: "centered" }, -}; +} satisfies Meta<typeof Badge>; export default meta; -type TBadgeStory = StoryObj<typeof Badge>; +type TBadgeStory = StoryObj<typeof meta>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/badge/Badge.stories.tsx` around lines 1 - 11, The meta declaration should use the TypeScript satisfies pattern to improve type safety: change the meta object (symbol: meta) so it is declared with the satisfies Meta<typeof Badge> operator, and adjust story typings to use StoryObj<typeof meta> (symbol: StoryObj) instead of StoryObj<typeof Badge>; keep exporting meta as the default export (export default meta) so Storybook picks it up. Ensure imports (Meta, StoryObj) remain from "@storybook/react-vite" and update any existing story consts to reference StoryObj<typeof meta>.
14-40:leftIconprop을 보여주는 스토리 누락
Badge컴포넌트의 주요 기능인leftIconprop이 어떤 스토리에서도 시연되지 않습니다. 아이콘을 함께 사용하는 케이스는 실무에서 흔하게 사용될 수 있으므로, 스토리에 예시를 추가하면 컴포넌트 문서로서의 가치가 높아집니다.♻️ 제안
+import { CircleIcon } from "lucide-react"; // 프로젝트에서 사용 중인 아이콘 라이브러리로 교체 + export const Syncing: TBadgeStory = { args: { variant: "syncing", children: "동기화중" }, }; + +export const WithIcon: TBadgeStory = { + args: { + variant: "running", + leftIcon: <CircleIcon size={12} />, + children: "운영중", + }, +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/badge/Badge.stories.tsx` around lines 14 - 40, The stories currently (Syncing, Success, Inactive, Running, Stopped, SizeSm, SizeMd) never demonstrate the Badge component's leftIcon prop; add a new story (e.g., WithLeftIcon or LeftIconExample) of type TBadgeStory that supplies a leftIcon arg and children to showcase the icon+label combination, import a small icon (from your icon set used elsewhere) at the top of Badge.stories.tsx, and set args like { variant: "running", leftIcon: <IconName />, children: "아이콘 포함" } so the storybook renders the leftIcon usage.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/common/alert/Alert.tsx`:
- Around line 22-27: The alert variant mapping in Alert.tsx is incorrect: update
the variantClasses Record<TAlertVariant, string> so the "success" key maps to
"text-status-green" (not "text-status-blue") to use the newly added
--color-status-green; ensure no other places in the Alert component (or
TAlertVariant usages) still expect blue for success and adjust tests/snapshots
if any reference the old class.
In `@src/components/common/badge/Badge.tsx`:
- Around line 36-49: The Badge component currently renders a plain <span> (root
span with {...rest}) which can omit ARIA semantics; set a sensible default
accessible role by adding role="status" (and optionally aria-live="polite") on
the root span while still allowing consumers to override via {...rest}, and
update any docs/stories to tell consumers to pass aria-label when using the
badge as a simple label; locate the root span in Badge.tsx (the element applying
twMerge, sizeClasses[size], variantClasses[variant], className, and {...rest})
and add the default role there.
In `@src/components/common/dropdownmenu/DropdownMenu.stories.tsx`:
- Around line 14-24: The Default story for DropdownMenu passes a nested <button>
via the trigger prop causing nested buttons; update the Default story's render
so trigger uses a non-interactive element (e.g., <span> or <div>) instead of
<button>—keep the same inner text ("...") and handlers if any—so the
DropdownMenu component (which already wraps the trigger in a button) does not
produce nested button elements.
In `@src/components/common/dropdownmenu/DropdownMenu.tsx`:
- Around line 35-37: The DropdownMenu currently wraps the passed-in trigger
ReactNode with a <button> (see the onClick using setOpen), which can cause
nested button elements if trigger is itself a button; update DropdownMenu to
either render the trigger node directly and attach the click handler to it
(e.g., cloneElement to inject onClick) or require/validate that trigger is a
non-button element; modify the component around the trigger rendering (where
setOpen is used) to avoid always wrapping in a <button>—use
React.cloneElement(trigger, { onClick: () => setOpen(v => !v) }) or render
trigger directly and attach a wrapper non-button (e.g., <span>) to toggle, and
update DropdownMenu.stories.tsx if needed to use a non-interactive trigger.
---
Nitpick comments:
In `@src/components/common/alert/Alert.tsx`:
- Around line 29-42: The Alert component should set an ARIA role on the outer
div based on the variant prop to improve screen-reader accessibility: in the
Alert function/component, compute a role string (e.g., if variant is "danger" or
"warning" use "alert", if "info" or "success" use "status", default to undefined
or "status") and add it to the returned root div alongside className/ {...rest};
reference the component return (the outer <div> that uses twMerge(base,
variantClasses[variant], className)) and the prop name variant to locate where
to implement this mapping.
In `@src/components/common/badge/Badge.stories.tsx`:
- Around line 1-11: The meta declaration should use the TypeScript satisfies
pattern to improve type safety: change the meta object (symbol: meta) so it is
declared with the satisfies Meta<typeof Badge> operator, and adjust story
typings to use StoryObj<typeof meta> (symbol: StoryObj) instead of
StoryObj<typeof Badge>; keep exporting meta as the default export (export
default meta) so Storybook picks it up. Ensure imports (Meta, StoryObj) remain
from "@storybook/react-vite" and update any existing story consts to reference
StoryObj<typeof meta>.
- Around line 14-40: The stories currently (Syncing, Success, Inactive, Running,
Stopped, SizeSm, SizeMd) never demonstrate the Badge component's leftIcon prop;
add a new story (e.g., WithLeftIcon or LeftIconExample) of type TBadgeStory that
supplies a leftIcon arg and children to showcase the icon+label combination,
import a small icon (from your icon set used elsewhere) at the top of
Badge.stories.tsx, and set args like { variant: "running", leftIcon: <IconName
/>, children: "아이콘 포함" } so the storybook renders the leftIcon usage.
In `@src/components/common/badge/Badge.tsx`:
- Around line 4-12: Export the badge type aliases so consumers can reference
them for type-safe mappings: add export to TBadgeVariant and TBadgeSize (e.g.,
export type TBadgeVariant = ... and export type TBadgeSize = ...); optionally
export IBadgeProps if external components need the prop shape. Update the
declarations for TBadgeVariant, TBadgeSize (and IBadgeProps if desired) in
Badge.tsx so they are exported for external use.
- Around line 22-34: Move the pure constant maps sizeClasses and variantClasses
out of the Badge component so they are defined once at module scope (keeping
their types Record<TBadgeSize, string> and Record<TBadgeVariant, string>), then
update the component to reference these top-level constants instead of
recreating them on each render; locate the definitions of sizeClasses and
variantClasses in Badge.tsx and cut them to the file top-level (above the Badge
component) and ensure any exported type names TBadgeSize and TBadgeVariant
remain available for the record typings.
In `@src/components/common/dropdownmenu/DropdownMenu.tsx`:
- Around line 33-70: The DropdownMenu component is missing accessibility
semantics and keyboard support; update the trigger button (the element rendering
{trigger}) to include aria-haspopup="menu", aria-expanded={open}, and
aria-controls pointing to the menu id, add role="menu" and an id to the menu
panel (the div rendered when open) and set role="menuitem" on each menu item
button (the elements inside items.map), implement keyboard handlers on the
trigger and menu to close on Escape and navigate items with ArrowUp/ArrowDown
(manage focus via refs for each item and ensure setOpen(false) still runs on
item activation), and ensure focus is moved into the menu when opened and
returned to the trigger when closed.
- Around line 22-31: The effect currently always registers the document
"mousedown" listener; update the useEffect that defines onDocClick to depend on
the open state and only attach the listener when open is true: add open to the
dependency array, wrap the document.addEventListener call in a conditional (if
(open) { ... }), and ensure the cleanup still removes the listener (or is a
no-op when not attached). This change touches the useEffect block that
references ref, onDocClick, and setOpen so the listener is only active while the
dropdown is open.
In `@src/components/common/modal/Modal.stories.tsx`:
- Around line 32-78: Add additional Storybook stories in Modal.stories.tsx that
demonstrate the different IModalProps combinations: create separate exported
stories (e.g., Small, Large, XLarge, NoPadding, HiddenCloseButton,
NoOverlayClose) that reuse the existing Default render pattern (EnsureModalRoot,
button to open, Modal component) but pass size="sm"|"lg"|"xl", different padding
values, hideCloseButton={true}, and disableOverlayClick={true} to the Modal
component so users can see each variant; keep each story self-contained and
named to reflect the prop combination.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/components/common/dropdownmenu/DropdownMenu.stories.tsx (2)
14-24: 이전 리뷰 이슈 해결 확인 및 추가 스토리 변형 제안
trigger에<button>대신<span>을 사용한 것이 확인되었습니다.DropdownMenu.tsx에서 트리거를<div role="button">으로 감싸기 때문에 현재처럼<span>사용이 접근성 측면에서 올바릅니다.다만
Default스토리 하나만 존재하기 때문에, 아이콘이 있는 메뉴 항목이나 전체 비활성 상태 등 실제 사용 사례를 추가 스토리로 커버하면 Storybook의 시각적 테스트 효용이 높아집니다.✨ 추가 스토리 변형 예시
+import { Home, Settings } from "lucide-react"; // 또는 프로젝트의 아이콘 라이브러리 + +export const WithIcons: TDropdownMenuStory = { + render: () => ( + <DropdownMenu + trigger={<span>⚙</span>} + items={[ + { label: "홈", icon: <Home size={16} />, active: true, onClick: () => alert("홈") }, + { label: "설정", icon: <Settings size={16} />, onClick: () => alert("설정") }, + ]} + /> + ), +}; + +export const AllInactive: TDropdownMenuStory = { + render: () => ( + <DropdownMenu + trigger={<span>···</span>} + items={[ + { label: "항목 A", onClick: () => alert("A") }, + { label: "항목 B", onClick: () => alert("B") }, + ]} + /> + ), +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/dropdownmenu/DropdownMenu.stories.tsx` around lines 14 - 24, Add additional Storybook variations to cover more real-world cases: create new exports (e.g., WithIcons and AllDisabled) alongside the existing Default TDropdownMenuStory and reuse the DropdownMenu component in each; for WithIcons provide items that include an icon property or visually distinct labels to demonstrate icon rendering, and for AllDisabled set all items' disabled (or equivalent) prop to true to show the fully-disabled state; keep the existing Default as-is (trigger as <span>) and ensure each new story follows the same shape as Default so Storybook picks them up.
5-12:satisfies연산자와StoryObj<typeof meta>패턴으로 전환하는 것을 권장합니다.현재
const meta: Meta<typeof DropdownMenu> = {...}형태의 명시적 타입 어노테이션 방식을 사용하고 있습니다. Storybook 공식 문서와 Storybook 10에서 도입된 권장 패턴은 더 나은 타입 안정성과 자동완성을 제공합니다.Storybook 10 마이그레이션 가이드에서는
@storybook/react가 아닌@storybook/react-vite에서Meta,StoryObj를 직접 임포트하는 방식을 명시적으로 권장하고 있습니다. 임포트 자체는 올바르지만,meta정의 방식과TDropdownMenuStory타입을 아래와 같이 개선하면 story 레벨의 args 기본값까지 타입 추론에 반영됩니다.♻️ 수정 제안: `satisfies` 패턴 적용
-const meta: Meta<typeof DropdownMenu> = { +const meta = { title: "Common/DropdownMenu", component: DropdownMenu, parameters: { layout: "centered" }, -}; +} satisfies Meta<typeof DropdownMenu>; export default meta; -type TDropdownMenuStory = StoryObj<typeof DropdownMenu>; +type TDropdownMenuStory = StoryObj<typeof meta>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/dropdownmenu/DropdownMenu.stories.tsx` around lines 5 - 12, Change the explicit type annotation on meta to use the TypeScript satisfies operator and switch the story type to StoryObj<typeof meta>: replace the current "const meta: Meta<typeof DropdownMenu> = {...}" with a plain const meta = {...} satisfies Meta<typeof DropdownMenu> so story-level args are inferred, and update "type TDropdownMenuStory = StoryObj<typeof DropdownMenu>;" to "type TDropdownMenuStory = StoryObj<typeof meta>;" (keep existing imports of Meta and StoryObj from `@storybook/react-vite`).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/common/dropdownmenu/DropdownMenu.stories.tsx`:
- Around line 14-24: Add additional Storybook variations to cover more
real-world cases: create new exports (e.g., WithIcons and AllDisabled) alongside
the existing Default TDropdownMenuStory and reuse the DropdownMenu component in
each; for WithIcons provide items that include an icon property or visually
distinct labels to demonstrate icon rendering, and for AllDisabled set all
items' disabled (or equivalent) prop to true to show the fully-disabled state;
keep the existing Default as-is (trigger as <span>) and ensure each new story
follows the same shape as Default so Storybook picks them up.
- Around line 5-12: Change the explicit type annotation on meta to use the
TypeScript satisfies operator and switch the story type to StoryObj<typeof
meta>: replace the current "const meta: Meta<typeof DropdownMenu> = {...}" with
a plain const meta = {...} satisfies Meta<typeof DropdownMenu> so story-level
args are inferred, and update "type TDropdownMenuStory = StoryObj<typeof
DropdownMenu>;" to "type TDropdownMenuStory = StoryObj<typeof meta>;" (keep
existing imports of Meta and StoryObj from `@storybook/react-vite`).
Seojegyeong
reviewed
Feb 19, 2026
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/components/common/dropdownmenu/DropdownMenu.tsx (2)
54-55:key={idx}대신 안정적인 식별자 사용을 권장합니다.현재
items배열이 정적이라면 큰 문제는 없지만, 향후 items가 동적으로 변경되거나 재정렬될 경우 React가 잘못된 DOM 업데이트를 할 수 있습니다.TMenuItem에 고유id필드를 추가하거나, 최소한key={it.label}처럼 의미 있는 값을 사용하는 것을 권장합니다.♻️ 안정적인 key 사용 제안 (label 기반 또는 id 추가)
TMenuItem타입에 id 추가 (선호):export type TMenuItem = { + id: string; label: string; icon?: React.ReactNode; onClick: () => void; active?: boolean; };- {items.map((it, idx) => ( - <div key={idx} className="px-2"> + {items.map((it) => ( + <div key={it.id} className="px-2">또는 id 추가 없이 label을 key로 사용 (label이 unique하다고 보장될 때만):
- {items.map((it, idx) => ( - <div key={idx} className="px-2"> + {items.map((it) => ( + <div key={it.label} className="px-2">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/dropdownmenu/DropdownMenu.tsx` around lines 54 - 55, The map in DropdownMenu.tsx currently uses key={idx} which can cause incorrect DOM updates; update the TMenuItem type to include a unique id (preferred) and change the map to use key={it.id}, or if you cannot add an id ensure you use a stable unique value such as key={it.label} (only if labels are guaranteed unique). Locate the items.map(...) usage inside DropdownMenu and replace the numeric index key with the chosen stable identifier and adjust TMenuItem where necessary.
28-31: 터치 기기 지원을 위해mousedown대신pointerdown사용을 고려하세요.
mousedown이벤트는 터치 전용 기기에서 발생하지 않아 외부 클릭으로 메뉴가 닫히지 않을 수 있습니다.pointerdown은 마우스와 터치를 모두 커버합니다.♻️ pointerdown으로 변경
- document.addEventListener("mousedown", onDocClick); return () => { - document.removeEventListener("mousedown", onDocClick); + document.addEventListener("pointerdown", onDocClick); + return () => { + document.removeEventListener("pointerdown", onDocClick); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/dropdownmenu/DropdownMenu.tsx` around lines 28 - 31, The document event listener currently uses "mousedown" which doesn't fire on touch-only devices; update the listener registration in DropdownMenu (where document.addEventListener("mousedown", onDocClick) and document.removeEventListener("mousedown", onDocClick) are used) to use "pointerdown" instead so both mouse and touch inputs are handled—replace both addEventListener and removeEventListener event names to "pointerdown" while keeping the same onDocClick handler.src/components/common/modal/Modal.stories.tsx (1)
65-72:alert()대신 Storybookfn()사용 권장PR 커밋 메시지에서 "fn 기반 액션 확인 환경 구성"을 명시하고 있는데, 확인 버튼의 핸들러에 브라우저 기본
alert()를 사용하면 Storybook의 Actions 패널에서 이벤트를 추적할 수 없습니다.♻️ 수정 제안
+import { fn } from "@storybook/test"; ... const meta: Meta<typeof Modal> = { title: "Common/Modal", component: Modal, parameters: { layout: "centered" }, + args: { + onClose: fn(), + }, }; ... <button type="button" - onClick={() => alert("확인")} + onClick={fn()} className="..." aria-label="모달 확인" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/common/modal/Modal.stories.tsx` around lines 65 - 72, Replace the browser alert used in the Modal story's confirm button with Storybook's action function so the click is tracked in the Actions panel: in Modal.stories.tsx import the action helper (e.g., import { action } from '@storybook/addon-actions') and change the button's onClick from () => alert("확인") to action('modal-confirm') (or use the story arg fn() if this story uses args) on the button element to emit a named action instead of calling alert().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/common/dropdownmenu/DropdownMenu.tsx`:
- Around line 51-82: The dropdown popup currently rendered when open (the div in
DropdownMenu that contains items.map) is missing ARIA roles and an id; add
role="menu" to that popup div and give it a stable id, then update the trigger
(the element that has aria-haspopup="menu") to include aria-controls pointing to
that id; also add role="menuitem" to each item button inside the map (the button
that calls it.onClick and setOpen(false)) and ensure focus/keyboard behavior
remains unchanged.
In `@src/components/common/input/Input.stories.tsx`:
- Around line 25-30: The exported story named Error shadows the global Error
constructor; rename the export (e.g., ErrorStory or ErrorState) and update any
references to that export to avoid colliding with the built-in Error. Locate the
TInputStory export object currently declared as export const Error and change
its identifier, keeping its args (error: true, helperText) intact and updating
any story imports/usages or Storybook references to the new name.
- Around line 32-38: The Success story currently passes value: "정상 입력" without
an onChange handler, causing React's controlled-input warning; update the
Success story (export const Success: TInputStory) to either replace value with
defaultValue: "정상 입력" or add an onChange noop plus readOnly if you want it
immutable (e.g., provide onChange: () => {} and/or readOnly: true), ensuring the
Input component is not treated as a controlled input without a change handler.
---
Duplicate comments:
In `@src/components/common/dropdownmenu/DropdownMenu.tsx`:
- Around line 42-47: The onKeyDown handler currently toggles the menu for
Enter/Space but omits Escape handling; update the onKeyDown in DropdownMenu to
also handle e.key === "Escape" by calling e.preventDefault(), calling
setOpen(false) to close the menu, and restoring focus to the menu trigger
element (use the existing trigger/button ref, e.g.,
triggerRef.current?.focus()); ensure the handler does not stop other key
behavior and that triggerRef is available/forwarded if not already.
---
Nitpick comments:
In `@src/components/common/dropdownmenu/DropdownMenu.tsx`:
- Around line 54-55: The map in DropdownMenu.tsx currently uses key={idx} which
can cause incorrect DOM updates; update the TMenuItem type to include a unique
id (preferred) and change the map to use key={it.id}, or if you cannot add an id
ensure you use a stable unique value such as key={it.label} (only if labels are
guaranteed unique). Locate the items.map(...) usage inside DropdownMenu and
replace the numeric index key with the chosen stable identifier and adjust
TMenuItem where necessary.
- Around line 28-31: The document event listener currently uses "mousedown"
which doesn't fire on touch-only devices; update the listener registration in
DropdownMenu (where document.addEventListener("mousedown", onDocClick) and
document.removeEventListener("mousedown", onDocClick) are used) to use
"pointerdown" instead so both mouse and touch inputs are handled—replace both
addEventListener and removeEventListener event names to "pointerdown" while
keeping the same onDocClick handler.
In `@src/components/common/modal/Modal.stories.tsx`:
- Around line 65-72: Replace the browser alert used in the Modal story's confirm
button with Storybook's action function so the click is tracked in the Actions
panel: in Modal.stories.tsx import the action helper (e.g., import { action }
from '@storybook/addon-actions') and change the button's onClick from () =>
alert("확인") to action('modal-confirm') (or use the story arg fn() if this story
uses args) on the button element to emit a named action instead of calling
alert().
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/common/input/Input.stories.tsx`:
- Around line 40-45: The Disabled story object (Disabled: TInputStory) passes a
controlled prop value without an onChange handler which triggers React's
controlled/uncontrolled warning; update the story to either replace value: "비활성"
with defaultValue: "비활성" to show an initial read-only value, or keep value but
add a noop onChange handler in the Disabled story args so the input is a
controlled component (adjust the Disabled story args accordingly).
Collaborator
|
P4: 확인했습니다! 수고하셨어요!! |
YermIm
approved these changes
Feb 19, 2026
Seojegyeong
approved these changes
Feb 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚨 관련 이슈
Closes #33
✨ 변경사항
✏️ 작업 내용
storybook@latest init)storybook/preview.ts에 프로젝트 전역 스타일 주입하여 Tailwind v4 스타일 적용MemoryRouter데코레이터 추가index.css에status-yellow,statuw-green컬러 추가😅 미완성 작업
N/A
📢 논의 사항 및 참고 사항
N/A
Summary by CodeRabbit
새로운 기능
문서
스타일