Skip to content

fix(Toolbar filter): Fixed null exception in Toolbar filter#12352

Merged
dlabaj merged 2 commits intopatternfly:mainfrom
tlabaj:toolbar_bug
Apr 16, 2026
Merged

fix(Toolbar filter): Fixed null exception in Toolbar filter#12352
dlabaj merged 2 commits intopatternfly:mainfrom
tlabaj:toolbar_bug

Conversation

@tlabaj
Copy link
Copy Markdown
Contributor

@tlabaj tlabaj commented Apr 10, 2026

What: Closes #12267

Summary

Fixes a crash in ToolbarFilter when label portal targets are missing or not yet mounted (#12267).

Changes

  • Collapsed listed filters: Portal only when labelGroupContentRef.current.firstElementChild resolves to a real node (fixes incorrect !== null checks vs undefined).
  • Toolbar content consumer: Use safe access for labelContainerRef so default / unset context does not throw on first render.

Tests

  • Added ToolbarFilter.test.tsx for default context and labelGroupContentRef.current === null while collapsed.

Summary by CodeRabbit

  • Bug Fixes

    • Improved ToolbarFilter stability so it no longer errors when internal references are missing or when rendered in different expanded/collapsed states, preventing render-time failures.
  • Tests

    • Added tests confirming ToolbarFilter renders reliably both inside and outside toolbar contexts and when certain internal conditions are null.
  • Chores

    • No public API or exported entities were changed.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 10, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9ca32ad8-f7df-4e5f-83ec-e4602c717be9

📥 Commits

Reviewing files that changed from the base of the PR and between 6a6d106 and e1aa72e.

📒 Files selected for processing (1)
  • packages/react-core/src/components/Toolbar/ToolbarFilter.tsx

Walkthrough

Null-safe changes to ToolbarFilter's portal target derivation for both collapsed and expanded states; added tests to ensure ToolbarFilter renders when related ref targets are null or when not nested under Toolbar/ToolbarContent.

Changes

Cohort / File(s) Summary
ToolbarFilter implementation
packages/react-core/src/components/Toolbar/ToolbarFilter.tsx
Adjusted portal target computation to avoid direct dereferencing of ref internals: compute collapsedLabelPortalTarget via optional chaining and labelContainer via ...Ref?.current ?? null; render portals only when the computed target is non-null.
ToolbarFilter tests
packages/react-core/src/components/Toolbar/__tests__/ToolbarFilter.test.tsx
Added tests verifying ToolbarFilter renders without throwing when not nested under Toolbar/ToolbarContent and when labelGroupContentRef.current is null while collapsed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly addresses the main fix: resolving a null exception in ToolbarFilter, which is the primary objective of this PR.
Linked Issues check ✅ Passed The PR successfully addresses all coding requirements from issue #12267: prevents null/undefined dereferences using optional chaining, ensures portal rendering only with valid DOM nodes, and guards toolbar content consumer accessors.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the null exception in ToolbarFilter: implementation changes in ToolbarFilter.tsx and supporting test coverage in ToolbarFilter.test.tsx remain aligned with issue #12267.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@patternfly-build
Copy link
Copy Markdown
Collaborator

patternfly-build commented Apr 10, 2026

@tlabaj tlabaj requested review from a team, dlabaj and wise-king-sullyman and removed request for a team April 13, 2026 15:27
Copy link
Copy Markdown
Contributor

@dlabaj dlabaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor comment.

{showToolbarItem && <ToolbarItem {...props}>{children}</ToolbarItem>}
{labelGroupContentRef?.current?.firstElementChild !== null &&
ReactDOM.createPortal(labelGroup, labelGroupContentRef.current.firstElementChild)}
{collapsedLabelPortalTarget != null && ReactDOM.createPortal(labelGroup, collapsedLabelPortalTarget)}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be !==

return (
<Fragment>
{showToolbarItem && <ToolbarItem {...props}>{children}</ToolbarItem>}
{labelContainer != null && ReactDOM.createPortal(labelGroup, labelContainer)}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

@dlabaj dlabaj merged commit 7ed1a01 into patternfly:main Apr 16, 2026
14 checks passed
@patternfly-build
Copy link
Copy Markdown
Collaborator

Your changes have been released in:

  • @patternfly/react-code-editor@6.5.0-prerelease.60
  • @patternfly/react-core@6.5.0-prerelease.57
  • @patternfly/react-docs@7.5.0-prerelease.67
  • @patternfly/react-drag-drop@6.5.0-prerelease.58
  • demo-app-ts@6.5.0-prerelease.86
  • @patternfly/react-table@6.5.0-prerelease.59
  • @patternfly/react-templates@6.5.0-prerelease.57

Thanks for your contribution! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression - [ToolbarFilter] - [chipGroupContentRef.current is null] #10857

4 participants