fix(Toolbar): add width and flex-grow props to ToolbarItem; add flex-grow to ToolbarGroup; update demo and tests #12074

khadesamrudhi · 2025-10-20T18:36:39Z

This PR adds the previously removed widths prop to <ToolbarItem /> and introduces a new flexGrow prop that maps to .pf-m-flex-grow[on-{breakpoint}] modifiers.

Changes

Added responsive widths and flexGrow support using formatBreakpointMods
Updated unit tests to verify classnames
All ToolbarItem tests passing ✅

Closes #11910

… classes (patternfly#11910)

patternfly-build · 2025-10-20T18:40:58Z

Preview: https://pf-react-pr-12074.surge.sh

A11y report: https://pf-react-pr-12074-a11y.surge.sh

…exGrow; update demo and tests; fix lint

khadesamrudhi · 2025-10-23T11:32:03Z

First-time contributor, could a maintainer please approve workflows and re-run the failed job?

Notes:

The TypeScript parse error in packages/tsconfig.base.json was fixed.
Remaining failure is a transient Surge preview upload DNS error (EAI_AGAIN), unrelated to code changes.
Changes are limited to ToolbarItem widths + flexGrow, ToolbarGroup flexGrow, tests, demo, and types/global.d.ts.

Thanks!

@mcoker @kmcfaul @thatblindgeye

khadesamrudhi · 2025-10-23T12:37:06Z

I’ve fixed the packages/tsconfig.base.json JSON issue and pushed the update. Could you please approve the workflows and re-run the CI? If Surge preview shows EAI_AGAIN, it indicates a transient network error that we can retry. Thanks!

khadesamrudhi · 2025-10-28T06:30:10Z

Hi, all automated checks have passed.
Requesting a review when convenient.
Please let me know if any additional changes are needed. Thank you.
@mcoker @thatblindgeye @kmcfaul

thatblindgeye

Some comments below. Additionally, there are several files updates not related to the issue. If those updates aren't necessary for the flexGrow and widths updates being made here, I might suggest pulling those out in favor of a separate issue (if they're more enhancements you feel would be good for the codebase for example)

thatblindgeye · 2025-10-28T14:02:52Z

packages/react-core/src/components/Toolbar/ToolbarItem.tsx

          {...(variant === 'label' && { 'aria-hidden': true })}
          id={id}
          role={role}
+          data-testid="toolbaritem"


Not sure about hardcoding a testid in, since there could be many items on a page, and a testid should be able to be passed with the spread props line below this.

thatblindgeye · 2025-10-28T14:12:48Z

packages/react-core/src/components/Toolbar/ToolbarGroup.tsx

    xl?: 'wrap' | 'nowrap';
    '2xl'?: 'wrap' | 'nowrap';
  };
+  /** Flex grow modifier at various breakpoints */


Suggested change

/** Flex grow modifier at various breakpoints */

/** Indicates whether a flex grow modifier of 1 is applied at various breakpoints */

Small nit to the description, just to make it clear that it's currently a hardcoded flex grow modifier rather than something more customizable.

thatblindgeye · 2025-10-28T14:20:09Z

packages/react-core/src/components/Toolbar/ToolbarGroup.tsx

+              flexGrow &&
+                Object.entries(flexGrow).reduce(
+                  (acc, [bp]) => ({
+                    ...acc,
+                    [`pf-m-flex-grow${bp !== 'default' ? `-on-${bp}` : ''}`]: true
+                  }),
+                  {}
+                ),


Rather than this, we can just use our formateBreakpointMods util function

thatblindgeye · 2025-10-28T14:20:52Z

packages/react-core/src/components/Toolbar/ToolbarItem.tsx

+    xl?: 'sm' | 'md' | 'lg' | 'xl' | '2xl' | '3xl' | '4xl';
+    '2xl'?: 'sm' | 'md' | 'lg' | 'xl' | '2xl' | '3xl' | '4xl';
+  };
+  /** Flex grow modifier at various breakpoints */


Similar nit here for prop description as above

thatblindgeye · 2025-10-28T14:21:51Z

packages/react-core/src/components/Toolbar/ToolbarItem.tsx

+            widths &&
+              Object.entries(widths).reduce(
+                (acc, [bp, size]) => ({
+                  ...acc,
+                  [`pf-m-w-${size}${bp !== 'default' ? `-on-${bp}` : ''}`]: true
+                }),
+                {}
+              ),
+            flexGrow &&
+              Object.entries(flexGrow).reduce(
+                (acc, [bp]) => ({
+                  ...acc,
+                  [`pf-m-flex-grow${bp !== 'default' ? `-on-${bp}` : ''}`]: true
+                }),
+                {}
+              )


Similar to above, we should be able to just use our formatBreakpointMods util function instead

Just an addendum to this, we should use the formatBreakpointMods function for the flexGrow here, but for handling applying the widths prop we should basically just add back in what was removed in this React PR https://github.com/patternfly/patternfly-react/pull/10022/files#diff-7beeea05e52252eb360f045d661fab89fe634b00becfd8a3dbaa48338c6032c0 (we want to apply a styles object rather than a class)

thatblindgeye · 2025-10-28T14:25:37Z

packages/react-integration/demo-app-ts/src/components/demos/ToolbarDemo.tsx

@@ -0,0 +1,40 @@
+import { useState } from 'react';


Was there an intention to add Cypress tests for this demo? Not entirely sure we need one to test the new props, so my feeling is we could remove this demo file and revert the changes made to the ToolbarDemo/ToolbarDemo.tsx file.

thatblindgeye · 2025-10-28T14:34:04Z

packages/react-core/src/components/Toolbar/__tests__/ToolbarGroup.test.tsx

+  describe('ToolbarGroup flexGrow', () => {
+    const bps = ['default', 'sm', 'md', 'lg', 'xl', '2xl'];
+
+    describe.each(bps)('flexGrow at various breakpoints', (bp) => {
+      it(`should render with pf-m-flex-grow when flexGrow is set at ${bp}`, () => {


We typically use the describe block a bit more rarely in our codebase, in this case I'm not sure we should use it. Instead what we could do is put the breakpoints variable as a global var or within the first describe('ToolbarGroup' ... block, then:

Move the flexGrow and widths tests inside that block

remove the nested describe blocks being used here

instead, just run a forEach over the breakpoints array and call the tests in this forEach block

We do something similar in some of our unit test files, such as our Button tests:

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

Lines 10 to 22 in 9421a92

Object.values(ButtonVariant).forEach((variant) => {

if (variant !== 'primary') {

test(`Does not render with class ${styles.modifiers[variant]} by default`, () => {

render(<Button>Not {variant} Button</Button>);

expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers[variant]);

});

}

test(`Renders with class ${styles.modifiers[variant]} when variant=${variant}`, () => {

render(<Button variant={variant}>{variant} Button</Button>);

expect(screen.getByRole('button')).toHaveClass(styles.modifiers[variant]);

});

});

fix(ToolbarItem): add width and flex-grow props and handle breakpoint…

8ee57f2

… classes (patternfly#11910)

khadesamrudhi mentioned this pull request Oct 20, 2025

ToolbarItem - props for width and flex-grow #11910

Open

mcoker requested review from kmcfaul, mcoker and thatblindgeye October 20, 2025 20:07

khadesamrudhi added 2 commits October 22, 2025 21:22

fix: update toolbar item width and flex-grow properties

834d7e4

feat(Toolbar): add ToolbarItem widths + flexGrow; add ToolbarGroup fl…

923dff4

…exGrow; update demo and tests; fix lint

khadesamrudhi changed the title ~~fix(ToolbarItem): add width and flex-grow props and handle breakpoint…~~ fix(Toolbar): add width and flex-grow props to ToolbarItem; add flex-grow to ToolbarGroup; update demo and tests Oct 23, 2025

chore(build): fix malformed packages/tsconfig.base.json

b8f9f6e

thatblindgeye requested changes Oct 28, 2025

View reviewed changes

	/** Flex grow modifier at various breakpoints */
	/** Indicates whether a flex grow modifier of 1 is applied at various breakpoints */

	Object.values(ButtonVariant).forEach((variant) => {
	if (variant !== 'primary') {
	test(`Does not render with class ${styles.modifiers[variant]} by default`, () => {
	render(<Button>Not {variant} Button</Button>);
	expect(screen.getByRole('button')).not.toHaveClass(styles.modifiers[variant]);
	});
	}

	test(`Renders with class ${styles.modifiers[variant]} when variant=${variant}`, () => {
	render(<Button variant={variant}>{variant} Button</Button>);
	expect(screen.getByRole('button')).toHaveClass(styles.modifiers[variant]);
	});
	});

fix(Toolbar): add width and flex-grow props to ToolbarItem; add flex-grow to ToolbarGroup; update demo and tests #12074

Are you sure you want to change the base?

fix(Toolbar): add width and flex-grow props to ToolbarItem; add flex-grow to ToolbarGroup; update demo and tests #12074

Conversation

khadesamrudhi commented Oct 20, 2025

Changes

Uh oh!

patternfly-build commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

khadesamrudhi commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

khadesamrudhi commented Oct 23, 2025

Uh oh!

khadesamrudhi commented Oct 28, 2025

Uh oh!

thatblindgeye left a comment

Choose a reason for hiding this comment

Uh oh!

thatblindgeye Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

thatblindgeye Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

thatblindgeye Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

thatblindgeye Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

thatblindgeye Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

thatblindgeye Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thatblindgeye Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

thatblindgeye Oct 28, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

patternfly-build commented Oct 20, 2025 •

edited

Loading

khadesamrudhi commented Oct 23, 2025 •

edited

Loading

thatblindgeye Oct 28, 2025 •

edited

Loading