Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
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="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>
);
},
};
Comment on lines +79 to +92
Copy link
Copy Markdown

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

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

단일 모드에서 타입 안정성을 개선할 수 있습니다.

defaultValuestring[] 타입으로 정의되어 있지만, 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

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