Skip to content

Commit 64befbe

Browse files
thatblindgeyethatblindgeye
authored
fix(ExpandableSection): made animation opt-in for detached variant (#11851)
* fix(ExpandableSection): made animation opt-in for detached variant * Test for more opt-in functionality * Removed detached class per Core updates * Fixed typo * Updated hamburgericon class to styles object * Bumped core version, feedback from Coker
1 parent 627a53b commit 64befbe

File tree

14 files changed

+154
-104
lines changed

14 files changed

+154
-104
lines changed

‎packages/react-core/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@
5454
"tslib": "^2.8.1"
5555
},
5656
"devDependencies": {
57-
"@patternfly/patternfly": "6.3.0-prerelease.29",
57+
"@patternfly/patternfly": "6.3.0-prerelease.33",
5858
"case-anything": "^3.1.2",
5959
"css": "^3.0.0",
6060
"fs-extra": "^11.3.0"

‎packages/react-core/src/components/Button/hamburgerIcon.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { css } from '@patternfly/react-styles';
44
// Because this is such a specific icon that requires being wrapped in a pf-v[current version]-c-button element,
55
// we don't want to export this to consumers nor include it in the react-icons package as a custom icon.
66
export const hamburgerIcon = (
7-
<svg viewBox="0 0 10 10" className="pf-v6-c-button--hamburger-icon pf-v6-svg" width="1em" height="1em">
7+
<svg viewBox="0 0 10 10" className={css(styles.buttonHamburgerIcon, 'pf-v6-svg')} width="1em" height="1em">
88
<path className={css(styles.buttonHamburgerIconTop)} d="M1,1 L9,1"></path>
99
<path className={css(styles.buttonHamburgerIconMiddle)} d="M1,5 L9,5"></path>
1010
<path className={css(styles.buttonHamburgerIconArrow)} d="M1,5 L1,5 L1,5"></path>

‎packages/react-core/src/components/ExpandableSection/ExpandableSection.tsx‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export interface ExpandableSectionProps extends Omit<React.HTMLProps<HTMLDivElem
3232
toggleId?: string;
3333
/** Display size variant. Set to "lg" for disclosure styling. */
3434
displaySize?: 'default' | 'lg';
35-
/** Indicates the expandable section has a detached toggle. */
35+
/** Flag indicating that the expandable section and expandable toggle are detached from one another. */
3636
isDetached?: boolean;
3737
/** Flag to indicate if the content is expanded. */
3838
isExpanded?: boolean;
@@ -64,6 +64,10 @@ export interface ExpandableSectionProps extends Omit<React.HTMLProps<HTMLDivElem
6464
* variant, the expandable content will be truncated after 3 lines by default.
6565
*/
6666
variant?: 'default' | 'truncate';
67+
/** Sets the direction of the expandable animation when isDetached is true. If this prop is not passed,
68+
* animation will not occur.
69+
*/
70+
direction?: 'up' | 'down';
6771
}
6872

6973
interface ExpandableSectionState {
@@ -72,6 +76,11 @@ interface ExpandableSectionState {
7276
previousWidth: number;
7377
}
7478

79+
const directionClassMap = {
80+
up: styles.modifiers.expandTop,
81+
down: styles.modifiers.expandBottom
82+
};
83+
7584
const setLineClamp = (lines: number, element: HTMLDivElement) => {
7685
if (!element || lines < 1) {
7786
return;
@@ -198,6 +207,7 @@ class ExpandableSection extends Component<ExpandableSectionProps, ExpandableSect
198207
variant,
199208
// eslint-disable-next-line @typescript-eslint/no-unused-vars
200209
truncateMaxLines,
210+
direction,
201211
...props
202212
} = this.props;
203213

@@ -258,6 +268,8 @@ class ExpandableSection extends Component<ExpandableSectionProps, ExpandableSect
258268
displaySize === 'lg' && styles.modifiers.displayLg,
259269
isWidthLimited && styles.modifiers.limitWidth,
260270
isIndented && styles.modifiers.indented,
271+
isDetached && direction && directionClassMap[direction],
272+
isDetached && direction && 'pf-m-detached',
261273
variant === ExpandableSectionVariant.truncate && styles.modifiers.truncate,
262274
className
263275
)}

‎packages/react-core/src/components/ExpandableSection/ExpandableSectionToggle.tsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ export interface ExpandableSectionToggleProps extends Omit<React.HTMLProps<HTMLD
2828
isExpanded?: boolean;
2929
/** Callback function to toggle the expandable content. */
3030
onToggle?: (isExpanded: boolean) => void;
31+
/** Flag indicating that the expandable section and expandable toggle are detached from one another. */
32+
isDetached?: boolean;
3133
}
3234

3335
export const ExpandableSectionToggle: React.FunctionComponent<ExpandableSectionToggleProps> = ({
@@ -39,13 +41,15 @@ export const ExpandableSectionToggle: React.FunctionComponent<ExpandableSectionT
3941
toggleId,
4042
direction = 'down',
4143
hasTruncatedContent = false,
44+
isDetached,
4245
...props
4346
}: ExpandableSectionToggleProps) => (
4447
<div
4548
className={css(
4649
styles.expandableSection,
4750
isExpanded && styles.modifiers.expanded,
4851
hasTruncatedContent && styles.modifiers.truncate,
52+
isDetached && 'pf-m-detached',
4953
className
5054
)}
5155
{...props}
@@ -63,7 +67,7 @@ export const ExpandableSectionToggle: React.FunctionComponent<ExpandableSectionT
6367
<span
6468
className={css(
6569
styles.expandableSectionToggleIcon,
66-
isExpanded && direction === 'up' && styles.modifiers.expandTop
70+
isExpanded && direction === 'up' && styles.modifiers.expandTop // TODO: next breaking change move this class to the outer styles.expandableSection wrapper
6771
)}
6872
>
6973
<AngleRightIcon />

‎packages/react-core/src/components/ExpandableSection/__tests__/ExpandableSection.test.tsx‎

Lines changed: 85 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
import { Fragment } from 'react';
21
import { render, screen } from '@testing-library/react';
32
import userEvent from '@testing-library/user-event';
43

54
import { ExpandableSection, ExpandableSectionVariant } from '../ExpandableSection';
6-
import { ExpandableSectionToggle } from '../ExpandableSectionToggle';
5+
import styles from '@patternfly/react-styles/css/components/ExpandableSection/expandable-section';
76

87
const props = { contentId: 'content-id', toggleId: 'toggle-id' };
98

@@ -22,7 +21,7 @@ test('Renders ExpandableSection expanded', () => {
2221
expect(asFragment()).toMatchSnapshot();
2322
});
2423

25-
test('ExpandableSection onToggle called', async () => {
24+
test('Calls onToggle when clicked', async () => {
2625
const mockfn = jest.fn();
2726
const user = userEvent.setup();
2827

@@ -32,6 +31,21 @@ test('ExpandableSection onToggle called', async () => {
3231
expect(mockfn.mock.calls).toHaveLength(1);
3332
});
3433

34+
test('Does not call onToggle when not clicked', async () => {
35+
const mockfn = jest.fn();
36+
const user = userEvent.setup();
37+
38+
render(
39+
<>
40+
<ExpandableSection onToggle={mockfn}> test </ExpandableSection>
41+
<button onClick={() => {}}>Test clicker</button>
42+
</>
43+
);
44+
45+
await user.click(screen.getByRole('button', { name: 'Test clicker' }));
46+
expect(mockfn).not.toHaveBeenCalled();
47+
});
48+
3549
test('Renders Uncontrolled ExpandableSection', () => {
3650
const { asFragment } = render(
3751
<ExpandableSection {...props} toggleText="Show More">
@@ -42,20 +56,6 @@ test('Renders Uncontrolled ExpandableSection', () => {
4256
expect(asFragment()).toMatchSnapshot();
4357
});
4458

45-
test('Detached ExpandableSection renders successfully', () => {
46-
const { asFragment } = render(
47-
<Fragment>
48-
<ExpandableSection isExpanded isDetached {...props}>
49-
test
50-
</ExpandableSection>
51-
<ExpandableSectionToggle isExpanded direction="up" {...props}>
52-
Toggle text
53-
</ExpandableSectionToggle>
54-
</Fragment>
55-
);
56-
expect(asFragment()).toMatchSnapshot();
57-
});
58-
5959
test('Disclosure ExpandableSection', () => {
6060
const { asFragment } = render(
6161
<ExpandableSection {...props} displaySize="lg" isWidthLimited>
@@ -75,22 +75,22 @@ test('Renders ExpandableSection indented', () => {
7575
expect(asFragment()).toMatchSnapshot();
7676
});
7777

78-
test('Does not render with pf-m-truncate class when variant is not passed', () => {
78+
test(`Does not render with ${styles.modifiers.truncate} class when variant is not passed`, () => {
7979
render(<ExpandableSection>test</ExpandableSection>);
8080

81-
expect(screen.getByText('test').parentElement).not.toHaveClass('pf-m-truncate');
81+
expect(screen.getByText('test').parentElement).not.toHaveClass(styles.modifiers.truncate);
8282
});
8383

84-
test('Does not render with pf-m-truncate class when variant is not truncate', () => {
84+
test(`Does not render with ${styles.modifiers.truncate} class when variant is not truncate`, () => {
8585
render(<ExpandableSection variant={ExpandableSectionVariant.default}>test</ExpandableSection>);
8686

87-
expect(screen.getByText('test').parentElement).not.toHaveClass('pf-m-truncate');
87+
expect(screen.getByText('test').parentElement).not.toHaveClass(styles.modifiers.truncate);
8888
});
8989

90-
test('Renders with pf-m-truncate class when variant is truncate', () => {
90+
test(`Renders with ${styles.modifiers.truncate} class when variant is truncate`, () => {
9191
render(<ExpandableSection variant={ExpandableSectionVariant.truncate}>test</ExpandableSection>);
9292

93-
expect(screen.getByText('test').parentElement).toHaveClass('pf-m-truncate');
93+
expect(screen.getByText('test').parentElement).toHaveClass(styles.modifiers.truncate);
9494
});
9595

9696
test('Renders with value passed to contentId', () => {
@@ -129,3 +129,65 @@ test('Renders with ARIA attributes when contentId and toggleId are passed', () =
129129
expect(wrapper).toContainHTML('aria-labelledby="toggle-id"');
130130
expect(wrapper).toContainHTML('aria-controls="content-id"');
131131
});
132+
133+
test(`Does not render with classes ${styles.modifiers.expandTop} nor ${styles.modifiers.expandBottom} by default`, () => {
134+
render(<ExpandableSection>Test content</ExpandableSection>);
135+
136+
expect(screen.getByText('Test content').parentElement).not.toHaveClass('pf-m-expand-top');
137+
expect(screen.getByText('Test content').parentElement).not.toHaveClass('pf-m-expand-bottom');
138+
});
139+
140+
test(`Does not render with classes ${styles.modifiers.expandTop} nor ${styles.modifiers.expandBottom} when only isDetached is true`, () => {
141+
render(<ExpandableSection isDetached>Test content</ExpandableSection>);
142+
143+
expect(screen.getByText('Test content').parentElement).not.toHaveClass('pf-m-expand-top');
144+
expect(screen.getByText('Test content').parentElement).not.toHaveClass('pf-m-expand-bottom');
145+
});
146+
147+
test(`Does not render with class ${styles.modifiers.expandTop} when direction="up" and isDetached is false`, () => {
148+
render(<ExpandableSection direction="up">Test content</ExpandableSection>);
149+
150+
expect(screen.getByText('Test content').parentElement).not.toHaveClass('pf-m-expand-top');
151+
});
152+
153+
test(`Does not render with class ${styles.modifiers.expandBottom} when direction="down" and isDetached is false`, () => {
154+
render(<ExpandableSection direction="down">Test content</ExpandableSection>);
155+
156+
expect(screen.getByText('Test content').parentElement).not.toHaveClass('pf-m-expand-bottom');
157+
});
158+
159+
test(`Renders with class ${styles.modifiers.expandTop} when isDetached is true and direction="up"`, () => {
160+
render(
161+
<ExpandableSection isDetached direction="up">
162+
Test content
163+
</ExpandableSection>
164+
);
165+
166+
expect(screen.getByText('Test content').parentElement).toHaveClass('pf-m-expand-top');
167+
});
168+
169+
test(`Renders with class ${styles.modifiers.expandBottom} when isDetached is true and direction="down"`, () => {
170+
render(
171+
<ExpandableSection isDetached direction="down">
172+
Test content
173+
</ExpandableSection>
174+
);
175+
176+
expect(screen.getByText('Test content').parentElement).toHaveClass('pf-m-expand-bottom');
177+
});
178+
179+
test('Does not render with class pf-m-detached when isDetached is true and direction is not passed', () => {
180+
render(<ExpandableSection isDetached>Test content</ExpandableSection>);
181+
182+
expect(screen.getByText('Test content').parentElement).not.toHaveClass('pf-m-detached');
183+
});
184+
185+
test('Renders with class pf-m-detached when isDetached is true and direction is passed', () => {
186+
render(
187+
<ExpandableSection isDetached direction="up">
188+
Test content
189+
</ExpandableSection>
190+
);
191+
192+
expect(screen.getByText('Test content').parentElement).toHaveClass('pf-m-detached');
193+
});

‎packages/react-core/src/components/ExpandableSection/__tests__/ExpandableSectionToggle.test.tsx‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import { render, screen } from '@testing-library/react';
2+
import userEvent from '@testing-library/user-event';
3+
import { ExpandableSectionToggle } from '../ExpandableSectionToggle';
4+
import styles from '@patternfly/react-styles/css/components/ExpandableSection/expandable-section';
5+
6+
test('Renders without children', () => {
7+
render(<ExpandableSectionToggle></ExpandableSectionToggle>);
8+
9+
expect(screen.getByRole('button')).toBeInTheDocument();
10+
});
11+
12+
test('Renders with children', () => {
13+
render(<ExpandableSectionToggle>Toggle test</ExpandableSectionToggle>);
14+
15+
expect(screen.getByRole('button')).toHaveTextContent('Toggle test');
16+
});
17+
18+
test('Does not render with class pf-m-detached by default', () => {
19+
render(<ExpandableSectionToggle data-testid="test-id">Toggle test</ExpandableSectionToggle>);
20+
21+
expect(screen.getByTestId('test-id')).not.toHaveClass('pf-m-detached');
22+
});
23+
24+
test('Renders with class pf-m-detached when isDetached is true', () => {
25+
render(
26+
<ExpandableSectionToggle data-testid="test-id" isDetached>
27+
Toggle test
28+
</ExpandableSectionToggle>
29+
);
30+
31+
expect(screen.getByTestId('test-id')).toHaveClass('pf-m-detached');
32+
});

‎packages/react-core/src/components/ExpandableSection/__tests__/__snapshots__/ExpandableSection.test.tsx.snap‎

Lines changed: 1 addition & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -1,67 +1,5 @@
11
// Jest Snapshot v1, https://goo.gl/fbAQLP
22

3-
exports[`Detached ExpandableSection renders successfully 1`] = `
4-
<DocumentFragment>
5-
<div
6-
class="pf-v6-c-expandable-section pf-m-expanded"
7-
>
8-
<div
9-
aria-labelledby="toggle-id"
10-
class="pf-v6-c-expandable-section__content"
11-
id="content-id"
12-
role="region"
13-
>
14-
test
15-
</div>
16-
</div>
17-
<div
18-
class="pf-v6-c-expandable-section pf-m-expanded"
19-
>
20-
<div
21-
class="pf-v6-c-expandable-section__toggle"
22-
>
23-
<button
24-
aria-controls="content-id"
25-
aria-expanded="true"
26-
class="pf-v6-c-button pf-m-link"
27-
data-ouia-component-id="OUIA-Generated-Button-link-5"
28-
data-ouia-component-type="PF6/Button"
29-
data-ouia-safe="true"
30-
id="toggle-id"
31-
type="button"
32-
>
33-
<span
34-
class="pf-v6-c-button__icon pf-m-start"
35-
>
36-
<span
37-
class="pf-v6-c-expandable-section__toggle-icon pf-m-expand-top"
38-
>
39-
<svg
40-
aria-hidden="true"
41-
class="pf-v6-svg"
42-
fill="currentColor"
43-
height="1em"
44-
role="img"
45-
viewBox="0 0 256 512"
46-
width="1em"
47-
>
48-
<path
49-
d="M224.3 273l-136 136c-9.4 9.4-24.6 9.4-33.9 0l-22.6-22.6c-9.4-9.4-9.4-24.6 0-33.9l96.4-96.4-96.4-96.4c-9.4-9.4-9.4-24.6 0-33.9L54.3 103c9.4-9.4 24.6-9.4 33.9 0l136 136c9.5 9.4 9.5 24.6.1 34z"
50-
/>
51-
</svg>
52-
</span>
53-
</span>
54-
<span
55-
class="pf-v6-c-button__text"
56-
>
57-
Toggle text
58-
</span>
59-
</button>
60-
</div>
61-
</div>
62-
</DocumentFragment>
63-
`;
64-
653
exports[`Disclosure ExpandableSection 1`] = `
664
<DocumentFragment>
675
<div
@@ -285,7 +223,7 @@ exports[`Renders Uncontrolled ExpandableSection 1`] = `
285223
<button
286224
aria-controls="content-id"
287225
class="pf-v6-c-button pf-m-link"
288-
data-ouia-component-id="OUIA-Generated-Button-link-4"
226+
data-ouia-component-id="OUIA-Generated-Button-link-5"
289227
data-ouia-component-type="PF6/Button"
290228
data-ouia-safe="true"
291229
id="toggle-id"

‎packages/react-core/src/components/ExpandableSection/examples/ExpandableSection.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ import CheckCircleIcon from '@patternfly/react-icons/dist/esm/icons/check-circle
3232

3333
When passing the `isDetached` property into `<ExpandableSection>`, you must also manually pass in the same `toggleId` and `contentId` properties to both `<ExpandableSection>` and `<ExpandableSectionToggle>`. This will link the content to the toggle via ARIA attributes.
3434

35+
By default animations will not be enabled for a detached `<ExpandableSection>`. You must manually pass the `direction` property with an appropriate value based on where the expandable content is rendered. If the expandable content is above the expandable toggle, `direction="up"` must be passed like in this example.
36+
3537
```ts file="ExpandableSectionDetached.tsx"
3638

3739
```

‎packages/react-core/src/components/ExpandableSection/examples/ExpandableSectionDetached.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export const ExpandableSectionDetached: React.FunctionComponent = () => {
1313
return (
1414
<Stack hasGutter>
1515
<StackItem>
16-
<ExpandableSection isExpanded={isExpanded} isDetached toggleId={toggleId} contentId={contentId}>
16+
<ExpandableSection isExpanded={isExpanded} isDetached direction="up" toggleId={toggleId} contentId={contentId}>
1717
This content is visible only when the component is expanded.
1818
</ExpandableSection>
1919
</StackItem>

‎packages/react-docs/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
"test:a11y": "patternfly-a11y --config patternfly-a11y.config"
2424
},
2525
"dependencies": {
26-
"@patternfly/patternfly": "6.3.0-prerelease.29",
26+
"@patternfly/patternfly": "6.3.0-prerelease.33",
2727
"@patternfly/react-charts": "workspace:^",
2828
"@patternfly/react-code-editor": "workspace:^",
2929
"@patternfly/react-core": "workspace:^",

0 commit comments

Comments
 (0)