Skip to content

Conversation

lokanandaprabhu
Copy link
Collaborator

Description

  • Added theme hooks for app bar background scheme and sidebar color configuration
  • Created CustomSidebarItem component with configurable selected background colors
  • Added support for light/dark mode sidebar background color customization

Which issue(s) does this PR fix

Videos:

-----------DEFAULT COLORS(no customization)------

RHDH-navigation-color-default

----------CUSTOMIZED (color only used for testing)------

RHDH-navigation-color-custom

PR acceptance criteria

Please make sure that the following steps are complete:

  • GitHub Actions are completed and successful
  • Unit Tests are updated and passing
  • E2E Tests are updated and passing
  • Documentation is updated if necessary (requirement for new features)
  • Add a screenshot if the change is UX/UI related

Copy link

openshift-ci bot commented Oct 15, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign nickboldt for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅

RHDHBUGS-2046 - PR Code Verified

Compliant requirements:

  • Make the selected sidebar menu item's background color configurable via app-config.
  • Support separate colors for light and dark modes.
  • Provide sensible default fallback colors ensuring adequate contrast.
  • Update documentation to explain the new configuration.
  • Ensure UI reflects the new settings across built-in and custom sidebar items.

Requires further human verification:

  • Visual verification in both light and dark themes to ensure contrast and correct application on all sidebar items.
  • Accessibility contrast checks (WCAG) between text, selected background, and navigation indicator colors.
  • E2E/UX check that dynamic theme switching updates styles without flicker or stale styles.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🔒 No security concerns identified
⚡ Recommended focus areas for review

CSS Targeting Robustness

The global style targets classes via substring match for built-in Backstage items. This may be brittle against class name changes and could have unintended matches. Validate selectors and consider a more stable targeting approach if available.

const GlobalSidebarStyles: React.FC<{ selectedBackgroundColor: string }> = ({
  selectedBackgroundColor,
}) => {
  React.useEffect(() => {
    const style = document.createElement('style');
    style.textContent = `
      /* Target built-in Backstage sidebar items like Settings and Search */
      [class*="BackstageSidebarItem-selected"] {
        background-color: ${selectedBackgroundColor} !important;
      }
    `;
    document.head.appendChild(style);

    return () => {
      document.head.removeChild(style);
    };
  }, [selectedBackgroundColor]);
Potential Icon Prop Crash

The SidebarItem receives icon={icon!}. If icon is undefined for some entries, this non-null assertion could cause runtime issues. Confirm all usages supply an icon or provide a safe default.

  <SidebarItem icon={icon!} to={to} text={text} style={style} />
</StyledSidebarItemWrapper>
Theme Switch Cleanup

When switching themes, verify that the injected global style updates correctly and no stale style tags remain. Ensure the effect dependency and cleanup handle rapid toggles.

/**
 * Gets the sidebar selected background color based on Backstage's current theme.
 * Falls back to default colors if not configured.
 */
export const useSidebarSelectedBackgroundColor = () => {
  const configApi = useApi(configApiRef);
  const theme = useTheme();

  // Detect theme using MUI's theme.palette.mode
  const colorScheme = theme.palette?.mode === 'dark' ? 'dark' : 'light';

  const lightColor = configApi.getOptional<string>(
    'app.branding.theme.light.sidebarSelectedBackgroundColor',
  );
  const darkColor = configApi.getOptional<string>(
    'app.branding.theme.dark.sidebarSelectedBackgroundColor',
  );

  // Default fallback colors that provide good contrast
  const defaultLightColor = '#ffffff'; // White background for light mode
  const defaultDarkColor = '#424242'; // Dark gray for dark mode

  return colorScheme === 'light'
    ? lightColor || defaultLightColor
    : darkColor || defaultDarkColor;
};
📄 References
  1. No matching references available

Copy link

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Type

Enhancement


Description

  • Added configurable sidebar selected item background color for light/dark themes

  • Created CustomSidebarItem component with dynamic background color styling

  • Added useSidebarSelectedBackgroundColor hook for theme-based color retrieval

  • Updated E2E test constants to include sidebar background color validation


File Walkthrough

Relevant files
Enhancement
CustomSidebarItem.tsx
New component for customizable sidebar item styling           

packages/app/src/components/Root/CustomSidebarItem.tsx

  • Created new CustomSidebarItem component with styled wrapper for custom
    background colors
  • Implemented GlobalSidebarStyles component to apply styles to built-in
    Backstage sidebar items
  • Added support for configurable selected item background color via
    theme hook
+70/-0   
useThemedConfig.ts
Hook for sidebar background color configuration                   

packages/app/src/hooks/useThemedConfig.ts

  • Added useSidebarSelectedBackgroundColor hook to retrieve theme-based
    sidebar colors
  • Implemented fallback to default colors (white for light, dark gray for
    dark mode)
  • Reads configuration from
    app.branding.theme.[light|dark].sidebarSelectedBackgroundColor
+27/-0   
Root.tsx
Integration of custom sidebar item component                         

packages/app/src/components/Root/Root.tsx

  • Replaced SidebarItem with CustomSidebarItem in menu rendering
  • Imported and integrated new CustomSidebarItem component
+3/-2     
Documentation
customization.md
Documentation for sidebar color customization                       

docs/customization.md

  • Added documentation section for customizing sidebar selected item
    background color
  • Included YAML configuration examples for light and dark modes
  • Added notes about contrast requirements with navigation indicator
    color
+18/-0   
Tests
theme-constants.ts
E2E test constants for sidebar colors                                       

e2e-tests/playwright/data/theme-constants.ts

  • Added sidebarSelectedBackgroundColor property to ThemeInfo type
  • Updated all theme configurations with sidebar background color values
+5/-0     

Copy link

You are nearing your monthly Qodo Merge usage quota. For more information, please visit here.

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Refactor styling to use theme overrides

Instead of injecting global CSS from a React component, use MUI's theme
overrides to customize the SidebarItem styles. This is a more idiomatic and
maintainable approach for styling within a Backstage application.

Examples:

packages/app/src/components/Root/CustomSidebarItem.tsx [20-39]
const GlobalSidebarStyles: React.FC<{ selectedBackgroundColor: string }> = ({
  selectedBackgroundColor,
}) => {
  React.useEffect(() => {
    const style = document.createElement('style');
    style.textContent = `
      /* Target built-in Backstage sidebar items like Settings and Search */
      [class*="BackstageSidebarItem-selected"] {
        background-color: ${selectedBackgroundColor} !important;
      }

 ... (clipped 10 lines)
packages/app/src/components/Root/CustomSidebarItem.tsx [52-70]
export const CustomSidebarItem: React.FC<CustomSidebarItemProps> = ({
  icon,
  to,
  text,
  style,
}) => {
  const selectedBackgroundColor = useSidebarSelectedBackgroundColor();

  return (
    <>

 ... (clipped 9 lines)

Solution Walkthrough:

Before:

// packages/app/src/components/Root/CustomSidebarItem.tsx

// Component to inject a <style> tag into the document head
const GlobalSidebarStyles = ({ selectedBackgroundColor }) => {
  useEffect(() => {
    const style = document.createElement('style');
    style.textContent = `
      [class*="BackstageSidebarItem-selected"] {
        background-color: ${selectedBackgroundColor} !important;
      }
    `;
    document.head.appendChild(style);
    // ... cleanup on unmount
  }, [selectedBackgroundColor]);
  return null;
};

// Custom component that wraps the original SidebarItem
export const CustomSidebarItem = (props) => {
  const selectedBackgroundColor = useSidebarSelectedBackgroundColor();
  return (
    <>
      <GlobalSidebarStyles selectedBackgroundColor={selectedBackgroundColor} />
      <StyledSidebarItemWrapper selectedBackgroundColor={selectedBackgroundColor}>
        <SidebarItem {...props} />
      </StyledSidebarItemWrapper>
    </>
  );
};

After:

// In a theme creation file (e.g., packages/app/src/theme.ts)

import { sidebarConfig, BackstageTheme } from '@backstage/theme';

export const createCustomTheme = (options): BackstageTheme => {
  // ... base theme creation
  return {
    ...baseTheme,
    components: {
      ...baseTheme.components,
      // Override Backstage's SidebarItem styles
      BackstageSidebarItem: {
        styleOverrides: {
          root: ({ theme }) => ({
            '&[aria-current=page]': {
              backgroundColor: theme.palette.mode === 'light'
                ? options.light.sidebarSelectedBackgroundColor
                : options.dark.sidebarSelectedBackgroundColor,
            },
          }),
        },
      },
    },
  };
};

// Root.tsx would revert to using the standard <SidebarItem />
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that injecting a global <style> tag via a React component is a poor pattern, and proposes a significantly more robust and idiomatic solution using MUI theme overrides, which is the standard practice in Backstage.

High
Possible issue
Avoid injecting duplicate global styles

Refactor the GlobalSidebarStyles component to use MUI's GlobalStyles and move it
higher in the component tree to prevent injecting duplicate style tags into the
DOM for each sidebar item.

packages/app/src/components/Root/CustomSidebarItem.tsx [19-70]

+import { GlobalStyles } from '@mui/material';
+
 // Global styles for built-in Backstage sidebar items (Settings, Search)
-const GlobalSidebarStyles: React.FC<{ selectedBackgroundColor: string }> = ({
-  selectedBackgroundColor,
-}) => {
-  React.useEffect(() => {
-    const style = document.createElement('style');
-    style.textContent = `
-      /* Target built-in Backstage sidebar items like Settings and Search */
-      [class*="BackstageSidebarItem-selected"] {
-        background-color: ${selectedBackgroundColor} !important;
-      }
-    `;
-    document.head.appendChild(style);
+export const GlobalSidebarStyles: React.FC = () => {
+  const selectedBackgroundColor = useSidebarSelectedBackgroundColor();
 
-    return () => {
-      document.head.removeChild(style);
-    };
-  }, [selectedBackgroundColor]);
-
-  return null;
+  return (
+    <GlobalStyles
+      styles={{
+        '[class*="BackstageSidebarItem-selected"]': {
+          backgroundColor: `${selectedBackgroundColor} !important`,
+        },
+      }}
+    />
+  );
 };
 
 interface CustomSidebarItemProps {
   icon?: React.ComponentType<{}>;
   to: string;
   text: string;
   style?: React.CSSProperties;
 }
 
 /**
  * Custom SidebarItem component that uses the configurable sidebar selected background color
  * from app-config.yaml. Falls back to default Backstage styling if not configured.
  */
 export const CustomSidebarItem: React.FC<CustomSidebarItemProps> = ({
   icon,
   to,
   text,
   style,
 }) => {
   const selectedBackgroundColor = useSidebarSelectedBackgroundColor();
 
   return (
-    <>
-      <GlobalSidebarStyles selectedBackgroundColor={selectedBackgroundColor} />
-      <StyledSidebarItemWrapper
-        selectedBackgroundColor={selectedBackgroundColor}
-      >
-        <SidebarItem icon={icon!} to={to} text={text} style={style} />
-      </StyledSidebarItemWrapper>
-    </>
+    <StyledSidebarItemWrapper
+      selectedBackgroundColor={selectedBackgroundColor}
+    >
+      <SidebarItem icon={icon!} to={to} text={text} style={style} />
+    </StyledSidebarItemWrapper>
   );
 };
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that the current implementation will inject multiple duplicate <style> tags into the DOM, and proposes a more efficient and idiomatic solution using MUI's GlobalStyles component.

Medium
  • More

Copy link
Contributor

@lokanandaprabhu
Copy link
Collaborator Author

/retest

1 similar comment
@lokanandaprabhu
Copy link
Collaborator Author

/retest

Copy link

openshift-ci bot commented Oct 16, 2025

@lokanandaprabhu: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-ocp-helm c909aa6 link true /test e2e-ocp-helm

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant