[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 sidebar color returned 🐞 Bug ☼ Reliability

Description

pickNavigationOrRhdhGeneralColor returns fromNavigation!/fromRhdh! after checking only for
!== undefined, but theme merging explicitly allows null overrides, so this function can return
null despite its string return type. That null can flow into component CSS overrides (e.g.,
border/background values), producing invalid style values and type-contract violations.

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/undefined contract issues are commonly accepted; non-null assertion after only undefined-check
can leak null.

PR-#2724
PR-#2840

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The theme merge utility explicitly preserves null overrides, meaning palette color fields can be
null at runtime. In pickNavigationOrRhdhGeneralColor, the customization checks treat null as
“present” (!== undefined) and then return it via non-null assertions
(fromNavigation!/fromRhdh!), allowing null to escape from a function typed to return string.

workspaces/theme/plugins/theme/src/utils/navigationSidebarColors.ts[93-106]
workspaces/theme/plugins/theme/src/utils/mergeTheme.ts[41-68]
workspaces/theme/plugins/theme/src/utils/mergeTheme.ts[70-78]

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` even though the function is typed to return `string`, because theme merging allows `null` overrides and the code only checks for `!== undefined` before returning `fromNavigation!` / `fromRhdh!`.

### Issue Context
The merge utility explicitly preserves `null` (used to override defaults). Any palette field can therefore be `null` at runtime. The resolver should treat `null` as “unset” and fall back to the intended alternative.

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

### Suggested fix
- Treat `null` the same as `undefined` when determining whether a value is set/customized (e.g., use `value != null` checks).
- Remove non-null assertions for `fromNavigation`/`fromRhdh` or guard them so they can never be `null`.
- Optionally normalize inputs when building the `pick` object, e.g. `navigationColor: palette.navigation?.background ?? undefined` and `rhdhGeneralColor: general.sidebarBackgroundColor ?? undefined` (same for hoverBackground/selectedBackground).

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

---

2. Breaking required palette token 🐞 Bug ⚙ Maintainability

Description

pageInsetBackgroundColor is added as a required field to the exported RHDHThemePalette.general
API while the changeset indicates only a patch release, forcing downstream TypeScript consumers to
update even if they want the default behavior. This also contradicts usage in createComponents,
which already treats the token as optional via ?? fallback.

Code

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

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

Relevance

⭐ Low

Team previously added required palette tokens in patch releases (e.g., starredItemsColor) without
treating as breaking.

PR-#1340
PR-#2840

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The changeset bumps @red-hat-developer-hub/backstage-plugin-theme as a patch while introducing a
new required field in a public interface, which is a breaking TS change for consumers constructing
palette objects. The implementation uses `general.pageInsetBackgroundColor ??
general.appBarBackgroundColor`, indicating the token is intended to be optional/unsettable.

workspaces/theme/plugins/theme/src/types.ts[21-25]
workspaces/theme/plugins/theme/src/utils/createComponents.ts[746-753]
workspaces/theme/.changeset/nasty-bears-relate.md[1-5]
workspaces/theme/plugins/theme/report.api.md[56-67]

Agent prompt

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

### Issue description
A new exported palette token `pageInsetBackgroundColor` was introduced as **required** (`string`) on `RHDHThemePalette.general`, which is a breaking TypeScript API change. The changeset declares a **patch** release and the implementation already treats the token as optional via `??` fallback.

### Issue Context
Downstream packages may build objects typed as `RHDHThemePalette`/`RHDHThemePalette['general']`. Adding a required field breaks compilation immediately, even if they rely on defaults.

### Fix Focus Areas
- workspaces/theme/plugins/theme/src/types.ts[21-61]
- workspaces/theme/plugins/theme/report.api.md[56-75]
- workspaces/theme/plugins/theme/src/utils/createComponents.ts[746-753]

### Suggested fix
- Change `pageInsetBackgroundColor: string;` to `pageInsetBackgroundColor?: string;` in `types.ts`.
- Regenerate/update `report.api.md` so the public API matches.
- Keep defaults in `lightTheme.ts`/`darkTheme.ts` as-is so the merged theme still has a value by default.

ⓘ 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

Align navigation sidebar colors with user config and add page inset background token

🐞 Bug fix ✨ Enhancement

Walkthroughs

Description

• Add pageInsetBackgroundColor token for independent page inset styling
• Resolve navigation sidebar colors from merged palette.navigation and rhdh.general configs
• Fix sidebar background color application to prevent unintended main content area styling
• Update sidebar item selected/hover states to use resolved navigation colors

Diagram

flowchart LR
  A["User Config<br/>palette.navigation<br/>palette.rhdh.general"] -->|resolveNavigationSidebarColors| B["Navigation Sidebar<br/>Chrome Colors"]
  B -->|sidebarBackgroundColor| C["Sidebar Background"]
  B -->|sidebarItemInteractionBackgroundColor| D["Sidebar Item States"]
  B -->|navigationItemColor| E["Item Text Color"]
  B -->|navigationSelectedColor| F["Selected Item Color"]
  G["pageInsetBackgroundColor<br/>Token"] -->|defaults to appBarBackgroundColor| H["Page Inset Background"]

Loading

File Changes

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

Add pageInsetBackgroundColor type definition

• Add pageInsetBackgroundColor 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

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

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

---

View more (5)

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

New navigation sidebar color resolution utility

• New utility module to resolve navigation sidebar colors from merged config
• Implements priority logic: prefer customized RHDH values over navigation values
• Exports resolveNavigationSidebarColors() function and NavigationSidebarChrome type
• Compares current values against baseline defaults to detect user customizations

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

---

5. workspaces/theme/plugins/theme/src/utils/createComponents.ts 🐞 Bug fix +33/-13

Use resolved navigation colors in component overrides

• Import and use resolveNavigationSidebarColors() for sidebar styling
• Replace hardcoded general.sidebarBackgroundColor with resolved sidebarBackgroundColor
• Update BackstageSidebarItem selected/hover states with resolved colors
• Add CSS selectors for selected sidebar items and submenu items
• Change BackstageSidebarPage background to use pageInsetBackgroundColor with fallback
• Update MuiBottomNavigation and MuiBottomNavigationAction to use resolved colors

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

---

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

Add tests for navigation sidebar color resolution

• New test file with 4 test cases for resolveNavigationSidebarColors()
• Test baseline defaults matching
• Test priority when only palette.navigation.background differs
• Test priority when only rhdh.general.sidebarBackgroundColor differs
• Test priority when only palette.navigation.navItem.hoverBackground differs

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

---

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

Add changeset for navigation sidebar color alignment

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

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

---

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

Update API report with pageInsetBackgroundColor field

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

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

---