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

[khadesamrudhi]

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

[patternfly-build]

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

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

[khadesamrudhi]

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]

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]

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]

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

[thatblindgeye]

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]

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]

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]);
	});
	});

[khadesamrudhi]

Pushed updates to fix review feedback:

• ToolbarItem: widths applied via setBreakpointCssVars using base token c_toolbar__item_Width; flexGrow via formatBreakpointMods; removed hardcoded data-testid; JSDoc clarifies flexGrow=1.
• ToolbarGroup: flexGrow via formatBreakpointMods; JSDoc updated.
• Tests: ToolbarItem width tests assert CSS var per breakpoint.

No extra demo files included.
Previous CI errors from missing react-tokens imports should be resolved. Could you please approve the workflow run so CI can proceed?

@thatblindgeye @mcoker @kmcfaul

[thatblindgeye]

Couple more change requests below. Also looks like there's still some unrelated file updates being made:

• .yarnrc.yml should be reverted
• cleanup.js should be removed
• eslint.config.mjs should be reverted
• packager.json should be reverted
• ToolbarDemo.tsx should be reverted
• tsconfig.base.json should be reverted
• build-single-packages.mjs should be reverted
• global.d.ts should be removed

The updates/additions of those files shouldn't be necessary for this PR

[thatblindgeye]

Related to the above comment, we'll need to tweak this and we can simplify it a bit :

Suggested change

	style={{
	...style,
	...(widths
	? setBreakpointCssVars(
	Object.entries(widths).reduce(
	(acc, [bp, size]) => {
	if (!size) {
	return acc;
	}
	const cssVarValueMap: Record<string, string> = {
	sm: 'var(--pf-c-toolbar__item--m-w-sm--Width)',
	md: 'var(--pf-c-toolbar__item--m-w-md--Width)',
	lg: 'var(--pf-c-toolbar__item--m-w-lg--Width)',
	xl: 'var(--pf-c-toolbar__item--m-w-xl--Width)',
	'2xl': 'var(--pf-c-toolbar__item--m-w-2xl--Width)',
	'3xl': 'var(--pf-c-toolbar__item--m-w-3xl--Width)',
	'4xl': 'var(--pf-c-toolbar__item--m-w-4xl--Width)'
	};
	const value = cssVarValueMap[size as keyof typeof cssVarValueMap];
	return value ? { ...acc, [bp]: value } : acc;
	},
	{} as Record<string, string>
	),
	(c_toolbar__item_Width as any).name
	)
	: undefined)
	}}
	style={{
	...style,
	...(widths ? setBreakpointCssVars(widths, c_toolbar__item_Width.name) : undefined)
	}}

Out of curiosity, where did the --pf-c-toolbar__item--m-w-xl--Width etc strings come from?

[thatblindgeye]

For this prop, we want the types to be just string, e.g. default?: string, as the expectation is that a consumer would pass a unit value for this prop like widths={{sm: '3rem'}} rather than us setting a token.

Suggested change

	default?: 'sm' | 'md' | 'lg' | 'xl' | '2xl' | '3xl' | '4xl';
	sm?: 'sm' | 'md' | 'lg' | 'xl' | '2xl' | '3xl' | '4xl';
	md?: 'sm' | 'md' | 'lg' | 'xl' | '2xl' | '3xl' | '4xl';
	lg?: 'sm' | 'md' | 'lg' | 'xl' | '2xl' | '3xl' | '4xl';
	xl?: 'sm' | 'md' | 'lg' | 'xl' | '2xl' | '3xl' | '4xl';
	'2xl'?: 'sm' | 'md' | 'lg' | 'xl' | '2xl' | '3xl' | '4xl';
	default?: string;
	sm?: string;
	md?: string;
	lg?: string;
	xl?: string;
	'2xl'?: string;