Skip to content

[REFACTOR] Accordion 컴포넌트의 defaultValue prop 지원 #85

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
yougyung merged 2 commits into from
Oct 17, 2025
Merged

[REFACTOR] Accordion 컴포넌트의 defaultValue prop 지원 #85

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2bfda60
refactor: defaultValue prop 지원
yougyung Oct 16, 2025
7257992
chore: 예시 text 변경
yougyung Oct 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/shared/ui/Accordion/Accordion.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export const NoneArrowAccordion: Story = {
);
},
};

export const InputAccordion: Story = {
render: () => {
return (
Expand All @@ -74,3 +75,18 @@ export const InputAccordion: Story = {
);
},
};

export const DefaultValueAccordion: Story = {
render: () => {
return (
<Accordion type="single" defaultValue={['item-2']}>
<AccordionItem value="item-1" isArrow={false} trigger="defualtValue를 설정하지 않은 요소">
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

초기 render시 조회되지 않아요
</AccordionItem>
<AccordionItem value="item-2" isArrow={false} trigger="defualtValue를 설정한 요소">
초기 render시 조회돼요
</AccordionItem>
</Accordion>
);
},
};
Comment on lines +79 to +92
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

스토리 코드는 잘 작동하지만, 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.

Suggested change
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.

9 changes: 8 additions & 1 deletion src/shared/ui/Accordion/Accordion.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ type AccordionRootProps = {
* The content of the Accordion, typically AccordionItem components.
*/
children: ReactNode;
/**
* The default value of the Accordion item that should be open on initial render.
*/
defaultValue?: string[];
Comment on lines +20 to +23
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.


/**
* Additional class names to apply to the AccordionRoot.
*/
Expand All @@ -27,9 +32,11 @@ export function AccordionRoot({
type = 'single',
className = '',
children,
defaultValue,
...props
}: AccordionRootProps) {
const [openItems, setOpenItems] = useState<string[]>([]);
const defaultOpenItem = defaultValue ? defaultValue : [];
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.

Suggested change
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.

const [openItems, setOpenItems] = useState<string[]>(defaultOpenItem);

const toggleItem = (value: string) => {
if (type === 'single') {
Expand Down