Skip to content

Accordion: added openItems to AccordionToggleData #28431

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

Closed
kkakroo wants to merge 24 commits into from
Closed

Accordion: added openItems to AccordionToggleData #28431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
527faf5
added openItems to onToggle
kkakroo Jul 4, 2023
a82ea93
fixed errors in test file
kkakroo Jul 4, 2023
1643fd0
Merge branch 'master' into accordion-openItems
kkakroo Jul 6, 2023
f1917d3
Merge branch 'master' into accordion-openItems
kkakroo Jul 6, 2023
ef42f8d
Merge branch 'master' into accordion-openItems
kkakroo Jul 11, 2023
0aee605
Merge branch 'master' into accordion-openItems
kkakroo Jul 11, 2023
2ebad0a
Merge branch 'master' into accordion-openItems
kkakroo Jul 12, 2023
0d01df5
Merge branch 'master' into accordion-openItems
kkakroo Jul 13, 2023
49b5124
Merge branch 'master' into accordion-openItems
kkakroo Jul 15, 2023
ae16620
Merge branch 'master' into accordion-openItems
kkakroo Jul 17, 2023
8ecf2ca
Merge branch 'master' into accordion-openItems
kkakroo Jul 17, 2023
05b1a71
Merge branch 'master' into accordion-openItems
kkakroo Jul 18, 2023
4b3d794
Merge branch 'master' into accordion-openItems
kkakroo Jul 18, 2023
67ae7c4
Merge branch 'master' into accordion-openItems
kkakroo Jul 19, 2023
8fc806d
Merge branch 'master' into accordion-openItems
kkakroo Jul 20, 2023
5feb18a
Merge branch 'master' into accordion-openItems
kkakroo Jul 21, 2023
fa0a401
Merge branch 'master' into accordion-openItems
kkakroo Jul 22, 2023
e6a05a5
Merge branch 'master' into accordion-openItems
kkakroo Jul 24, 2023
ef115ac
Merge branch 'master' into accordion-openItems
kkakroo Jul 25, 2023
67dd3ba
Merge branch 'master' into accordion-openItems
kkakroo Jul 25, 2023
2159553
Merge branch 'master' into accordion-openItems
kkakroo Jul 25, 2023
c853b1c
Merge branch 'master' into accordion-openItems
kkakroo Jul 26, 2023
f5948b5
Merge branch 'master' into accordion-openItems
kkakroo Jul 27, 2023
7b0267b
Merge branch 'master' into accordion-openItems
kkakroo Aug 1, 2023
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
7 changes: 7 additions & 0 deletions change/@fluentui-react-accordion-2072fdcd-6e91-497d-8a09-567c6d4e60a1.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "fix: added openItems to AccordionToggleData which respects multiple and collapsible props for controlled Accordion",
"packageName": "@fluentui/react-accordion",
"email": "[email protected]",
"dependentChangeType": "patch"
}
1 change: 1 addition & 0 deletions packages/react-components/react-accordion/etc/react-accordion.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ export type AccordionState = ComponentState<AccordionSlots> & AccordionContextVa
// @public (undocumented)
export type AccordionToggleData = {
value: AccordionItemValue;
openItems: AccordionItemValue[];
};

// @public (undocumented)
Expand Down
1 change: 1 addition & 0 deletions packages/react-components/react-accordion/src/components/Accordion/Accordion.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export type AccordionSlots = {

export type AccordionToggleData = {
value: AccordionItemValue;
openItems: AccordionItemValue[];
};

export type AccordionProps = ComponentProps<AccordionSlots> & {
Expand Down
20 changes: 10 additions & 10 deletions packages/react-components/react-accordion/src/components/Accordion/useAccordion.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('useAccordion_unstable', () => {
expect(result.current.openItems.length).toEqual(1);
expect(result.current.openItems.includes(0)).toBeTruthy();

act(() => result.current.requestToggle(undefined!, { value: 1 }));
act(() => result.current.requestToggle(undefined!, { value: 1, openItems: [] }));

expect(result.current.openItems.length).toEqual(1);
expect(result.current.openItems.includes(1)).toBeTruthy();
Expand All @@ -20,9 +20,9 @@ describe('useAccordion_unstable', () => {
it('should only have zero open items before having any items', () => {
const { result } = renderHook(() => useAccordion_unstable({ multiple: true }, React.createRef()));
expect(result.current.openItems.length).toEqual(0);
act(() => result.current.requestToggle(undefined!, { value: 0 }));
act(() => result.current.requestToggle(undefined!, { value: 0, openItems: [] }));
expect(result.current.openItems.length).toEqual(1);
act(() => result.current.requestToggle(undefined!, { value: 0 }));
act(() => result.current.requestToggle(undefined!, { value: 0, openItems: [] }));
expect(result.current.openItems.length).toEqual(1);
});
it('should not have less than 1 open item', () => {
Expand All @@ -32,16 +32,16 @@ describe('useAccordion_unstable', () => {

expect(result.current.openItems.length).toEqual(1);

act(() => result.current.requestToggle(undefined!, { value: 0 }));
act(() => result.current.requestToggle(undefined!, { value: 0, openItems: [] }));
expect(result.current.openItems.length).toEqual(1);
expect(result.current.openItems.includes(0)).toBeTruthy();
});
it('should open multiple panels', () => {
const { result } = renderHook(() => useAccordion_unstable({ multiple: true }, React.createRef()));
expect(result.current.openItems.length).toEqual(0);
act(() => result.current.requestToggle(undefined!, { value: 0 }));
act(() => result.current.requestToggle(undefined!, { value: 0, openItems: [] }));
expect(result.current.openItems.includes(0)).toBeTruthy();
act(() => result.current.requestToggle(undefined!, { value: 1 }));
act(() => result.current.requestToggle(undefined!, { value: 1, openItems: [] }));
expect(result.current.openItems.includes(1)).toBeTruthy();
expect(result.current.openItems.length).toEqual(2);
});
Expand All @@ -50,18 +50,18 @@ describe('useAccordion_unstable', () => {
it('should have zero panels opened', () => {
const { result } = renderHook(() => useAccordion_unstable({ collapsible: true }, React.createRef()));
expect(result.current.openItems.length).toEqual(0);
act(() => result.current.requestToggle(undefined!, { value: 0 }));
act(() => result.current.requestToggle(undefined!, { value: 0, openItems: [] }));
expect(result.current.openItems.length).toEqual(1);
act(() => result.current.requestToggle(undefined!, { value: 0 }));
act(() => result.current.requestToggle(undefined!, { value: 0, openItems: [] }));
expect(result.current.openItems.length).toEqual(0);
});
it('should not open more than one panel', () => {
const { result } = renderHook(() => useAccordion_unstable({ collapsible: true }, React.createRef()));
expect(result.current.openItems.length).toEqual(0);
act(() => result.current.requestToggle(undefined!, { value: 0 }));
act(() => result.current.requestToggle(undefined!, { value: 0, openItems: [] }));
expect(result.current.openItems.length).toEqual(1);
expect(result.current.openItems.includes(0)).toBeTruthy();
act(() => result.current.requestToggle(undefined!, { value: 1 }));
act(() => result.current.requestToggle(undefined!, { value: 1, openItems: [] }));
expect(result.current.openItems.length).toEqual(1);
expect(result.current.openItems.includes(1)).toBeTruthy();
expect(result.current.openItems.includes(0)).toBeFalsy();
Expand Down
5 changes: 3 additions & 2 deletions packages/react-components/react-accordion/src/components/Accordion/useAccordion.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,9 @@ export const useAccordion_unstable = (props: AccordionProps, ref: React.Ref<HTML
});

const requestToggle = useEventCallback((event: AccordionToggleEvent, data: AccordionToggleData) => {
onToggle?.(event, data);
setOpenItems(previousOpenItems => updateOpenItems(data.value, previousOpenItems, multiple, collapsible));
const currentOpenItems = updateOpenItems(data.value, openItems, multiple, collapsible);
onToggle?.(event, { value: data.value, openItems: currentOpenItems });
setOpenItems(currentOpenItems);
});

return {
Expand Down
4 changes: 2 additions & 2 deletions packages/react-components/react-accordion/src/components/AccordionItem/useAccordionItem.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export const useAccordionItem_unstable = (
const requestToggle = useAccordionContext_unstable(ctx => ctx.requestToggle);
const open = useAccordionContext_unstable(ctx => ctx.openItems.includes(value));
const onAccordionHeaderClick = React.useCallback(
(ev: AccordionToggleEvent) => requestToggle(ev, { value }),
(ev: AccordionToggleEvent) => requestToggle(ev, { value, openItems: [] }),
Copy link
Contributor

@bsunderhus bsunderhus Jul 28, 2023

Choose a reason for hiding this comment

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

this looks like a hack. instead of this, how about to simply modify requestToggle signature?

[requestToggle, value],
);

Expand All @@ -30,7 +30,7 @@ export const useAccordionItem_unstable = (
root: 'div',
},
root: getNativeElementProps('div', {
ref: ref,
ref,
...props,
}),
};
Expand Down