feat(toolbar): implement container query option #12174
Conversation
WalkthroughThis PR introduces container query support to the PatternFly Toolbar component, enabling responsive behavior based on container size rather than viewport width. New Changes
Sequence DiagramsequenceDiagram
actor User
participant Toolbar as Toolbar<br/>(useContainerQuery)
participant Observer as ResizeObserver<br/>or Window
participant Context as ToolbarContext
participant Child as Child Components<br/>(ToolbarToggleGroup)
participant Render as UI Rendering
User->>Toolbar: Mount with useContainerQuery=true
activate Toolbar
Toolbar->>Observer: Register observer on<br/>toolbar ref
deactivate Toolbar
Note over User,Render: Container or window resizes
Observer->>Toolbar: Trigger resize event<br/>(container width)
activate Toolbar
Toolbar->>Toolbar: Update containerWidth state<br/>Calculate breakpoint
Toolbar->>Toolbar: Determine if content<br/>should collapse
Toolbar->>Toolbar: closeExpandableContent<br/>if needed
Toolbar->>Context: Propagate useContainerQuery<br/>& containerQueryBreakpoint
deactivate Toolbar
Toolbar->>Child: Pass context (useContainerQuery)
activate Child
Child->>Child: formatBreakpointMods<br/>respects container mode
deactivate Child
Toolbar->>Render: Apply container-query<br/>CSS classes & render
Render->>User: Update UI visibility<br/>and layout
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
|
Preview: https://pf-react-pr-12174.surge.sh A11y report: https://pf-react-pr-12174-a11y.surge.sh |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/react-core/src/components/Toolbar/ToolbarToggleGroup.tsx (1)
263-273: Apply container query handling consistently to all breakpoint modifiers.The
breakpointModformatting correctly uses a conditional to passundefinedwhenuseContainerQueryis true (line 267), enabling container query support. However, the subsequentformatBreakpointModscalls forvisibility,gap,columnGap, androwGap(lines 269-272) always passgetBreakpoint(width), preventing them from responding to container queries. WhenuseContainerQueryis enabled, all breakpoint-aware modifiers should follow the same pattern asbreakpointMod:formatBreakpointMods(visibility, styles, '', useContainerQuery ? undefined : getBreakpoint(width)), formatBreakpointMods(gap, styles, '', useContainerQuery ? undefined : getBreakpoint(width)), formatBreakpointMods(columnGap, styles, '', useContainerQuery ? undefined : getBreakpoint(width)), formatBreakpointMods(rowGap, styles, '', useContainerQuery ? undefined : getBreakpoint(width)),
🧹 Nitpick comments (12)
packages/react-core/src/demos/examples/Toolbar/Toolbar.css (1)
5-5: Remove or uncomment the padding rule.The commented-out padding rule should either be removed if not needed or uncommented with an explanation if it will be used. Leaving commented code without context creates maintenance debt.
packages/react-core/src/demos/examples/Toolbar/ToolbarComparison.tsx (1)
37-88: Consider refactoring the parameter list.The
createToolbarContenthelper has 7 parameters, which makes the function calls verbose (lines 108-116, 140-148). Consider using an object parameter for better readability:-const createToolbarContent = ( - inputValue: string, - setInputValue: (value: string) => void, - statusExpanded: boolean, - setStatusExpanded: (value: boolean) => void, - statusSelected: string, - setStatusSelected: (value: string) => void, - label: string -) => ( +const createToolbarContent = ({ + inputValue, + setInputValue, + statusExpanded, + setStatusExpanded, + statusSelected, + setStatusSelected, + label +}: { + inputValue: string; + setInputValue: (value: string) => void; + statusExpanded: boolean; + setStatusExpanded: (value: boolean) => void; + statusSelected: string; + setStatusSelected: (value: string) => void; + label: string; +}) => (packages/react-core/src/demos/examples/Toolbar/ToolbarInSidebar.tsx (2)
77-222: Consider extracting a reusable toolbar items generator.The
sidebarToolbarItems(lines 77-120) andmainToolbarItems(lines 150-222) sections contain significant duplication. Both create a SearchInput and Select dropdowns with similar state management patterns. Consider extracting a reusable function to reduce duplication:const createToolbarItems = (config: { searchValue: string; onSearchChange: (value: string) => void; searchLabel: string; selects: Array<{ options: string[]; expanded: boolean; selected: string; onToggle: () => void; onSelect: (selection: string) => void; onOpenChange: (isOpen: boolean) => void; placeholder: string; width: string; }>; }) => { /* implementation */ };
96-96: Inline styles with explicit type casting.Lines 96, 169, and 198 use inline styles with explicit
as React.CSSPropertiescasting. While functional, consider extracting these to constants or using a CSS class for better maintainability:const selectToggleStyle: React.CSSProperties = { width: '150px' }; const riskToggleStyle: React.CSSProperties = { width: '120px' };Also applies to: 169-169, 198-198
packages/react-core/src/components/Toolbar/examples/ToolbarContainerQueryBasic.tsx (1)
70-74: Consider extracting inline styles.The inline style objects with explicit casting (lines 70-74 and 100-104) are repeated across multiple examples. Consider extracting to shared constants for consistency:
const MENU_TOGGLE_STYLES = { status: { width: '150px' } as React.CSSProperties, risk: { width: '120px' } as React.CSSProperties };Also applies to: 100-104
packages/react-core/src/components/Toolbar/Toolbar.tsx (3)
128-136: Redundant conditional logic.Line 130 has a redundant ternary:
specificBreakpoint === 'sm' ? 'sm' : specificBreakpointalways evaluates tospecificBreakpoint. The comment suggests handling a special 'sm' case, but the code doesn't actually do anything different.// Use container breakpoints when using container queries, otherwise use global breakpoints let isWideEnoughForInline: boolean; if (this.props.useContainerQuery) { - // Handle 'sm' case for container breakpoints - const breakpointKey = specificBreakpoint === 'sm' ? 'sm' : specificBreakpoint; - isWideEnoughForInline = newWidth >= containerBreakpoints[breakpointKey]; + isWideEnoughForInline = newWidth >= containerBreakpoints[specificBreakpoint]; } else { // Handle 'sm' case - map to 'md' since globalBreakpoints doesn't have 'sm' const breakpointKey = specificBreakpoint === 'sm' ? 'md' : specificBreakpoint; isWideEnoughForInline = newWidth >= globalBreakpoints[breakpointKey]; }
89-90: Consider stronger typing for observer.The
observerproperty is typed asanyand initialized to a no-op function. A more precise type would improve maintainability.toolbarRef = createRef<HTMLDivElement>(); - observer: any = () => {}; + observer: () => void = () => {};
153-160: Initial container width not captured on mount.When
useContainerQueryis enabled,containerWidthstarts at0but isn't set to the actual width until a resize event fires. Consider measuring the initial width immediately after setting up the observer.componentDidMount() { if (this.isToggleManaged() && canUseDOM) { if (this.props.useContainerQuery && this.toolbarRef.current) { this.observer = getResizeObserver(this.toolbarRef.current, this.closeExpandableContentOnContainerResize, true); + // Capture initial width + this.closeExpandableContentOnContainerResize(); } else { window.addEventListener('resize', this.closeExpandableContent); } } }packages/react-core/src/components/Toolbar/examples/ToolbarContainerQueryBreakpoints.tsx (2)
24-26: Misleading variable names:*Lgvariables used forxlbreakpoint.The state variables
inputValueLg,statusExpandedLg, andstatusSelectedLgare used for the'xl'breakpoint toolbar (line 127), which is confusing. Consider renaming for clarity.- const [inputValueLg, setInputValueLg] = useState(''); - const [statusExpandedLg, setStatusExpandedLg] = useState(false); - const [statusSelectedLg, setStatusSelectedLg] = useState(''); + const [inputValueXl, setInputValueXl] = useState(''); + const [statusExpandedXl, setStatusExpandedXl] = useState(false); + const [statusSelectedXl, setStatusSelectedXl] = useState('');And update the usage on lines 126-134 accordingly.
Also applies to: 126-134
70-75: Prefer stable keys over array indices.Using
indexas a key is acceptable here sincestatusOptionsis static, but using the option value itself would be more semantically meaningful and resilient to future changes.- {statusOptions.map((option, index) => ( - <SelectOption key={index} value={option}> + {statusOptions.map((option) => ( + <SelectOption key={option} value={option}> {option} </SelectOption> ))}packages/react-core/src/demos/examples/Toolbar/ToolbarInModal.tsx (1)
80-84: Consider using option values as keys instead of indices.Similar to the other example file, using the option string as the key would be more semantic.
- {statusOptions.map((option, index) => ( - <SelectOption key={index} value={option}> + {statusOptions.map((option) => ( + <SelectOption key={option} value={option}>Also applies to: 109-113
.claude/settings.local.json (1)
1-8: Clarify the naming of this committed configuration file:.local.jsonsuggests a developer-local override, but the file is tracked in git as team-wide policy.The allowlist is appropriate for the defined lint scripts (
lint:all,lint:md,lint:ts,lint:tests,lint:publication). Consider either renaming tosettings.json(if this is the official team config) or adding documentation in CONTRIBUTING.md to clarify the purpose and usage of.claude/settings.local.json.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.claude/settings.local.json(1 hunks)packages/react-core/src/components/Toolbar/Toolbar.tsx(8 hunks)packages/react-core/src/components/Toolbar/ToolbarToggleGroup.tsx(2 hunks)packages/react-core/src/components/Toolbar/ToolbarUtils.tsx(3 hunks)packages/react-core/src/components/Toolbar/examples/Toolbar.css(1 hunks)packages/react-core/src/components/Toolbar/examples/Toolbar.md(2 hunks)packages/react-core/src/components/Toolbar/examples/ToolbarContainerQueryBasic.tsx(1 hunks)packages/react-core/src/components/Toolbar/examples/ToolbarContainerQueryBreakpoints.tsx(1 hunks)packages/react-core/src/demos/Toolbar.md(3 hunks)packages/react-core/src/demos/examples/Toolbar/Toolbar.css(1 hunks)packages/react-core/src/demos/examples/Toolbar/ToolbarComparison.tsx(1 hunks)packages/react-core/src/demos/examples/Toolbar/ToolbarInModal.tsx(1 hunks)packages/react-core/src/demos/examples/Toolbar/ToolbarInSidebar.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
packages/react-core/src/components/Toolbar/examples/ToolbarContainerQueryBasic.tsx (12)
packages/react-core/src/components/Toolbar/ToolbarItem.tsx (1)
ToolbarItem(173-237)packages/react-core/src/components/SearchInput/SearchInput.tsx (1)
SearchInput(573-575)packages/react-styles/scripts/generateClassMaps.mjs (1)
value(37-37)packages/react-core/src/components/Toolbar/ToolbarGroup.tsx (1)
ToolbarGroup(237-239)packages/react-core/src/components/Select/Select.tsx (1)
Select(207-207)packages/react-core/src/components/MenuToggle/MenuToggle.tsx (2)
MenuToggleElement(23-23)MenuToggle(246-248)packages/react-core/src/components/Select/SelectList.tsx (1)
SelectList(13-22)packages/react-icons/scripts/writeIcons.mjs (1)
index(81-81)packages/react-core/src/components/Select/SelectOption.tsx (1)
SelectOption(48-50)packages/react-core/src/components/Toolbar/ToolbarToggleGroup.tsx (1)
ToolbarToggleGroup(293-293)packages/react-core/src/components/Toolbar/Toolbar.tsx (1)
Toolbar(292-292)packages/react-core/src/components/Toolbar/ToolbarContent.tsx (1)
ToolbarContent(127-127)
packages/react-core/src/demos/examples/Toolbar/ToolbarInSidebar.tsx (12)
packages/react-core/src/components/Page/PageToggleButton.tsx (1)
PageToggleButton(21-59)packages/react-core/src/components/Masthead/MastheadBrand.tsx (1)
MastheadBrand(11-19)packages/react-core/src/components/SearchInput/SearchInput.tsx (1)
SearchInput(573-575)packages/react-core/src/components/Select/Select.tsx (1)
Select(207-207)packages/react-core/src/components/MenuToggle/MenuToggle.tsx (2)
MenuToggleElement(23-23)MenuToggle(246-248)packages/react-core/src/components/Select/SelectList.tsx (1)
SelectList(13-22)packages/react-core/src/components/Select/SelectOption.tsx (1)
SelectOption(48-50)packages/react-core/src/components/Page/PageSidebar.tsx (1)
PageSidebar(30-58)packages/react-core/src/components/Toolbar/Toolbar.tsx (1)
Toolbar(292-292)packages/react-core/src/components/Toolbar/ToolbarContent.tsx (1)
ToolbarContent(127-127)packages/react-core/src/components/Toolbar/ToolbarToggleGroup.tsx (1)
ToolbarToggleGroup(293-293)packages/react-core/src/components/Page/Page.tsx (1)
Page(363-363)
packages/react-core/src/components/Toolbar/ToolbarToggleGroup.tsx (1)
packages/react-core/src/helpers/util.ts (2)
formatBreakpointMods(291-328)getBreakpoint(364-384)
packages/react-core/src/demos/examples/Toolbar/ToolbarComparison.tsx (11)
packages/react-core/src/components/SearchInput/SearchInput.tsx (1)
SearchInput(573-575)packages/react-core/src/components/Select/Select.tsx (1)
Select(207-207)packages/react-core/src/components/MenuToggle/MenuToggle.tsx (2)
MenuToggleElement(23-23)MenuToggle(246-248)packages/react-core/src/components/Select/SelectList.tsx (1)
SelectList(13-22)packages/react-core/src/layouts/Stack/Stack.tsx (1)
Stack(15-28)packages/react-core/src/layouts/Stack/StackItem.tsx (1)
StackItem(15-28)packages/react-core/src/components/Card/CardHeader.tsx (1)
CardHeader(82-215)packages/react-core/src/components/Title/Title.tsx (1)
Title(31-50)packages/react-core/src/components/Toolbar/Toolbar.tsx (1)
Toolbar(292-292)packages/react-core/src/components/Toolbar/ToolbarContent.tsx (1)
ToolbarContent(127-127)packages/react-core/src/components/Toolbar/ToolbarToggleGroup.tsx (1)
ToolbarToggleGroup(293-293)
packages/react-core/src/components/Toolbar/Toolbar.tsx (3)
packages/react-core/src/components/Toolbar/ToolbarUtils.tsx (2)
containerBreakpoints(56-62)globalBreakpoints(48-53)packages/react-core/src/helpers/resizeObserver.tsx (1)
getResizeObserver(70-108)packages/react-core/src/helpers/util.ts (2)
formatBreakpointMods(291-328)getBreakpoint(364-384)
packages/react-core/src/components/Toolbar/examples/ToolbarContainerQueryBreakpoints.tsx (11)
packages/react-core/src/components/Toolbar/ToolbarItem.tsx (1)
ToolbarItem(173-237)packages/react-core/src/components/SearchInput/SearchInput.tsx (1)
SearchInput(573-575)packages/react-core/src/components/Toolbar/ToolbarGroup.tsx (1)
ToolbarGroup(237-239)packages/react-core/src/components/Select/Select.tsx (1)
Select(207-207)packages/react-core/src/components/MenuToggle/MenuToggle.tsx (2)
MenuToggleElement(23-23)MenuToggle(246-248)packages/react-core/src/components/Select/SelectList.tsx (1)
SelectList(13-22)packages/react-core/src/components/Select/SelectOption.tsx (1)
SelectOption(48-50)packages/react-core/src/components/Title/Title.tsx (1)
Title(31-50)packages/react-core/src/components/Toolbar/Toolbar.tsx (1)
Toolbar(292-292)packages/react-core/src/components/Toolbar/ToolbarContent.tsx (1)
ToolbarContent(127-127)packages/react-core/src/components/Toolbar/ToolbarToggleGroup.tsx (1)
ToolbarToggleGroup(293-293)
🪛 GitHub Actions: Documentation
packages/react-core/src/components/Toolbar/Toolbar.tsx
[error] 225-225: TS2339: Property 'container' does not exist on type '{ hidden: "pf-m-hidden"; hiddenOnSm: "pf-m-hidden-on-sm"; visibleOnSm: "pf-m-visible-on-sm"; hiddenOnMd: "pf-m-hidden-on-md"; visibleOnMd: "pf-m-visible-on-md"; hiddenOnLg: "pf-m-hidden-on-lg"; ... 346 more ...; hideOn_2xl: "pf-m-hide-on-2xl"; }'.
🪛 GitHub Check: Build, test & deploy
packages/react-core/src/components/Toolbar/Toolbar.tsx
[failure] 234-234:
Property 'containerXl' does not exist on type '{ hidden: "pf-m-hidden"; hiddenOnSm: "pf-m-hidden-on-sm"; visibleOnSm: "pf-m-visible-on-sm"; hiddenOnMd: "pf-m-hidden-on-md"; visibleOnMd: "pf-m-visible-on-md"; hiddenOnLg: "pf-m-hidden-on-lg"; ... 346 more ...; hideOn_2xl: "pf-m-hide-on-2xl"; }'.
[failure] 233-233:
Property 'containerLg' does not exist on type '{ hidden: "pf-m-hidden"; hiddenOnSm: "pf-m-hidden-on-sm"; visibleOnSm: "pf-m-visible-on-sm"; hiddenOnMd: "pf-m-hidden-on-md"; visibleOnMd: "pf-m-visible-on-md"; hiddenOnLg: "pf-m-hidden-on-lg"; ... 346 more ...; hideOn_2xl: "pf-m-hide-on-2xl"; }'.
[failure] 232-232:
Property 'containerMd' does not exist on type '{ hidden: "pf-m-hidden"; hiddenOnSm: "pf-m-hidden-on-sm"; visibleOnSm: "pf-m-visible-on-sm"; hiddenOnMd: "pf-m-hidden-on-md"; visibleOnMd: "pf-m-visible-on-md"; hiddenOnLg: "pf-m-hidden-on-lg"; ... 346 more ...; hideOn_2xl: "pf-m-hide-on-2xl"; }'.
[failure] 231-231:
Property 'containerSm' does not exist on type '{ hidden: "pf-m-hidden"; hiddenOnSm: "pf-m-hidden-on-sm"; visibleOnSm: "pf-m-visible-on-sm"; hiddenOnMd: "pf-m-hidden-on-md"; visibleOnMd: "pf-m-visible-on-md"; hiddenOnLg: "pf-m-hidden-on-lg"; ... 346 more ...; hideOn_2xl: "pf-m-hide-on-2xl"; }'.
[failure] 230-230:
Property 'container_2xl' does not exist on type '{ hidden: "pf-m-hidden"; hiddenOnSm: "pf-m-hidden-on-sm"; visibleOnSm: "pf-m-visible-on-sm"; hiddenOnMd: "pf-m-hidden-on-md"; visibleOnMd: "pf-m-visible-on-md"; hiddenOnLg: "pf-m-hidden-on-lg"; ... 346 more ...; hideOn_2xl: "pf-m-hide-on-2xl"; }'.
[failure] 225-225:
Property 'container' does not exist on type '{ hidden: "pf-m-hidden"; hiddenOnSm: "pf-m-hidden-on-sm"; visibleOnSm: "pf-m-visible-on-sm"; hiddenOnMd: "pf-m-hidden-on-md"; visibleOnMd: "pf-m-visible-on-md"; hiddenOnLg: "pf-m-hidden-on-lg"; ... 346 more ...; hideOn_2xl: "pf-m-hide-on-2xl"; }'.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (15)
packages/react-core/src/components/Toolbar/examples/Toolbar.md (2)
8-8: LGTM!The CSS import is correctly placed and uses the appropriate relative path.
117-136: LGTM!The container queries documentation is clear and well-structured. It explains the use case (toolbars in constrained spaces), provides basic usage, and demonstrates breakpoint configuration.
packages/react-core/src/demos/Toolbar.md (3)
6-6: LGTM!The CSS import correctly references the demo-specific styling needed for container resize functionality.
16-17: LGTM!The new icon imports (FilterIcon and BarsIcon) are appropriately added for the container query demos.
29-41: ToolbarInModal.tsx exists in the repository.The file
packages/react-core/src/demos/examples/Toolbar/ToolbarInModal.tsxis present in the PR and the documentation reference is correct.packages/react-core/src/components/Toolbar/ToolbarToggleGroup.tsx (1)
194-194: LGTM!Correctly destructures
useContainerQueryfrom the ToolbarContext to enable container query support.packages/react-core/src/demos/examples/Toolbar/ToolbarComparison.tsx (2)
1-22: LGTM!The imports are well-organized and include all necessary components for the comparison demo.
105-151: LGTM!The comparison between media query (line 105) and container query (line 137) toolbars is clearly demonstrated. The use of
useContainerQueryprop on line 137 correctly enables container query mode.packages/react-core/src/demos/examples/Toolbar/ToolbarInSidebar.tsx (2)
138-138: LGTM!The sidebar toolbar correctly uses both
useContainerQueryandcontainerQueryBreakpoint="md"to enable container-aware responsive behavior.
241-241: LGTM!The main content toolbar correctly uses default viewport-based responsive behavior by not specifying
useContainerQuery, providing a clear contrast with the sidebar toolbar.packages/react-core/src/components/Toolbar/examples/ToolbarContainerQueryBasic.tsx (2)
18-48: LGTM!The state management and handler functions are well-structured and follow React best practices. The component correctly manages the toolbar's interactive state.
134-137: LGTM!The component correctly demonstrates container query usage by:
- Wrapping the toolbar in a resizable container (line 134)
- Enabling container queries with
useContainerQueryprop (line 135)This provides a clear, minimal example for users to follow.
packages/react-core/src/components/Toolbar/Toolbar.tsx (1)
260-261: LGTM: Context propagation of useContainerQuery.Propagating
useContainerQuerythrough context allows child components likeToolbarToggleGroupto adjust their behavior accordingly.packages/react-core/src/demos/examples/Toolbar/ToolbarInModal.tsx (2)
161-172: Verify: Toolbar placed inside ModalHeader children.The
Toolbaris rendered as a child ofModalHeader, which is unusual. Typically, toolbars appear in the modal body or as a separate section. If this is intentional for the demo, consider adding a comment explaining the design choice.
32-127: Demo implementation looks good overall.The demo effectively showcases the container query feature within a modal context. State management is clear, and the component structure follows PatternFly patterns.
| .ws-react-c-toolbar-resize-container { | ||
| resize: horizontal; | ||
| overflow: auto; | ||
| border: var(--pf-t--global--border--width--extra-strong) dashed var(--pf-t--global--border--color--default); | ||
| /* padding: var(--pf-t--global--spacer--md); */ | ||
| width: 800px; | ||
| min-width: 300px; | ||
| max-width: 100%; | ||
| height: 15em; | ||
| } | ||
|
|
There was a problem hiding this comment.
Consolidate duplicate CSS files.
This file is identical to packages/react-core/src/demos/examples/Toolbar/Toolbar.css. Having two identical CSS files creates maintenance overhead and risks divergence. Consider:
- Keeping one canonical version in a shared location
- Importing it from both demos and examples
- Or documenting why both copies are necessary
🤖 Prompt for AI Agents
packages/react-core/src/components/Toolbar/examples/Toolbar.css lines 1-11: this
CSS is duplicated in packages/react-core/src/demos/examples/Toolbar/Toolbar.css;
remove the duplicate and pick one canonical location (prefer the demos or a new
shared styles folder), update both demo and example imports to reference that
single file (or re-export it from a shared module), delete the redundant file,
and verify build imports/paths and any package exports so nothing breaks; if
both must remain temporarily, add a TODO comment pointing to the canonical file
and include a follow-up ticket.
| const createToolbar = ( | ||
| breakpoint: 'sm' | 'md' | 'lg', |
There was a problem hiding this comment.
Type mismatch: function called with 'xl' but typed for 'sm' | 'md' | 'lg'.
The createToolbar function's breakpoint parameter is typed as 'sm' | 'md' | 'lg', but it's invoked with 'xl' on line 127. This would cause a TypeScript error.
const createToolbar = (
- breakpoint: 'sm' | 'md' | 'lg',
+ breakpoint: 'sm' | 'md' | 'lg' | 'xl' | '2xl',
inputValue: string,
setInputValue: (value: string) => void,
statusExpanded: boolean,
setStatusExpanded: (value: boolean) => void,
statusSelected: string,
setStatusSelected: (value: string) => void
) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const createToolbar = ( | |
| breakpoint: 'sm' | 'md' | 'lg', | |
| const createToolbar = ( | |
| breakpoint: 'sm' | 'md' | 'lg' | 'xl' | '2xl', | |
| inputValue: string, | |
| setInputValue: (value: string) => void, | |
| statusExpanded: boolean, | |
| setStatusExpanded: (value: boolean) => void, | |
| statusSelected: string, | |
| setStatusSelected: (value: string) => void | |
| ) => { |
🤖 Prompt for AI Agents
In
packages/react-core/src/components/Toolbar/examples/ToolbarContainerQueryBreakpoints.tsx
around lines 30-31, the createToolbar function's breakpoint parameter is
currently typed as 'sm' | 'md' | 'lg' but is later called with 'xl' (line ~127);
update the function signature to include 'xl' (or change the call to a supported
value) so the types align — modify the type to 'sm' | 'md' | 'lg' | 'xl' (or
adjust the invocation to use an existing union member) and ensure any downstream
logic that depends on breakpoint handles 'xl'.
| useContainerQuery && !containerQueryBreakpoint && styles.modifiers.container, | ||
| useContainerQuery && | ||
| containerQueryBreakpoint && | ||
| ((): string => { | ||
| const breakpointClassMap: Record<string, string> = { | ||
| '2xl': styles.modifiers.container_2xl, | ||
| sm: styles.modifiers.containerSm, | ||
| md: styles.modifiers.containerMd, | ||
| lg: styles.modifiers.containerLg, | ||
| xl: styles.modifiers.containerXl | ||
| }; | ||
| return breakpointClassMap[containerQueryBreakpoint] || ''; | ||
| })(), |
There was a problem hiding this comment.
Build failure: CSS modifier classes don't exist yet.
The TypeScript build fails because styles.modifiers.container, styles.modifiers.containerSm, etc. don't exist in the current @patternfly/react-styles package. This PR depends on patternfly/patternfly#8025 for these CSS classes.
Consider one of:
- Wait for the core PR to merge and release before this PR
- Add type assertions to suppress errors temporarily (not recommended)
- Use string literals for class names until the styles package is updated
- useContainerQuery && !containerQueryBreakpoint && styles.modifiers.container,
- useContainerQuery &&
- containerQueryBreakpoint &&
- ((): string => {
- const breakpointClassMap: Record<string, string> = {
- '2xl': styles.modifiers.container_2xl,
- sm: styles.modifiers.containerSm,
- md: styles.modifiers.containerMd,
- lg: styles.modifiers.containerLg,
- xl: styles.modifiers.containerXl
- };
- return breakpointClassMap[containerQueryBreakpoint] || '';
- })(),
+ useContainerQuery && !containerQueryBreakpoint && 'pf-m-container',
+ useContainerQuery &&
+ containerQueryBreakpoint &&
+ `pf-m-container-${containerQueryBreakpoint}`,Note: The above is a workaround. The proper fix is to coordinate with the core PatternFly release.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useContainerQuery && !containerQueryBreakpoint && styles.modifiers.container, | |
| useContainerQuery && | |
| containerQueryBreakpoint && | |
| ((): string => { | |
| const breakpointClassMap: Record<string, string> = { | |
| '2xl': styles.modifiers.container_2xl, | |
| sm: styles.modifiers.containerSm, | |
| md: styles.modifiers.containerMd, | |
| lg: styles.modifiers.containerLg, | |
| xl: styles.modifiers.containerXl | |
| }; | |
| return breakpointClassMap[containerQueryBreakpoint] || ''; | |
| })(), | |
| useContainerQuery && !containerQueryBreakpoint && 'pf-m-container', | |
| useContainerQuery && | |
| containerQueryBreakpoint && | |
| `pf-m-container-${containerQueryBreakpoint}`, |
🧰 Tools
🪛 GitHub Actions: Documentation
[error] 225-225: TS2339: Property 'container' does not exist on type '{ hidden: "pf-m-hidden"; hiddenOnSm: "pf-m-hidden-on-sm"; visibleOnSm: "pf-m-visible-on-sm"; hiddenOnMd: "pf-m-hidden-on-md"; visibleOnMd: "pf-m-visible-on-md"; hiddenOnLg: "pf-m-hidden-on-lg"; ... 346 more ...; hideOn_2xl: "pf-m-hide-on-2xl"; }'.
🪛 GitHub Check: Build, test & deploy
[failure] 234-234:
Property 'containerXl' does not exist on type '{ hidden: "pf-m-hidden"; hiddenOnSm: "pf-m-hidden-on-sm"; visibleOnSm: "pf-m-visible-on-sm"; hiddenOnMd: "pf-m-hidden-on-md"; visibleOnMd: "pf-m-visible-on-md"; hiddenOnLg: "pf-m-hidden-on-lg"; ... 346 more ...; hideOn_2xl: "pf-m-hide-on-2xl"; }'.
[failure] 233-233:
Property 'containerLg' does not exist on type '{ hidden: "pf-m-hidden"; hiddenOnSm: "pf-m-hidden-on-sm"; visibleOnSm: "pf-m-visible-on-sm"; hiddenOnMd: "pf-m-hidden-on-md"; visibleOnMd: "pf-m-visible-on-md"; hiddenOnLg: "pf-m-hidden-on-lg"; ... 346 more ...; hideOn_2xl: "pf-m-hide-on-2xl"; }'.
[failure] 232-232:
Property 'containerMd' does not exist on type '{ hidden: "pf-m-hidden"; hiddenOnSm: "pf-m-hidden-on-sm"; visibleOnSm: "pf-m-visible-on-sm"; hiddenOnMd: "pf-m-hidden-on-md"; visibleOnMd: "pf-m-visible-on-md"; hiddenOnLg: "pf-m-hidden-on-lg"; ... 346 more ...; hideOn_2xl: "pf-m-hide-on-2xl"; }'.
[failure] 231-231:
Property 'containerSm' does not exist on type '{ hidden: "pf-m-hidden"; hiddenOnSm: "pf-m-hidden-on-sm"; visibleOnSm: "pf-m-visible-on-sm"; hiddenOnMd: "pf-m-hidden-on-md"; visibleOnMd: "pf-m-visible-on-md"; hiddenOnLg: "pf-m-hidden-on-lg"; ... 346 more ...; hideOn_2xl: "pf-m-hide-on-2xl"; }'.
[failure] 230-230:
Property 'container_2xl' does not exist on type '{ hidden: "pf-m-hidden"; hiddenOnSm: "pf-m-hidden-on-sm"; visibleOnSm: "pf-m-visible-on-sm"; hiddenOnMd: "pf-m-hidden-on-md"; visibleOnMd: "pf-m-visible-on-md"; hiddenOnLg: "pf-m-hidden-on-lg"; ... 346 more ...; hideOn_2xl: "pf-m-hide-on-2xl"; }'.
[failure] 225-225:
Property 'container' does not exist on type '{ hidden: "pf-m-hidden"; hiddenOnSm: "pf-m-hidden-on-sm"; visibleOnSm: "pf-m-visible-on-sm"; hiddenOnMd: "pf-m-hidden-on-md"; visibleOnMd: "pf-m-visible-on-md"; hiddenOnLg: "pf-m-hidden-on-lg"; ... 346 more ...; hideOn_2xl: "pf-m-hide-on-2xl"; }'.
🤖 Prompt for AI Agents
packages/react-core/src/components/Toolbar/Toolbar.tsx around lines 225 to 237:
the code currently references non‑existent properties on styles.modifiers
(container, containerSm, containerMd, containerLg, containerXl, container_2xl)
which breaks the TypeScript build; replace those property accesses with explicit
string literal class names (use the class names added by
patternfly/patternfly#8025 as the fallback values) or guard with a fallback
string (e.g., styles.modifiers?.container || 'pf-c-toolbar__container') so
builds succeed until the core package is released, and add a TODO comment to
revert to styles.modifiers.* once the patternfly release is available
(alternatively delay merge or use a narrow type assertion if you prefer).
| // Container query breakpoints match CSS container query values | ||
| export const containerBreakpoints = { | ||
| sm: 286, | ||
| md: 478, | ||
| lg: 702, | ||
| xl: 992, // You may need to verify this value | ||
| '2xl': 1200 // You may need to verify this value | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
PatternFly container query breakpoints CSS values @container
💡 Result:
PatternFly’s standard breakpoint sizes (exposed as CSS variables) are:
- sm: 36rem — 576px. [--pf-global--breakpoint--sm]. [1]
- md: 48rem — 768px. [--pf-global--breakpoint--md]. [1]
- lg: 62rem — 992px. [--pf-global--breakpoint--lg]. [1]
- xl: 75rem — 1200px. [--pf-global--breakpoint--xl]. [1]
- 2xl: 90.625rem — 1450px. [--pf-global--breakpoint--2xl]. [1]
You can use those values in container queries (or media/container-query CSS) directly or via the PatternFly CSS custom properties. Example usages:
- @container (min-width: 36rem) { /* sm and up relative to container */ } [1]
- @container (min-width: var(--pf-global--breakpoint--lg)) { /* lg and up */ } [2]
Sources: PatternFly development guidelines (breakpoint values) and PatternFly global CSS variables. [1][2]
References
[1] PatternFly development guidelines — breakpoint values.
[2] PatternFly global CSS variables (variable names such as --pf-global--breakpoint--[breakpoint]).
🏁 Script executed:
# Search for references to containerBreakpoints and context about these values
rg "containerBreakpoints|container.*breakpoint" packages/react-core/src/components/Toolbar/ -A 3 -B 3
# Also check if there are comments or documentation explaining the choice
rg "286|478|702" packages/ -B 2 -A 2
# Search for any related issue references or PR descriptions
rg "8025|container.*query" packages/react-core/src/components/Toolbar/ -B 2 -A 2Repository: patternfly/patternfly-react
Length of output: 50384
🏁 Script executed:
# Search for CSS container query definitions or related documentation
fd -e css -e scss | xargs rg -l "container.*query|@container" 2>/dev/null | head -10
# Look for any tests or examples that might validate these breakpoint values
fd -e test.tsx -e spec.tsx | xargs rg "containerBreakpoints|286|478|702" 2>/dev/null | head -20
# Check if there's any documentation about these values
fd -e md | xargs rg "container.*breakpoint|286|478|702" 2>/dev/nullRepository: patternfly/patternfly-react
Length of output: 21022
Remove unverified comments from hardcoded breakpoint values or verify them against the CSS container query definitions.
The hardcoded containerBreakpoints values (sm: 286, md: 478, lg: 702, xl: 992, 2xl: 1200) do not match PatternFly's standard global breakpoints (sm: 576px, md: 768px, lg: 992px, xl: 1200px, 2xl: 1450px). While these custom values appear intentional for container-based layout, the comments flagging xl and 2xl as needing verification indicate uncertainty. Either remove the comments if these values are confirmed, or provide a reference to the CSS @container definitions from the PatternFly core CSS that validates them.
🤖 Prompt for AI Agents
In packages/react-core/src/components/Toolbar/ToolbarUtils.tsx around lines 55
to 62, the hardcoded containerBreakpoints include unverified inline comments for
xl and 2xl and contain values that don't match PatternFly global breakpoints;
either remove the “You may need to verify this value” comments if these numbers
are intentionally correct, or verify and update the breakpoint numbers to match
the CSS @container definitions (or PatternFly globals) and add a short code
comment referencing the exact CSS file and selector/variable (e.g., core CSS
file and @container rules) used to derive the values so future reviewers can
confirm them.
2 participants
What: This implements support for allowing the toolbar to respond to container queries.
Closes #12173
Additional issues: This relies on a core PR implementing container queries patternfly/patternfly#8025
Implemented with Claude and Cursor and a sprinkling of magic.
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.