Skip to content

Commit 2cc59bb

Browse files
Mcourtgcornut
authored andcommitted
fix(expansion-panel): fix children remaining in DOM when isOpen changes
DSW-294
1 parent b1c31e4 commit 2cc59bb

File tree

5 files changed

+62
-33
lines changed

5 files changed

+62
-33
lines changed

Diff for: CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
### Fixed
1111

1212
- `Chip`: trigger onClick when `Enter` key is pressed
13+
- `ExpansionPanel`: fix children remaining in the DOM when `isOpen` prop changes
1314

1415
## [3.9.4][] - 2024-11-04
1516

Diff for: packages/lumx-core/src/js/constants/index.ts

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export * from './keycodes';
1313
* you need to update their scss counterpart as well
1414
*/
1515
export const DIALOG_TRANSITION_DURATION = 400;
16+
export const EXPANSION_PANEL_TRANSITION_DURATION = 400;
1617
export const NOTIFICATION_TRANSITION_DURATION = 200;
1718
export const SLIDESHOW_TRANSITION_DURATION = 5000;
1819

Diff for: packages/lumx-react/src/components/expansion-panel/ExpansionPanel.test.tsx

+32-2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { getByClassName, queryByClassName } from '@lumx/react/testing/utils/quer
66
import userEvent from '@testing-library/user-event';
77
import { isFocusVisible } from '@lumx/react/utils/isFocusVisible';
88

9+
import { useBooleanState } from '@lumx/react/hooks/useBooleanState';
910
import { ExpansionPanel, ExpansionPanelProps } from '.';
1011

1112
const CLASSNAME = ExpansionPanel.className as string;
@@ -14,16 +15,29 @@ jest.mock('@lumx/react/utils/isFocusVisible');
1415

1516
const mockChildrenContent = 'children content';
1617

18+
/** Controlled component that uses a local state to toggle the component */
19+
const ControlledComponent = (props: ExpansionPanelProps) => {
20+
const [isOpen, handleClose, handleOpen] = useBooleanState(false);
21+
22+
return (
23+
<ExpansionPanel isOpen={isOpen} onClose={handleClose} onOpen={handleOpen} {...props}>
24+
{mockChildrenContent}
25+
</ExpansionPanel>
26+
);
27+
};
28+
1729
/**
1830
* Mounts the component and returns common DOM elements / data needed in multiple tests further down.
1931
*/
20-
const setup = (propsOverride: Partial<ExpansionPanelProps> = {}) => {
32+
const setup = (propsOverride: Partial<ExpansionPanelProps> = {}, options: { controlled?: boolean } = {}) => {
2133
const props = {
2234
toggleButtonProps: { label: 'Toggle' },
2335
children: mockChildrenContent,
2436
...propsOverride,
2537
};
26-
const { container } = render(<ExpansionPanel {...props} />);
38+
39+
const Component = options.controlled ? ControlledComponent : ExpansionPanel;
40+
const { container } = render(<Component {...props} />);
2741

2842
return {
2943
container,
@@ -116,6 +130,22 @@ describe(`<${ExpansionPanel.displayName}>`, () => {
116130
expect(onClose).toHaveBeenCalled();
117131
expect(onToggleOpen).toHaveBeenCalledWith(false, expect.anything());
118132
});
133+
134+
it('should hide children after toggling the expansion panel', async () => {
135+
const user = userEvent.setup();
136+
const { query } = setup({}, { controlled: true });
137+
138+
// Content is not visible by default
139+
expect(query.content()).not.toBeInTheDocument();
140+
141+
await user.click(query.header() as any);
142+
143+
expect(query.content()).toBeInTheDocument();
144+
145+
await user.click(query.header() as any);
146+
147+
expect(query.content()).not.toBeInTheDocument();
148+
});
119149
});
120150

121151
// Common tests suite.

Diff for: packages/lumx-react/src/components/expansion-panel/ExpansionPanel.tsx

+6-30
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ import { ColorPalette, DragHandle, Emphasis, IconButton, IconButtonProps, Theme
1212
import { Comp, GenericProps, HasTheme, isComponent } from '@lumx/react/utils/type';
1313
import { getRootClassName, handleBasicClasses } from '@lumx/react/utils/className';
1414
import { partitionMulti } from '@lumx/react/utils/partitionMulti';
15-
import { WINDOW } from '@lumx/react/constants';
15+
import { useTransitionVisibility } from '@lumx/react/hooks/useTransitionVisibility';
16+
import { EXPANSION_PANEL_TRANSITION_DURATION } from '@lumx/core/js/constants';
1617

1718
/**
1819
* Defines the props of the component.
@@ -81,7 +82,6 @@ export const ExpansionPanel: Comp<ExpansionPanelProps, HTMLDivElement> = forward
8182
...forwardedProps
8283
} = props;
8384

84-
const [isChildrenVisible, setIsChildrenVisible] = useState(isOpen);
8585
const children: ReactNode[] = Children.toArray(anyChildren);
8686

8787
// Partition children by types.
@@ -96,34 +96,16 @@ export const ExpansionPanel: Comp<ExpansionPanelProps, HTMLDivElement> = forward
9696
);
9797

9898
const toggleOpen = (event: React.MouseEvent) => {
99-
const hasReducedMotionEnabled = WINDOW?.matchMedia?.('(prefers-reduced-motion: reduce)')?.matches;
10099
const shouldOpen = !isOpen;
101100

102101
if (isFunction(onOpen) && shouldOpen) {
103102
onOpen(event);
104-
// On open, we immediately show children
105-
setIsChildrenVisible(true);
106103
}
107104
if (isFunction(onClose) && !shouldOpen) {
108105
onClose(event);
109-
/**
110-
* On close, we only show children immediately if reduced motion is enabled
111-
* When disabled, the children will be hidden via the `onTransitionEnd` prop.
112-
*/
113-
if (hasReducedMotionEnabled) {
114-
setIsChildrenVisible(false);
115-
}
116106
}
117107
if (isFunction(onToggleOpen)) {
118108
onToggleOpen(shouldOpen, event);
119-
/**
120-
* On toggle, we forward the show state if
121-
* * the toggle will open the expansion panel
122-
* * reduced motion is enabled. When disabled, the children will be hidden via the `onTransitionEnd` prop.
123-
* */
124-
if (shouldOpen || hasReducedMotionEnabled) {
125-
setIsChildrenVisible(shouldOpen);
126-
}
127109
}
128110
};
129111

@@ -145,6 +127,9 @@ export const ExpansionPanel: Comp<ExpansionPanelProps, HTMLDivElement> = forward
145127

146128
const wrapperRef = useRef<HTMLDivElement>(null);
147129

130+
/** Hide the children at the end of the transition */
131+
const isChildrenVisible = useTransitionVisibility(wrapperRef, !!isOpen, EXPANSION_PANEL_TRANSITION_DURATION);
132+
148133
// Switch max height on/off to activate the CSS transition (updates when children changes).
149134
const [maxHeight, setMaxHeight] = useState('0');
150135
useEffect(() => {
@@ -174,16 +159,7 @@ export const ExpansionPanel: Comp<ExpansionPanelProps, HTMLDivElement> = forward
174159
</header>
175160

176161
{(isOpen || isChildrenVisible) && (
177-
<div
178-
className={`${CLASSNAME}__wrapper`}
179-
style={{ maxHeight }}
180-
// At the end of the closing transition, remove the children from the DOM
181-
onTransitionEnd={() => {
182-
if (!isOpen) {
183-
setIsChildrenVisible(false);
184-
}
185-
}}
186-
>
162+
<div className={`${CLASSNAME}__wrapper`} style={{ maxHeight }}>
187163
<div className={`${CLASSNAME}__container`} ref={wrapperRef}>
188164
<div className={`${CLASSNAME}__content`}>{content}</div>
189165

Diff for: packages/site-demo/content/product/components/expansion-panel/react/examples.tsx

+22-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,16 @@
11
import { mdiDotsVertical } from '@lumx/icons';
22

3-
import { Divider, ExpansionPanel, FlexBox, GenericBlock, Heading, IconButton, Text, Thumbnail } from '@lumx/react';
3+
import {
4+
Button,
5+
Divider,
6+
ExpansionPanel,
7+
FlexBox,
8+
GenericBlock,
9+
Heading,
10+
IconButton,
11+
Text,
12+
Thumbnail,
13+
} from '@lumx/react';
414
import React, { useState } from 'react';
515

616
export const App = () => {
@@ -11,8 +21,19 @@ export const App = () => {
1121
const [isOpen5, setOpen5] = useState(false);
1222
const stopPropagation = (evt: Event) => evt.stopPropagation();
1323

24+
const handleCloseAll = () => {
25+
setOpen1(false);
26+
setOpen2(false);
27+
setOpen3(false);
28+
setOpen4(false);
29+
setOpen5(false);
30+
};
31+
1432
return (
1533
<>
34+
<Button type="button" onClick={handleCloseAll}>
35+
Close all
36+
</Button>
1637
<ExpansionPanel
1738
hasBackground
1839
hasHeaderDivider

0 commit comments

Comments
 (0)