Fix/global header icons

[rohitratannagar]

Hey, I just made a Pull Request!

✔️ Checklist

• A changeset describing the change and affected packages. (more info)
• Added or Updated documentation
• Tests for new functionality and regression tests for bug fixes
• Screenshots attached (for UI changes)

[rhdh-gh-app]

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@red-hat-developer-hub/backstage-plugin-global-header	workspaces/global-header/plugins/global-header	patch	v2.0.0

[rhdh-qodo-merge]

PR Summary by Qodo

Fix global-header icon fallbacks and restore notebook sidebar separators

🐞 Bug fix 🧪 Tests ⚙️ Configuration changes 🕐 20-40 Minutes

AI Description

• Add bundled MUI icon fallbacks so default global-header icons render without the webfont.
• Adjust global-header icon button sizing to improve toolbar icon visibility/consistency.
• Restore notebook sidebar section divider and make drawer splitter border visible (with regression
 test).

Diagram

graph TD
  A["Global Header toolbar"] --> B["HeaderIcon"] --> C["Built-in icon map"] --> D{{"@mui/icons-material"}}
  A --> E["HeaderIconButton"]
  F["NotebookView"] --> G["Drawer splitter style"]
  H["DocumentSidebar"] --> I["Section divider"]
  J["DocumentSidebar.test"] --> H

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Ensure Material Icons webfont is always loaded

• ➕ No need to maintain a local fallback icon map
• ➕ Potentially smaller JS bundle if icons are primarily font-based
• ➖ Couples correct rendering to runtime font loading and CSP/network conditions
• ➖ Harder to guarantee in embedded/backstage deployments; regressions likely to recur

2. Use a centralized icon registry (Backstage/Iconify)

• ➕ Single place to map icon names to renderers across plugins
• ➕ Scales better if many icons need consistent handling
• ➖ Larger refactor across plugins/components
• ➖ May introduce new dependency/behavior differences vs current MUI-based rendering

Recommendation: The PR’s approach (small, explicit bundled fallbacks for known default icon names) is a good reliability trade-off: it eliminates dependency on the Material Icons webfont for common cases while keeping the change localized. If more icons start failing, consider graduating the BUILTIN_ICONS mapping into a shared icon registry to avoid per-component growth and to keep bundle impact intentional.

Files changed (6) +45 / -3

Enhancement (1) +1 / -1

HeaderIconButton.tsxChange default HeaderIconButton size to medium +1/-1

Change default HeaderIconButton size to medium

• Updates the default icon button size from small to medium to improve consistent icon presentation in the global header.

workspaces/global-header/plugins/global-header/src/components/HeaderIconButton/HeaderIconButton.tsx

Bug fix (3) +34 / -2

HeaderIcon.tsxAdd bundled MUI icon fallback mapping for default icon names +20/-1

Add bundled MUI icon fallback mapping for default icon names

• Adds a BUILTIN_ICONS map (starting with addCircleOutline) and renders these components when the requested icon name matches. This avoids relying on the Material Icons ligature webfont being present at runtime.

workspaces/global-header/plugins/global-header/src/components/HeaderIcon/HeaderIcon.tsx

DocumentSidebar.tsxRestore horizontal divider between notebook title and documents list +13/-1

Restore horizontal divider between notebook title and documents list

• Introduces sectionDivider styles and renders an accessible horizontal separator between the title row and the documents section, restoring the missing visual separation.

workspaces/intelligent-assistant/plugins/intelligent-assistant/src/components/notebooks/DocumentSidebar.tsx

NotebookView.tsxAdd drawer splitter border styling for visibility +1/-0

Add drawer splitter border styling for visibility

• Adds a right border to the PatternFly drawer splitter so the resize handle/divider is visible against the floating background.

workspaces/intelligent-assistant/plugins/intelligent-assistant/src/components/notebooks/NotebookView.tsx

Tests (1) +5 / -0

DocumentSidebar.test.tsxAdd regression test for sidebar section separator +5/-0

Add regression test for sidebar section separator

• Adds a test asserting a role='separator' element is rendered between the title and documents sections.

workspaces/intelligent-assistant/plugins/intelligent-assistant/src/components/tests/DocumentSidebar.test.tsx

Other (1) +5 / -0

real-windows-remain.mdAdd changeset for global-header icon fix patch release +5/-0

Add changeset for global-header icon fix patch release

• Introduces a patch changeset for @red-hat-developer-hub/backstage-plugin-global-header documenting the icon fix.

workspaces/global-header/.changeset/real-windows-remain.md

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.06%. Comparing base (d755095) to head (d0bc859).
⚠️ Report is 13 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4174   +/-   ##
=======================================
  Coverage   58.06%   58.06%           
=======================================
  Files        2411     2411           
  Lines       96368    96372    +4     
  Branches    26846    26858   +12     
=======================================
+ Hits        55954    55958    +4     
  Misses      38918    38918           
  Partials     1496     1496           

Flag	Coverage Δ		*Carryforward flag
adoption-insights	84.55% <ø> (ø)		Carriedforward from d755095
ai-integrations	69.76% <ø> (ø)		Carriedforward from d755095
app-defaults	69.79% <ø> (ø)		Carriedforward from d755095
augment	46.67% <ø> (ø)		Carriedforward from d755095
boost	76.77% <ø> (ø)		Carriedforward from d755095
bulk-import	72.56% <ø> (ø)		Carriedforward from d755095
cost-management	13.55% <ø> (ø)		Carriedforward from d755095
dcm	60.72% <ø> (ø)		Carriedforward from d755095
extensions	56.59% <ø> (ø)		Carriedforward from d755095
global-floating-action-button	71.18% <ø> (ø)		Carriedforward from d755095
global-header	66.61% <100.00%> (+0.11%)	⬆️
homepage	47.50% <ø> (ø)		Carriedforward from d755095
install-dynamic-plugins	59.95% <ø> (ø)		Carriedforward from d755095
intelligent-assistant	74.61% <ø> (ø)		Carriedforward from d755095
konflux	91.98% <ø> (ø)		Carriedforward from d755095
lightspeed	69.02% <ø> (ø)		Carriedforward from d755095
mcp-integrations	83.40% <ø> (ø)		Carriedforward from d755095
orchestrator	66.87% <ø> (ø)		Carriedforward from d755095
quickstart	63.74% <ø> (ø)		Carriedforward from d755095
sandbox	79.56% <ø> (ø)		Carriedforward from d755095
scorecard	85.45% <ø> (ø)		Carriedforward from d755095
theme	88.52% <ø> (ø)		Carriedforward from d755095
translations	5.12% <ø> (ø)		Carriedforward from d755095
x2a	79.20% <ø> (ø)		Carriedforward from d755095

*This pull request uses carry forward flags. Click here to find out more.

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d755095...d0bc859. Read the comment docs.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[rhdh-qodo-merge]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Unsafe builtin icon lookup 🐞 Bug ☼ Reliability

Description

HeaderIcon indexes BUILTIN_ICONS with an arbitrary string on a plain object, so values like
'__proto__'/'toString' can resolve inherited properties and be rendered as a React component,
causing runtime errors or unexpected output.

Code

workspaces/global-header/plugins/global-header/src/components/HeaderIcon/HeaderIcon.tsx[R70-73]

+  const BuiltinIcon = BUILTIN_ICONS[icon];
+  if (BuiltinIcon) {
+    return (
+      <Box sx={{ display: 'flex', alignItems: 'center', ...layout }}>

Relevance

●●● Strong

Team often accepts defensive hasOwnProperty-style guards for untrusted keys; avoids runtime crashes
from prototype keys.

PR-#3536
PR-#2866

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR adds a new builtin-icon lookup via BUILTIN_ICONS[icon] and renders the resulting value as a
JSX component. The icon string can come straight from configuration (no validation), so
prototype-property keys can reach this lookup and break rendering.

workspaces/global-header/plugins/global-header/src/components/HeaderIcon/HeaderIcon.tsx[40-77]
workspaces/global-header/plugins/global-header/src/utils/readConfigComponents.tsx[30-55]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`BUILTIN_ICONS[icon]` performs a prototype-inheriting object lookup using an externally supplied string. Certain values (e.g. `__proto__`, `toString`) can resolve to inherited properties that are not valid icon components, which can crash rendering.

### Issue Context
`icon` can be configured via app config and flows into `HeaderIcon` without validation.

### Fix Focus Areas
- workspaces/global-header/plugins/global-header/src/components/HeaderIcon/HeaderIcon.tsx[40-77]

### Suggested fix
- Replace `BUILTIN_ICONS` with a `Map<string, ComponentType<SvgIconProps>>` and use `BUILTIN_ICONS.get(icon)`.
 -or-
- Create a null-prototype object (`const BUILTIN_ICONS = Object.create(null) as Record<string, ComponentType<SvgIconProps>>`) and guard with `Object.prototype.hasOwnProperty.call(BUILTIN_ICONS, icon)` before rendering.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. HeaderIconButton default size drift 🐞 Bug ⚙ Maintainability

Description

HeaderIconButton now defaults to size='medium', changing the rendered size for callers that omit the
size prop (including config/blueprint-created toolbar link buttons), despite the changeset declaring
only a patch change.

Code

workspaces/global-header/plugins/global-header/src/components/HeaderIconButton/HeaderIconButton.tsx[51]

+  size = 'medium',

Relevance

●●● Strong

Repo frequently enforces semver/changeset alignment for behavior changes; default prop drift likely
requires revert or bump.

PR-#3413
PR-#2936

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The PR changes the default size to 'medium' and that size is forwarded to both MUI IconButton and
HeaderIcon. Several code paths create HeaderIconButton without an explicit size, so their rendered
size will change; the changeset marks this as a patch release, increasing compatibility risk.

workspaces/global-header/plugins/global-header/src/components/HeaderIconButton/HeaderIconButton.tsx[45-76]
workspaces/global-header/plugins/global-header/src/defaults/toolbarExtensions.tsx[39-46]
workspaces/global-header/plugins/global-header/src/utils/readConfigComponents.tsx[36-52]
workspaces/global-header/.changeset/real-windows-remain.md[1-5]
workspaces/global-header/plugins/global-header/src/components/NotificationButton/NotificationButton.tsx[65-100]
workspaces/global-header/plugins/global-header/src/components/HeaderDropdownComponent/HeaderDropdownComponent.tsx[75-107]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Changing `HeaderIconButton`'s default `size` from `small` to `medium` is a behavior change that affects any consumer not explicitly passing `size` (including config-driven buttons). This is risky under a patch-level changeset.

### Issue Context
Multiple places construct `HeaderIconButton` without passing `size`, so they will change appearance automatically.

### Fix Focus Areas
- workspaces/global-header/plugins/global-header/src/components/HeaderIconButton/HeaderIconButton.tsx[45-76]
- workspaces/global-header/plugins/global-header/src/defaults/toolbarExtensions.tsx[39-46]
- workspaces/global-header/.changeset/real-windows-remain.md[1-5]

### Suggested fix
Pick one:
1) Restore `HeaderIconButton` default back to `small` and set `size="medium"` only at the specific call sites that need it (e.g. SelfServiceButton).
2) If the new default is intentional for all consumers, bump release semantics accordingly (e.g. minor) and document the change for downstream users.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context used

✅ Compliance rules (platform): 11 rules

✅ Cross-repo context

  Explored: repo: redhat-developer/rhdh (sha: 744d5784)

  Not relevant to this PR: redhat-developer/rhdh-chart

  Not relevant to this PR: redhat-developer/rhdh-operator

  Not relevant to this PR: redhat-developer/rhdh-local

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud