[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. Unreadable submenu defaults 🐞 Bug ≡ Correctness

Description

In the light theme, navigation.submenu.background remains dark (#222427) while
navigation.color/selectedColor are dark (#151515) and navigation.background is now light
(#f2f2f2). Any UI that uses navigation.* tokens for submenu rows will default to low-contrast
text on the dark submenu background.

Code

workspaces/theme/plugins/theme/src/lightTheme.ts[R29-33]

  navigation: {
-    background: '#222427',
+    background: '#f2f2f2',
    indicator: 'transparent',
    color: '#151515',
    selectedColor: '#151515',

Relevance

⭐⭐⭐ High

Light nav bg became #f2f2f2; submenu likely still dark, creating clear contrast mismatch.

PR-#2840

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new light theme defaults set a dark submenu background but keep dark navigation text colors,
which is a direct low-contrast combination. This mismatch is introduced by changing only
navigation.background while leaving navigation.submenu.background unchanged.

workspaces/theme/plugins/theme/src/lightTheme.ts[29-40]

Agent prompt

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

### Issue description
`lightThemeOverrides` now uses a light `navigation.background` but keeps a dark `navigation.submenu.background` while also keeping dark `navigation.color` and `navigation.selectedColor`. This likely makes submenu entries unreadable by default.

### Issue Context
This PR’s intent is to align sidebar/navigation colors and selected states. The light theme’s submenu background should be consistent with the rest of the sidebar background (or the text colors must change accordingly).

### Fix Focus Areas
- workspaces/theme/plugins/theme/src/lightTheme.ts[29-40]

### What to change
- Update `lightThemeOverrides.navigation.submenu.background` to match the new light sidebar/navigation background (e.g. `#f2f2f2`), OR
- If the submenu background is intended to stay dark, update `navigation.color`/`selectedColor` (and possibly submenu-specific text colors if available) so contrast is acceptable.
- Consider adding a small unit/snapshot test asserting light theme navigation and submenu defaults are consistent/legible.

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

---

2. Null color returned 🐞 Bug ≡ Correctness

Description

pickNavigationOrRhdhGeneralColor treats null as “customized” (it only checks !== undefined)
and can directly return null via return fromNavigation!, even though the function is typed to
return string. This can propagate into createComponents CSS (e.g., `borderRight: 0.5rem solid
null), breaking sidebar styling for configs that intentionally use null` overrides.

Code

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

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

Relevance

⭐⭐⭐ High

Function can return null despite string type; could propagate into CSS and break styling.

PR-#2840

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

Theme merging explicitly allows null to override defaults; when palette.navigation.background
(or navigation.navItem.hoverBackground) is set to null, the picker’s `fromNavigation !==
undefined check evaluates true and it can return fromNavigation! directly, bypassing later ||`
fallbacks. The returned value is then interpolated into CSS without sanitization.

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

Agent prompt

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

### Issue description
`resolveNavigationSidebarColors` / `pickNavigationOrRhdhGeneralColor` can return `null` at runtime (despite returning `string`) because theme merging allows `null` overrides and the picker only checks for `undefined`. This leads to invalid CSS values being emitted by `createComponents`.

### Issue Context
`deepMergeObjects` intentionally allows `null` to override defaults, so users can “unset” values via config. The color-resolution logic must therefore treat `null` as “missing” (or otherwise handle it explicitly).

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

### What to change
- In `pickNavigationOrRhdhGeneralColor`, normalize inputs before comparisons/returns, e.g.:
 - `const fromNavigation = pick.navigationColor ?? undefined;`
 - `const fromRhdh = pick.rhdhGeneralColor ?? undefined;`
 - Optionally, only accept `typeof value === 'string'`.
- Avoid non-null assertions (`fromNavigation!` / `fromRhdh!`) when the value may be `null`; return a safe fallback instead.
- Add/extend unit tests to cover `null` config overrides for `navigation.background` and `navigation.navItem.hoverBackground` and assert returned values are strings (or empty string) rather than `null`.

ⓘ 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

🐞 Bug fix ✨ Enhancement

Walkthroughs

Description

• Add pageInsetBackgroundColor token to control page inset background independently
• Implement intelligent color resolution between palette.navigation and rhdh.general configs
• Update sidebar styling to use resolved navigation colors for items and submenu states
• Fix sidebar background color application to page inset only, not main content

Diagram

flowchart LR
  A["User Theme Config"] -->|navigation or rhdh.general| B["resolveNavigationSidebarColors"]
  B -->|sidebarBackgroundColor| C["BackstageSidebar & MuiDrawer"]
  B -->|sidebarItemInteractionBackgroundColor| D["BackstageSidebarItem & MuiBottomNavigationAction"]
  B -->|navigationItemColor & navigationSelectedColor| E["Selected/Hover States"]
  F["pageInsetBackgroundColor token"] -->|defaults to appBarBackgroundColor| G["BackstageSidebarPage root"]
  C --> H["Styled Navigation Sidebar"]
  D --> H
  E --> H
  G --> H

Loading

File Changes

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

Add pageInsetBackgroundColor type definition

• Add pageInsetBackgroundColor: string field to RHDHThemePalette.general interface

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

---

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

Update light theme navigation and page inset colors

• Change navigation.background from #222427 to #f2f2f2 for light theme
• Add pageInsetBackgroundColor: '#f2f2f2' to rhdh.general defaults

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

---

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

Add page inset background color to dark theme

• Add pageInsetBackgroundColor: '#151515' to rhdh.general defaults

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

---

View more (5)

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

Implement navigation sidebar color resolution logic

• Implement resolveNavigationSidebarColors() function to intelligently pick between
 palette.navigation and rhdh.general colors
• Compare current values against baseline defaults to determine user customization intent
• Prefer RHDH colors when both navigation and RHDH values are customized
• Return resolved colors for sidebar background, item interaction, and text styling

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

---

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

Add navigation sidebar colors resolution tests

• Add comprehensive test suite for resolveNavigationSidebarColors() function
• Test baseline defaults matching
• Test individual customization of palette.navigation.background,
 rhdh.general.sidebarBackgroundColor, and palette.navigation.navItem.hoverBackground

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

---

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

Update component overrides to use resolved navigation colors

• Import and use resolveNavigationSidebarColors() for sidebar component styling
• Replace hardcoded general.sidebarBackgroundColor references with resolved
 sidebarBackgroundColor
• Replace general.sidebarItemSelectedBackgroundColor with resolved
 sidebarItemInteractionBackgroundColor
• Add styling for selected BackstageSidebarItem and BackstageSidebarSubmenuItem states with
 resolved colors
• Update BackstageSidebarPage to use pageInsetBackgroundColor (defaults to
 appBarBackgroundColor)
• Apply resolved navigationItemColor and navigationSelectedColor to bottom navigation components

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

---

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

Update API documentation with new token

• Add pageInsetBackgroundColor: string to public API documentation for RHDHThemePalette.general
• Reformat interface indentation for consistency

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

---

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

Add changeset for sidebar color alignment

• Document alignment of navigation sidebar with merged palette.navigation and rhdh.general
 colors
• Document new pageInsetBackgroundColor token with fallback behavior
• Note that main content area remains on mainSectionBackgroundColor

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

---