[REFACTOR] Accordion 컴포넌트의 defaultValue prop 지원#85
Conversation
WalkthroughAccordion 루트 컴포넌트에 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev
participant Story as Storybook Story
participant Accordion as AccordionRoot
participant State as openItems State
Note over Story,Accordion: 스토리에서 Accordion을 defaultValue로 렌더링
Story->>Accordion: render(defaultValue=['item-2'])
Accordion->>State: initialize openItems from defaultValue
State-->>Accordion: openItems = ['item-2']
Accordion->>Accordion: render items with initial open state
Note right of Accordion: 기존 토글 로직은 변경 없음
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8분 Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ 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 |
|
Update: 2025년 10월 16일 17시 21분 26초 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/shared/ui/Accordion/Accordion.tsx (1)
31-49: 단일 모드에서 여러 기본값에 대한 유효성 검사를 고려하세요.
type="single"일 때defaultValue에 여러 항목이 포함되면, 초기 렌더링에서 여러 항목이 열린 상태로 표시되어 단일 선택 제약을 위반합니다. 사용자가 상호작용하면 단일 항목으로 축소되지만, 초기 상태의 불일치가 혼란을 줄 수 있습니다.초기화 시 유효성 검사를 추가하는 것을 고려하세요:
- const defaultOpenItem = defaultValue ? defaultValue : []; + const defaultOpenItem = defaultValue ?? []; + + // Validate: for single mode, only use the first item + const initialOpenItems = type === 'single' && defaultOpenItem.length > 1 + ? [defaultOpenItem[0]] + : defaultOpenItem; + - const [openItems, setOpenItems] = useState<string[]>(defaultOpenItem); + const [openItems, setOpenItems] = useState<string[]>(initialOpenItems);또는 개발 모드에서 경고를 표시할 수 있습니다:
if (process.env.NODE_ENV === 'development' && type === 'single' && defaultOpenItem.length > 1) { console.warn('Accordion: defaultValue should contain only one item when type is "single"'); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/shared/ui/Accordion/Accordion.stories.tsx(2 hunks)src/shared/ui/Accordion/Accordion.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/shared/ui/Accordion/Accordion.stories.tsx (2)
src/shared/ui/Accordion/Accordion.tsx (1)
AccordionItem(103-172)src/shared/ui/Accordion/index.ts (1)
AccordionItem(2-2)
⏰ 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: preview-storybook
| export const DefaultValueAccordion: Story = { | ||
| render: () => { | ||
| return ( | ||
| <Accordion type="multiple" defaultValue={['item-2']}> | ||
| <AccordionItem value="item-1" isArrow={false} trigger={<div>질문 1</div>}> | ||
| <div>내용</div> | ||
| </AccordionItem> | ||
| <AccordionItem value="item-2" isArrow={false} trigger={<div>질문 1</div>}> | ||
| <div>내용</div> | ||
| </AccordionItem> | ||
| </Accordion> | ||
| ); | ||
| }, | ||
| }; |
There was a problem hiding this comment.
스토리 코드는 잘 작동하지만, trigger 텍스트에 오타가 있습니다.
86번 라인에서 두 번째 AccordionItem의 trigger가 "질문 1"로 되어 있는데, "질문 2"로 수정해야 합니다. 첫 번째 아이템과 동일한 텍스트가 표시되어 혼란을 줄 수 있습니다.
다음 diff를 적용하여 수정하세요:
- <AccordionItem value="item-2" isArrow={false} trigger={<div>질문 1</div>}>
+ <AccordionItem value="item-2" isArrow={false} trigger={<div>질문 2</div>}>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const DefaultValueAccordion: Story = { | |
| render: () => { | |
| return ( | |
| <Accordion type="multiple" defaultValue={['item-2']}> | |
| <AccordionItem value="item-1" isArrow={false} trigger={<div>질문 1</div>}> | |
| <div>내용</div> | |
| </AccordionItem> | |
| <AccordionItem value="item-2" isArrow={false} trigger={<div>질문 1</div>}> | |
| <div>내용</div> | |
| </AccordionItem> | |
| </Accordion> | |
| ); | |
| }, | |
| }; | |
| export const DefaultValueAccordion: Story = { | |
| render: () => { | |
| return ( | |
| <Accordion type="multiple" defaultValue={['item-2']}> | |
| <AccordionItem value="item-1" isArrow={false} trigger={<div>질문 1</div>}> | |
| <div>내용</div> | |
| </AccordionItem> | |
| <AccordionItem value="item-2" isArrow={false} trigger={<div>질문 2</div>}> | |
| <div>내용</div> | |
| </AccordionItem> | |
| </Accordion> | |
| ); | |
| }, | |
| }; |
🤖 Prompt for AI Agents
In src/shared/ui/Accordion/Accordion.stories.tsx around lines 79 to 92, the
second AccordionItem's trigger text is incorrect — it currently reads "질문 1" on
line ~86 but should be "질문 2"; update that trigger prop to "질문 2" so each item
displays the correct, distinct label in the story.
| /** | ||
| * The default value of the Accordion item that should be open on initial render. | ||
| */ | ||
| defaultValue?: string[]; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
단일 모드에서 타입 안정성을 개선할 수 있습니다.
defaultValue가 string[] 타입으로 정의되어 있지만, type="single"일 때는 의미상 단일 값만 허용되어야 합니다. 현재 구현은 작동하지만, 타입 시스템을 통해 이를 강제하면 더 명확한 API를 제공할 수 있습니다.
타입을 조건부로 정의하는 것을 고려해보세요:
type AccordionRootProps = {
type?: 'single' | 'multiple';
children: ReactNode;
className?: string;
} & (
| { type?: 'single'; defaultValue?: string }
| { type?: 'multiple'; defaultValue?: string[] }
);또는 더 간단하게 유니온 타입을 사용할 수 있습니다:
type AccordionRootProps = {
type?: 'single' | 'multiple';
children: ReactNode;
defaultValue?: string | string[];
className?: string;
};🤖 Prompt for AI Agents
In src/shared/ui/Accordion/Accordion.tsx around lines 20 to 23, the prop
defaultValue is currently typed as string[] but should be conditionally typed to
reflect the component's type prop; update the AccordionRootProps so that when
type is "single" defaultValue is a string and when type is "multiple"
defaultValue is a string[] (either by using a discriminated union with two
variants or by using defaultValue?: string | string[] for a simpler change), and
ensure any usages and forwarded props/interfaces are updated to match the new
prop type so TypeScript enforces the correct shape in single vs multiple modes.
| ...props | ||
| }: AccordionRootProps) { | ||
| const [openItems, setOpenItems] = useState<string[]>([]); | ||
| const defaultOpenItem = defaultValue ? defaultValue : []; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
nullish coalescing 연산자로 간소화할 수 있습니다.
현재 삼항 연산자 대신 nullish coalescing 연산자(??)를 사용하면 코드가 더 간결하고 의도가 명확해집니다.
다음 diff를 적용하세요:
- const defaultOpenItem = defaultValue ? defaultValue : [];
+ const defaultOpenItem = defaultValue ?? [];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const defaultOpenItem = defaultValue ? defaultValue : []; | |
| const defaultOpenItem = defaultValue ?? []; |
🤖 Prompt for AI Agents
In src/shared/ui/Accordion/Accordion.tsx around line 38, replace the ternary
fallback "const defaultOpenItem = defaultValue ? defaultValue : [];" with a
nullish coalescing fallback so the variable uses defaultValue unless it is null
or undefined, otherwise an empty array; update the line to use the ?? operator
and keep the original variable name and intended type.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/shared/ui/Accordion/Accordion.stories.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/shared/ui/Accordion/Accordion.stories.tsx (2)
src/shared/ui/Accordion/Accordion.tsx (1)
AccordionItem(103-172)src/shared/ui/Accordion/index.ts (1)
AccordionItem(2-2)
🔇 Additional comments (1)
src/shared/ui/Accordion/Accordion.stories.tsx (1)
79-92: 잘 구현된 스토리입니다.
DefaultValueAccordion스토리가 새로운defaultValueprop의 동작을 명확하게 보여주고 있습니다.defaultValue={['item-2']}로 설정하여 두 번째 아이템이 초기 렌더링 시 열린 상태로 표시되는 것을 잘 시연하고 있습니다.
| render: () => { | ||
| return ( | ||
| <Accordion type="single" defaultValue={['item-2']}> | ||
| <AccordionItem value="item-1" isArrow={false} trigger="defualtValue를 설정하지 않은 요소"> |
There was a problem hiding this comment.
trigger 텍스트의 오타를 수정하세요.
83번과 86번 라인의 trigger 텍스트에서 "defualtValue"를 "defaultValue"로 수정해야 합니다. 철자 오류가 사용자에게 노출되는 스토리북 문서에 표시됩니다.
다음 diff를 적용하여 수정하세요:
- <AccordionItem value="item-1" isArrow={false} trigger="defualtValue를 설정하지 않은 요소">
+ <AccordionItem value="item-1" isArrow={false} trigger="defaultValue를 설정하지 않은 요소">
초기 render시 조회되지 않아요
</AccordionItem>
- <AccordionItem value="item-2" isArrow={false} trigger="defualtValue를 설정한 요소">
+ <AccordionItem value="item-2" isArrow={false} trigger="defaultValue를 설정한 요소">
초기 render시 조회돼요
</AccordionItem>Also applies to: 86-86
🤖 Prompt for AI Agents
In src/shared/ui/Accordion/Accordion.stories.tsx around lines 83 and 86, there
is a spelling mistake in the trigger text ("defualtValue") exposed in Storybook;
update both occurrences on lines 83 and 86 to "defaultValue" so the displayed
trigger text reads correctly.
2 participants
🔥 연관 이슈
🚀 작업 내용
Accordion의defaultValue속성에AccordionItem의value를 설정해주면 초기 화면 렌더시 open되어져있는 상태를 구현 가능합니다.Summary by CodeRabbit
릴리스 노트
새로운 기능
문서