core-components: fix highlighting of active sidebar items

Co-authored-by: Fredrik Adelöw <freben@gmail.com>
Co-authored-by: blam <ben@blam.sh>
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>

.changeset/strong-paws-float.md

@@ -0,0 +1,5 @@
+---
+'@backstage/core-components': patch
+---
+
+Fix highlighting of active sidebar items.

packages/core-components/src/layout/Sidebar/Items.tsx

@@ -37,6 +37,7 @@ import React, {
   KeyboardEventHandler,
   ReactNode,
   useContext,
+  useMemo,
   useState,
 } from 'react';
 import {
@@ -83,122 +84,132 @@ export type SidebarItemClassKey =
   | 'arrows'
   | 'selected';
 
-const useStyles = makeStyles<BackstageTheme, { sidebarConfig: SidebarConfig }>(
-  theme => ({
-    root: {
-      color: theme.palette.navigation.color,
-      display: 'flex',
-      flexFlow: 'row nowrap',
-      alignItems: 'center',
-      height: 48,
-      cursor: 'pointer',
-    },
-    buttonItem: {
-      background: 'none',
-      border: 'none',
-      width: '100%',
-      margin: 0,
-      padding: 0,
-      textAlign: 'inherit',
-      font: 'inherit',
-    },
-    closed: props => ({
-      width: props.sidebarConfig.drawerWidthClosed,
-      justifyContent: 'center',
-    }),
-    open: props => ({
-      [theme.breakpoints.up('sm')]: {
-        width: props.sidebarConfig.drawerWidthOpen,
+const makeSidebarStyles = (sidebarConfig: SidebarConfig) =>
+  makeStyles<BackstageTheme>(
+    theme => ({
+      root: {
+        color: theme.palette.navigation.color,
+        display: 'flex',
+        flexFlow: 'row nowrap',
+        alignItems: 'center',
+        height: 48,
+        cursor: 'pointer',
       },
-    }),
-    highlightable: {
-      '&:hover': {
+      buttonItem: {
+        background: 'none',
+        border: 'none',
+        width: '100%',
+        margin: 0,
+        padding: 0,
+        textAlign: 'inherit',
+        font: 'inherit',
+      },
+      closed: {
+        width: sidebarConfig.drawerWidthClosed,
+        justifyContent: 'center',
+      },
+      open: {
+        [theme.breakpoints.up('sm')]: {
+          width: sidebarConfig.drawerWidthOpen,
+        },
+      },
+      highlightable: {
+        '&:hover': {
+          background:
+            theme.palette.navigation.navItem?.hoverBackground ?? '#404040',
+        },
+      },
+      highlighted: {
         background:
           theme.palette.navigation.navItem?.hoverBackground ?? '#404040',
       },
-    },
-    highlighted: {
-      background:
-        theme.palette.navigation.navItem?.hoverBackground ?? '#404040',
-    },
-    label: {
-      // XXX (@koroeskohr): I can't seem to achieve the desired font-weight from the designs
-      fontWeight: 'bold',
-      whiteSpace: 'nowrap',
-      lineHeight: 'auto',
-      flex: '3 1 auto',
-      width: '110px',
-      overflow: 'hidden',
-      'text-overflow': 'ellipsis',
-    },
-    iconContainer: props => ({
-      boxSizing: 'border-box',
-      height: '100%',
-      width: props.sidebarConfig.iconContainerWidth,
-      marginRight: -theme.spacing(2),
-      display: 'flex',
-      alignItems: 'center',
-      justifyContent: 'center',
-    }),
-    searchRoot: {
-      marginBottom: 12,
-    },
-    searchField: {
-      color: '#b5b5b5',
-      fontWeight: 'bold',
-      fontSize: theme.typography.fontSize,
-    },
-    searchFieldHTMLInput: {
-      padding: theme.spacing(2, 0, 2),
-    },
-    searchContainer: props => ({
-      width:
-        props.sidebarConfig.drawerWidthOpen -
-        props.sidebarConfig.iconContainerWidth,
-    }),
-    secondaryAction: {
-      width: theme.spacing(6),
-      textAlign: 'center',
-      marginRight: theme.spacing(1),
-    },
-    closedItemIcon: {
-      width: '100%',
-      justifyContent: 'center',
-    },
-    submenuArrow: {
-      display: 'flex',
-    },
-    expandButton: {
-      background: 'none',
-      border: 'none',
-      color: theme.palette.navigation.color,
-      width: '100%',
-      cursor: 'pointer',
-      position: 'relative',
-      height: 48,
-    },
-    arrows: {
-      position: 'absolute',
-      right: 10,
-    },
-    selected: props => ({
-      '&$root': {
-        borderLeft: `solid ${props.sidebarConfig.selectedIndicatorWidth}px ${theme.palette.navigation.indicator}`,
-        color: theme.palette.navigation.selectedColor,
+      label: {
+        // XXX (@koroeskohr): I can't seem to achieve the desired font-weight from the designs
+        fontWeight: 'bold',
+        whiteSpace: 'nowrap',
+        lineHeight: 'auto',
+        flex: '3 1 auto',
+        width: '110px',
+        overflow: 'hidden',
+        'text-overflow': 'ellipsis',
       },
-      '&$closed': {
-        width: props.sidebarConfig.drawerWidthClosed,
+      iconContainer: {
+        boxSizing: 'border-box',
+        height: '100%',
+        width: sidebarConfig.iconContainerWidth,
+        marginRight: -theme.spacing(2),
+        display: 'flex',
+        alignItems: 'center',
+        justifyContent: 'center',
       },
-      '& $closedItemIcon': {
-        paddingRight: props.sidebarConfig.selectedIndicatorWidth,
+      searchRoot: {
+        marginBottom: 12,
       },
-      '& $iconContainer': {
-        marginLeft: -props.sidebarConfig.selectedIndicatorWidth,
+      searchField: {
+        color: '#b5b5b5',
+        fontWeight: 'bold',
+        fontSize: theme.typography.fontSize,
+      },
+      searchFieldHTMLInput: {
+        padding: theme.spacing(2, 0, 2),
+      },
+      searchContainer: {
+        width: sidebarConfig.drawerWidthOpen - sidebarConfig.iconContainerWidth,
+      },
+      secondaryAction: {
+        width: theme.spacing(6),
+        textAlign: 'center',
+        marginRight: theme.spacing(1),
+      },
+      closedItemIcon: {
+        width: '100%',
+        justifyContent: 'center',
+      },
+      submenuArrow: {
+        display: 'flex',
+      },
+      expandButton: {
+        background: 'none',
+        border: 'none',
+        color: theme.palette.navigation.color,
+        width: '100%',
+        cursor: 'pointer',
+        position: 'relative',
+        height: 48,
+      },
+      arrows: {
+        position: 'absolute',
+        right: 10,
+      },
+      selected: {
+        '&$root': {
+          borderLeft: `solid ${sidebarConfig.selectedIndicatorWidth}px ${theme.palette.navigation.indicator}`,
+          color: theme.palette.navigation.selectedColor,
+        },
+        '&$closed': {
+          width: sidebarConfig.drawerWidthClosed,
+        },
+        '& $closedItemIcon': {
+          paddingRight: sidebarConfig.selectedIndicatorWidth,
+        },
+        '& $iconContainer': {
+          marginLeft: -sidebarConfig.selectedIndicatorWidth,
+        },
       },
     }),
-  }),
-  { name: 'BackstageSidebarItem' },
-);
+    { name: 'BackstageSidebarItem' },
+  );
+
+// This is a workaround for this issue https://github.com/mui/material-ui/issues/15511
+// The styling of the `selected` elements doesn't work as expected when using a prop callback.
+// Don't use this pattern unless needed
+function useMemoStyles(sidebarConfig: SidebarConfig) {
+  const useStyles = useMemo(
+    () => makeSidebarStyles(sidebarConfig),
+    [sidebarConfig],
+  );
+  return useStyles();
+}
 
 /**
  * Evaluates the routes of the SubmenuItems & nested DropdownItems.
@@ -352,7 +363,7 @@ const SidebarItemBase = forwardRef<any, SidebarItemProps>((props, ref) => {
     ...navLinkProps
   } = props;
   const { sidebarConfig } = useContext(SidebarConfigContext);
-  const classes = useStyles({ sidebarConfig });
+  const classes = useMemoStyles(sidebarConfig);
   // XXX (@koroeskohr): unsure this is optimal. But I just really didn't want to have the item component
   // depend on the current location, and at least have it being optionally forced to selected.
   // Still waiting on a Q answered to fine tune the implementation
@@ -426,7 +437,7 @@ const SidebarItemWithSubmenu = ({
   children: React.ReactElement<SidebarSubmenuProps>;
 }) => {
   const { sidebarConfig } = useContext(SidebarConfigContext);
-  const classes = useStyles({ sidebarConfig });
+  const classes = useMemoStyles(sidebarConfig);
   const [isHoveredOn, setIsHoveredOn] = useState(false);
   const location = useLocation();
   const isActive = useLocationMatch(children, location);
@@ -517,7 +528,7 @@ type SidebarSearchFieldProps = {
 export function SidebarSearchField(props: SidebarSearchFieldProps) {
   const { sidebarConfig } = useContext(SidebarConfigContext);
   const [input, setInput] = useState('');
-  const classes = useStyles({ sidebarConfig });
+  const classes = useMemoStyles(sidebarConfig);
   const Icon = props.icon ? props.icon : SearchIcon;
 
   const search = () => {
@@ -646,7 +657,7 @@ export const SidebarScrollWrapper = styled('div')(({ theme }) => {
  */
 export const SidebarExpandButton = () => {
   const { sidebarConfig } = useContext(SidebarConfigContext);
-  const classes = useStyles({ sidebarConfig });
+  const classes = useMemoStyles(sidebarConfig);
   const { isOpen, setOpen } = useContext(SidebarContext);
   const isSmallScreen = useMediaQuery<BackstageTheme>(
     theme => theme.breakpoints.down('md'),