[release-1.9] fix(theme): pick the navigation or rhdh colors based on user's config

[karthikjeeyar]

Release-1.9 PR

This is a cherry-pick of 482ba12

---

https://redhat.atlassian.net/browse/RHDHBUGS-2981

The PR containss following chagns:

Align the navigation sidebar with merged palette.navigation and rhdh.general colors, including submenu rows and selected/active BackstageSidebarItem states.
Removes a theme override that set sidebarBackgroundColor in a way that applied the sidebar background color to main page inset.
Introduce a new token pageInsetBackgroundColor, so the page inset background can be controlled by config (defaults to appBarBackgroundColor).

Default RHDH without any customizations:

---

Customized sidebar background color states:

  branding:
   theme:
    light:
        mode: "light"
        palette:
          navigation:
            background: "#222427"
            color: "#fffff"
            selectedColor: "#ffffff"
            submenu:
              background: "#222427"
            navItem:
              hoverBackground: "#333333"

✔️ 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-qodo-merge]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Null override breaks sidebar colors 🐞 Bug ≡ Correctness

Description

pickNavigationOrRhdhGeneralColor treats null as a customized value (it only checks `!==
undefined) and can return null via return fromNavigation! / return fromRhdh!`, which then gets
interpolated into CSS in createComponents. Since your merge logic explicitly allows null to
override defaults, a user setting a nav color to null can unintentionally break sidebar styling.

Code

workspaces/theme/plugins/theme/src/utils/navigationSidebarColors.ts[R93-105]

+  const navigationCustomized =
+    fromNavigation !== undefined && fromNavigation !== defaultNav;
+  const rhdhCustomized = fromRhdh !== undefined && fromRhdh !== defaultRhdh;
+
+  if (navigationCustomized && !rhdhCustomized) {
+    return fromNavigation!;
+  }
+  if (rhdhCustomized && !navigationCustomized) {
+    return fromRhdh!;
+  }
+  if (navigationCustomized && rhdhCustomized && fromNavigation !== fromRhdh) {
+    return fromRhdh ?? fromNavigation ?? '';
+  }

Relevance

⭐⭐⭐ High

Null-vs-undefined propagation into styling is a real bug class; team often accepts fixes using
nullish coalescing.

PR-#2840
PR-#2724

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

mergeUnifiedThemeOptions preserves null overrides. The new picker’s customization detection
ignores null (it only guards against undefined) and uses non-null assertions to return the value
directly, allowing null to propagate. createComponents then uses the resolved color values
inside template strings for CSS, so a null return yields invalid/empty CSS values for sidebar
backgrounds/borders.

workspaces/theme/plugins/theme/src/utils/mergeTheme.ts[41-68]
workspaces/theme/plugins/theme/src/utils/navigationSidebarColors.ts[93-106]
workspaces/theme/plugins/theme/src/utils/createComponents.ts[620-636]

Agent prompt

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

### Issue description
The navigation-vs-RHDH picker can return `null` when a user explicitly sets a palette field to `null`. This is possible because the theme merge supports `null` overrides, but the picker’s logic only checks for `undefined` and uses non-null assertions (`!`). The resolved value is then interpolated into CSS, causing broken styles.

### Issue Context
- `mergeUnifiedThemeOptions` preserves `null` (customValue === null returns null).
- `pickNavigationOrRhdhGeneralColor` uses checks like `fromNavigation !== undefined` and returns `fromNavigation!`, which can be `null`.
- `createComponents` consumes these values for sidebar CSS.

### Fix Focus Areas
- workspaces/theme/plugins/theme/src/utils/navigationSidebarColors.ts[66-107]
- workspaces/theme/plugins/theme/src/utils/mergeTheme.ts[41-68]
- workspaces/theme/plugins/theme/src/utils/createComponents.ts[620-636]

### Expected fix
- Normalize inputs in `pickNavigationOrRhdhGeneralColor` (and/or before calling it) so `null` is treated as “unset”, e.g. `const fromNavigation = pick.navigationColor ?? undefined;` and same for `fromRhdh`.
- Update `navigationCustomized` / `rhdhCustomized` checks to use `!= null` (or explicit `!== undefined && !== null`).
- Remove the unsafe non-null assertions or ensure they cannot be reached with `null` values.
- (Optional) Add a unit test that sets `palette.navigation.background = null` and verifies it falls back to the RHDH/general/default value instead of returning null/empty.

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

---

2. Breaking required palette field 🐞 Bug ⚙ Maintainability

Description

RHDHThemePalette.general.pageInsetBackgroundColor was added as a required field to a publicly
exported type, which will break downstream TypeScript consumers compiling against this package
unless they update their theme objects. This contradicts the stated "falls back … when unset"
behavior and is risky as a patch release change.

Code

workspaces/theme/plugins/theme/src/types.ts[R21-25]

export interface RHDHThemePalette {
  general: {
    pageInset: string;
+    pageInsetBackgroundColor: string;

Relevance

⭐ Low

Team merged similar new required theme tokens; tends to reject BC concerns for new required fields.

PR-#2840
PR-#1340
PR-#2375

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The type is exported publicly, so adding a required property is source-breaking for any consumer
constructing RHDHThemePalette. The changeset and component code both imply the value may be unset
and should fall back, which aligns with making the property optional rather than required.

workspaces/theme/plugins/theme/src/types.ts[21-27]
workspaces/theme/plugins/theme/src/index.ts[20-31]
workspaces/theme/.changeset/nasty-bears-relate.md[1-5]
workspaces/theme/plugins/theme/src/utils/createComponents.ts[746-753]
workspaces/theme/plugins/theme/report.api.md[64-92]

Agent prompt

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

### Issue description
`RHDHThemePalette.general.pageInsetBackgroundColor` is currently required in the exported public types and API report, which is a breaking TypeScript change for a patch release and contradicts the intended "fallback when unset" behavior.

### Issue Context
- `RHDHThemePalette` is exported from the package.
- The changeset and `createComponents` logic indicate the value can be unset and should fall back.

### Fix Focus Areas
- workspaces/theme/plugins/theme/src/types.ts[21-61]
- workspaces/theme/plugins/theme/report.api.md[64-92]
- workspaces/theme/plugins/theme/src/utils/createComponents.ts[748-753]

### Expected fix
- Change the type to `pageInsetBackgroundColor?: string` (and update API report accordingly).
- Ensure runtime fallback behavior remains the same when the value is `undefined`/missing (and optionally also handle `null` if supported by config).

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

---

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

[rhdh-qodo-merge]

Review Summary by Qodo

Fix navigation sidebar colors based on user configuration

🐞 Bug fix ✨ Enhancement

Walkthroughs

Description

• Add pageInsetBackgroundColor to theme palette for independent page inset styling
• Implement intelligent color resolution between navigation and rhdh.general configs
• Fix sidebar item selected/hover states to use navigation palette colors
• Update light theme navigation background from dark to light color

Diagram

flowchart LR
  A["User Theme Config"] -->|"navigation or rhdh.general"| B["resolveNavigationSidebarColors"]
  B -->|"intelligent selection"| C["NavigationSidebarChrome"]
  C -->|"applied to"| D["Component Overrides"]
  D -->|"styled"| E["Sidebar & Navigation UI"]
  F["pageInsetBackgroundColor"] -->|"new field"| G["Page Inset Styling"]

Loading

File Changes

1. workspaces/theme/plugins/theme/src/types.ts ✨ Enhancement +1/-0

Add pageInsetBackgroundColor to theme palette

• Add pageInsetBackgroundColor field to RHDHThemePalette.general interface

workspaces/theme/plugins/theme/src/types.ts

---

2. workspaces/theme/plugins/theme/src/darkTheme.ts ✨ Enhancement +1/-0

Set dark theme page inset background color

• Add pageInsetBackgroundColor: '#151515' to dark theme defaults

workspaces/theme/plugins/theme/src/darkTheme.ts

---

3. workspaces/theme/plugins/theme/src/lightTheme.ts 🐞 Bug fix +2/-1

Fix light theme navigation and page inset colors

• Change navigation background from #222427 to #f2f2f2 (light color)
• Add pageInsetBackgroundColor: '#f2f2f2' to light theme defaults

workspaces/theme/plugins/theme/src/lightTheme.ts

---

View more (5)

4. workspaces/theme/plugins/theme/src/utils/navigationSidebarColors.ts ✨ Enhancement +157/-0

New utility for resolving navigation sidebar colors

• Create new utility module to intelligently resolve sidebar colors
• Implement logic to pick between navigation and rhdh.general palette configs
• Prefer user-customized values while maintaining backward compatibility
• Export NavigationSidebarChrome type with resolved color values

workspaces/theme/plugins/theme/src/utils/navigationSidebarColors.ts

---

5. workspaces/theme/plugins/theme/src/utils/navigationSidebarChrome.test.ts 🧪 Tests +92/-0

Add tests for navigation sidebar color resolution

• Add comprehensive test suite for resolveNavigationSidebarColors function
• Test baseline defaults matching behavior
• Test individual palette customization scenarios
• Verify correct color selection when only navigation or rhdh values differ

workspaces/theme/plugins/theme/src/utils/navigationSidebarChrome.test.ts

---

6. workspaces/theme/plugins/theme/src/utils/createComponents.ts ✨ Enhancement +33/-13

Apply resolved navigation colors to component overrides

• Import new resolveNavigationSidebarColors utility function
• Replace hardcoded general.sidebarBackgroundColor with resolved colors
• Add CSS selectors for selected/active BackstageSidebarItem states
• Apply navigation colors to submenu items and bottom navigation components
• Update BackstageSidebarPage to use pageInsetBackgroundColor with fallback

workspaces/theme/plugins/theme/src/utils/createComponents.ts

---

7. workspaces/theme/.changeset/nasty-bears-relate.md 📝 Documentation +5/-0

Add changeset for theme color resolution fix

• Document patch release for theme plugin
• Describe alignment of navigation sidebar with merged palette colors
• Note new pageInsetBackgroundColor configuration option

workspaces/theme/.changeset/nasty-bears-relate.md

---

8. workspaces/theme/plugins/theme/report.api.md 📝 Documentation +46/-45

Update API documentation with new color field

• Add pageInsetBackgroundColor to public API documentation
• Update formatting/indentation in RHDHThemePalette interface docs

workspaces/theme/plugins/theme/report.api.md

---

[karthikjeeyar]

/approve
/lgtm

[openshift-ci]

@karthikjeeyar: you cannot LGTM your own PR.

Details

In response to this:

/approve
/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.