Skip to content

Commit dab7e8a

Browse files
kmcfaulkmcfaul
authored
feat(Page,Compass,Nav,MenuToggle,Button): simplify docked nav expand props (#12661)
* feat(Page,Compass,Nav,MenuToggle,Button): simplify docked nav expand props * wording * update tests for props * coderabbit feedback: update toggle to clear dockoverlay, update select to only void inline desktop, remove unused prop * fix inline dock nav closing on outside click, fix overlay being toggled on secondary nav group toggle * fix mobile docked nav click outside behavior, update nav docs wording * update prop desc for isExpanded * remove dupe test * remove dupe bullet point * update snap for ouia
1 parent 7ad66d2 commit dab7e8a

22 files changed

Lines changed: 122 additions & 247 deletions

File tree

‎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.6.0-prerelease.49",
57+
"@patternfly/patternfly": "6.6.0-prerelease.50",
5858
"case-anything": "^3.1.2",
5959
"css": "^3.0.0",
6060
"fs-extra": "^11.3.3"

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ export interface ButtonProps extends Omit<React.HTMLProps<HTMLButtonElement>, 'r
9999
tabIndex?: number;
100100
/** Adds danger styling to secondary or link button variants */
101101
isDanger?: boolean;
102-
/** Flag indicating whether content the button controls is expanded or not. Required when isHamburger is true. */
102+
/** Flag indicating whether content the button controls is expanded or not. Required when isHamburger is true. Applies additional expanded styling when isDocked is true. */
103103
isExpanded?: boolean;
104104
/** Flag indicating the button is a settings button. This will override the icon property. */
105105
isSettings?: boolean;
@@ -111,8 +111,6 @@ export interface ButtonProps extends Omit<React.HTMLProps<HTMLButtonElement>, 'r
111111
isCircle?: boolean;
112112
/** @beta Flag indicating the button is a docked variant button. For use in docked navigation. */
113113
isDocked?: boolean;
114-
/** @beta Flag indicating the docked button should display text. Only applies when isDocked is true. */
115-
isTextExpanded?: boolean;
116114
/** @hide Forwarded ref */
117115
innerRef?: React.Ref<any>;
118116
/** Adds count number to button */
@@ -139,7 +137,6 @@ const ButtonBase: React.FunctionComponent<ButtonProps> = ({
139137
hamburgerVariant,
140138
isCircle,
141139
isDocked = false,
142-
isTextExpanded = false,
143140
spinnerAriaValueText,
144141
spinnerAriaLabelledBy,
145142
spinnerAriaLabel,
@@ -272,7 +269,7 @@ const ButtonBase: React.FunctionComponent<ButtonProps> = ({
272269
size === ButtonSize.lg && styles.modifiers.displayLg,
273270
isCircle && styles.modifiers.circle,
274271
isDocked && styles.modifiers.docked,
275-
isDocked && isTextExpanded && styles.modifiers.textExpanded,
272+
isDocked && isExpanded && styles.modifiers.expanded,
276273
className
277274
)}
278275
disabled={isButtonElement ? isDisabled : null}

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

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -566,34 +566,23 @@ describe('Dock variant', () => {
566566
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.docked);
567567
});
568568

569-
test(`Renders with class ${styles.modifiers.textExpanded} when isTextExpanded = true and isDocked = true`, () => {
569+
test(`Renders with class ${styles.modifiers.expanded} when isExpanded = true and isDocked = true`, () => {
570570
render(
571-
<Button isTextExpanded isDocked>
571+
<Button isExpanded isDocked>
572572
Text Expanded Button
573573
</Button>
574574
);
575-
expect(screen.getByRole('button')).toHaveClass(styles.modifiers.textExpanded);
575+
expect(screen.getByRole('button')).toHaveClass(styles.modifiers.expanded);
576576
});
577577

578-
test(`Does not render with class ${styles.modifiers.textExpanded} when isTextExpanded is not passed`, () => {
578+
test(`Does not render with class ${styles.modifiers.expanded} when isExpanded is not passed`, () => {
579579
render(<Button>Button</Button>);
580-
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.textExpanded);
580+
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.expanded);
581581
});
582582

583-
test(`Does not render with class ${styles.modifiers.textExpanded} when isTextExpanded = true but isDocked is not passed`, () => {
584-
render(<Button isTextExpanded>Text Expanded Button</Button>);
585-
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.textExpanded);
586-
});
587-
588-
test(`Renders with both ${styles.modifiers.docked} and ${styles.modifiers.textExpanded} when both props are true`, () => {
589-
render(
590-
<Button isDocked isTextExpanded>
591-
Dock Text Expanded Button
592-
</Button>
593-
);
594-
const button = screen.getByRole('button');
595-
expect(button).toHaveClass(styles.modifiers.docked);
596-
expect(button).toHaveClass(styles.modifiers.textExpanded);
583+
test(`Does not render with class ${styles.modifiers.expanded} when isExpanded = true but isDocked is not passed`, () => {
584+
render(<Button isExpanded>Text Expanded Button</Button>);
585+
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.expanded);
597586
});
598587
});
599588

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ exports[`Renders basic button 1`] = `
55
<button
66
aria-label="basic button"
77
class="pf-v6-c-button pf-m-primary"
8-
data-ouia-component-id="OUIA-Generated-Button-primary-:r36:"
8+
data-ouia-component-id="OUIA-Generated-Button-primary-:r35:"
99
data-ouia-component-type="PF6/Button"
1010
data-ouia-safe="true"
1111
type="button"

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

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,10 @@ export interface CompassProps extends React.HTMLProps<HTMLDivElement> {
1010
masthead?: React.ReactNode;
1111
/** Content of the docked navigation area of the layout */
1212
dock?: React.ReactNode;
13-
/** @beta Flag indicating the docked nav is expanded on mobile. Only applies when dock content is passed. */
13+
/** @beta Flag indicating the docked nav is expanded. Only applies when dock content is passed. */
1414
isDockExpanded?: boolean;
15-
/** @beta Flag indicating a docked nav is expanded as an overlay, triggered by expandable nav children. Only applies when dock content is passed. */
16-
isDockExpandableExpanded?: boolean;
17-
/** @beta Flag indicating the docked nav should display text on desktop. Only applies when dock content is passed, and
18-
* will handle toggling the visibility of the text in individual isDocked components.
19-
*/
20-
isDockTextExpanded?: boolean;
15+
/** @beta Flag indicating the docked nav should expand as an overlay instead of the default inline. */
16+
isDockOverlay?: boolean;
2117
/** Content placed at the top of the compass layout */
2218
header?: React.ReactNode;
2319
/** Flag indicating if the header is expanded */
@@ -47,8 +43,7 @@ export const Compass: React.FunctionComponent<CompassProps> = ({
4743
masthead,
4844
dock,
4945
isDockExpanded,
50-
isDockExpandableExpanded,
51-
isDockTextExpanded,
46+
isDockOverlay,
5247
header,
5348
isHeaderExpanded = true,
5449
sidebarStart,
@@ -72,8 +67,7 @@ export const Compass: React.FunctionComponent<CompassProps> = ({
7267
className={css(
7368
`${styles.compass}__dock`,
7469
isDockExpanded && styles.modifiers.expanded,
75-
isDockExpandableExpanded && styles.modifiers.expandableExpanded,
76-
isDockTextExpanded && styles.modifiers.textExpanded
70+
isDockOverlay && styles.modifiers.overlay
7771
)}
7872
>
7973
{dock}

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

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -94,19 +94,9 @@ test('Renders footer without expanded class and with inert when isFooterExpanded
9494
expect(footerElement).toHaveAttribute('inert');
9595
});
9696

97-
test(`Renders with ${styles.modifiers.expandableExpanded} class when isDockExpandableExpanded is true`, () => {
98-
render(<Compass dock={<div>Dock content</div>} isDockExpandableExpanded />);
99-
expect(screen.getByText('Dock content').parentElement).toHaveClass(styles.modifiers.expandableExpanded);
100-
});
101-
102-
test(`Does not render with ${styles.modifiers.expandableExpanded} class when isDockExpandableExpanded is false`, () => {
103-
render(<Compass dock={<div>Dock content</div>} isDockExpandableExpanded={false} />);
104-
expect(screen.getByText('Dock content').parentElement).not.toHaveClass(styles.modifiers.expandableExpanded);
105-
});
106-
107-
test(`Does not render with ${styles.modifiers.expandableExpanded} class by default`, () => {
108-
render(<Compass dock={<div>Dock content</div>} />);
109-
expect(screen.getByText('Dock content').parentElement).not.toHaveClass(styles.modifiers.expandableExpanded);
97+
test(`Renders with ${styles.modifiers.overlay} class when isDockOverlay is true`, () => {
98+
render(<Compass dock={<div>Dock content</div>} isDockOverlay />);
99+
expect(screen.getByText('Dock content').parentElement).toHaveClass(styles.modifiers.overlay);
110100
});
111101

112102
test('Renders with drawer when drawerContent is provided', () => {
@@ -194,13 +184,3 @@ test(`Renders dock without ${styles.modifiers.expanded} class when isDockExpande
194184
render(<Compass dock="Dock content" isDockExpanded={false} />);
195185
expect(screen.getByText('Dock content')).not.toHaveClass(styles.modifiers.expanded);
196186
});
197-
198-
test(`Renders dock with ${styles.modifiers.textExpanded} class when isDockTextExpanded is true`, () => {
199-
render(<Compass dock="Dock content" isDockTextExpanded />);
200-
expect(screen.getByText('Dock content')).toHaveClass(styles.modifiers.textExpanded);
201-
});
202-
203-
test(`Renders dock without ${styles.modifiers.textExpanded} class when isDockTextExpanded is false`, () => {
204-
render(<Compass dock="Dock content" isDockTextExpanded={false} />);
205-
expect(screen.getByText('Dock content')).not.toHaveClass(styles.modifiers.textExpanded);
206-
});

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,6 @@ export interface MenuToggleProps
5555
isSettings?: boolean;
5656
/** @beta Flag indicating the menu toggle is a docked variant. For use in docked navigation. */
5757
isDocked?: boolean;
58-
/** @beta Flag indicating the docked toggle should display text. Only applies when isDocked is true. */
59-
isTextExpanded?: boolean;
6058
/** Elements to display before the toggle button. When included, renders the menu toggle as a split button. */
6159
splitButtonItems?: React.ReactNode[];
6260
/** Variant styles of the menu toggle */
@@ -95,7 +93,6 @@ class MenuToggleBase extends Component<MenuToggleProps> {
9593
isPlaceholder: false,
9694
isCircle: false,
9795
isDocked: false,
98-
isTextExpanded: false,
9996
size: 'default',
10097
ouiaSafe: true,
10198
'aria-haspopup': 'menu'
@@ -116,7 +113,6 @@ class MenuToggleBase extends Component<MenuToggleProps> {
116113
isCircle,
117114
isSettings,
118115
isDocked,
119-
isTextExpanded,
120116
splitButtonItems,
121117
variant,
122118
status,
@@ -204,7 +200,6 @@ class MenuToggleBase extends Component<MenuToggleProps> {
204200
isPlaceholder && styles.modifiers.placeholder,
205201
isSettings && styles.modifiers.settings,
206202
isDocked && styles.modifiers.docked,
207-
isDocked && isTextExpanded && styles.modifiers.textExpanded,
208203
size === MenuToggleSize.sm && styles.modifiers.small,
209204
className
210205
);

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

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -166,32 +166,21 @@ test(`Does not render with class ${styles.modifiers.docked} when isDocked is not
166166
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.docked);
167167
});
168168

169-
test(`Renders with class ${styles.modifiers.textExpanded} when isTextExpanded is passed and isDocked is passed`, () => {
169+
test(`Renders with class ${styles.modifiers.expanded} when isExpanded is passed and isDocked is passed`, () => {
170170
render(
171-
<MenuToggle isTextExpanded isDocked>
171+
<MenuToggle isExpanded isDocked>
172172
Text Expanded Toggle
173173
</MenuToggle>
174174
);
175-
expect(screen.getByRole('button')).toHaveClass(styles.modifiers.textExpanded);
175+
expect(screen.getByRole('button')).toHaveClass(styles.modifiers.expanded);
176176
});
177177

178-
test(`Does not render with class ${styles.modifiers.textExpanded} when isTextExpanded is not passed`, () => {
178+
test(`Does not render with class ${styles.modifiers.expanded} when isExpanded is not passed`, () => {
179179
render(<MenuToggle>Toggle</MenuToggle>);
180-
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.textExpanded);
180+
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.expanded);
181181
});
182182

183-
test(`Does not render with class ${styles.modifiers.textExpanded} when isTextExpanded is passed but isDocked is not passed`, () => {
184-
render(<MenuToggle isTextExpanded>Text Expanded Toggle</MenuToggle>);
185-
expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers.textExpanded);
186-
});
187-
188-
test(`Renders with both ${styles.modifiers.docked} and ${styles.modifiers.textExpanded} when both props are passed`, () => {
189-
render(
190-
<MenuToggle isDocked isTextExpanded>
191-
Dock Text Expanded Toggle
192-
</MenuToggle>
193-
);
194-
const button = screen.getByRole('button');
195-
expect(button).toHaveClass(styles.modifiers.docked);
196-
expect(button).toHaveClass(styles.modifiers.textExpanded);
183+
test(`Does not render with class ${styles.modifiers.expanded} when isExpanded is passed but isDocked is not passed`, () => {
184+
render(<MenuToggle isExpanded>Text Expanded Toggle</MenuToggle>);
185+
expect(screen.getByRole('button')).toHaveClass(styles.modifiers.expanded);
197186
});

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export interface NavProps
4040
/** The nav variant to use. Docked is in beta. */
4141
variant?: 'default' | 'horizontal' | 'horizontal-subnav' | 'docked';
4242
/** @beta Flag indicating the docked nav should display text. Only applies when variant is docked. */
43-
isTextExpanded?: boolean;
43+
isExpanded?: boolean;
4444
/** Value to overwrite the randomly generated data-ouia-component-id.*/
4545
ouiaId?: number | string;
4646
/** Set the value of data-ouia-safe. Only set to true when the component is in a static state, i.e. no animations are occurring. At all other times, this value must be false. */
@@ -121,7 +121,7 @@ class Nav extends Component<NavProps, { isScrollable: boolean; flyoutRef: React.
121121
ouiaId,
122122
ouiaSafe,
123123
variant,
124-
isTextExpanded = false,
124+
isExpanded = false,
125125
...props
126126
} = this.props;
127127
const isHorizontal = ['horizontal', 'horizontal-subnav'].includes(variant);
@@ -159,7 +159,7 @@ class Nav extends Component<NavProps, { isScrollable: boolean; flyoutRef: React.
159159
isHorizontal && styles.modifiers.horizontal,
160160
isDocked && styles.modifiers.docked,
161161
variant === 'horizontal-subnav' && styles.modifiers.subnav,
162-
isDocked && isTextExpanded && styles.modifiers.textExpanded,
162+
isDocked && isExpanded && styles.modifiers.expanded,
163163
this.state.isScrollable && styles.modifiers.scrollable,
164164
className
165165
)}

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -275,9 +275,9 @@ describe('Nav', () => {
275275
expect(screen.getByTestId('docked-nav')).toHaveClass(styles.modifiers.docked);
276276
});
277277

278-
test(`Renders with ${styles.modifiers.textExpanded} class when isTextExpanded is true and variant is docked`, () => {
278+
test(`Renders with ${styles.modifiers.expanded} class when isExpanded is true and variant is docked`, () => {
279279
renderNav(
280-
<Nav isTextExpanded variant="docked" data-testid="text-expanded-nav">
280+
<Nav isExpanded variant="docked" data-testid="expanded-nav">
281281
<NavList>
282282
{props.items.map((item) => (
283283
<NavItem to={item.to} key={item.to}>
@@ -287,10 +287,10 @@ describe('Nav', () => {
287287
</NavList>
288288
</Nav>
289289
);
290-
expect(screen.getByTestId('text-expanded-nav')).toHaveClass(styles.modifiers.textExpanded);
290+
expect(screen.getByTestId('expanded-nav')).toHaveClass(styles.modifiers.expanded);
291291
});
292292

293-
test(`Does not render with ${styles.modifiers.textExpanded} class when isTextExpanded is not passed`, () => {
293+
test(`Does not render with ${styles.modifiers.expanded} class when isExpanded is not passed`, () => {
294294
renderNav(
295295
<Nav data-testid="nav">
296296
<NavList>
@@ -302,12 +302,12 @@ describe('Nav', () => {
302302
</NavList>
303303
</Nav>
304304
);
305-
expect(screen.getByTestId('nav')).not.toHaveClass(styles.modifiers.textExpanded);
305+
expect(screen.getByTestId('nav')).not.toHaveClass(styles.modifiers.expanded);
306306
});
307307

308-
test(`Does not render with ${styles.modifiers.textExpanded} class when isTextExpanded is true but variant is not docked`, () => {
308+
test(`Does not render with ${styles.modifiers.expanded} class when isExpanded is true but variant is not docked`, () => {
309309
renderNav(
310-
<Nav isTextExpanded data-testid="nav">
310+
<Nav isExpanded data-testid="nav">
311311
<NavList>
312312
{props.items.map((item) => (
313313
<NavItem to={item.to} key={item.to}>
@@ -317,6 +317,6 @@ describe('Nav', () => {
317317
</NavList>
318318
</Nav>
319319
);
320-
expect(screen.getByTestId('nav')).not.toHaveClass(styles.modifiers.textExpanded);
320+
expect(screen.getByTestId('nav')).not.toHaveClass(styles.modifiers.expanded);
321321
});
322322
});

0 commit comments

Comments
 (0)