From d505e43ffcd33af16b0b06992a555a1a6b11af9f Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 12 Apr 2022 12:14:28 +0200 Subject: [PATCH] core-components: fix highlighting of active sidebar items MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Signed-off-by: Patrik Oldsberg --- .changeset/strong-paws-float.md | 5 + .../src/layout/Sidebar/Items.tsx | 233 +++++++++--------- 2 files changed, 127 insertions(+), 111 deletions(-) create mode 100644 .changeset/strong-paws-float.md diff --git a/.changeset/strong-paws-float.md b/.changeset/strong-paws-float.md new file mode 100644 index 0000000000..3906185b79 --- /dev/null +++ b/.changeset/strong-paws-float.md @@ -0,0 +1,5 @@ +--- +'@backstage/core-components': patch +--- + +Fix highlighting of active sidebar items. diff --git a/packages/core-components/src/layout/Sidebar/Items.tsx b/packages/core-components/src/layout/Sidebar/Items.tsx index b3beed9147..4a0376946a 100644 --- a/packages/core-components/src/layout/Sidebar/Items.tsx +++ b/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( - 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( + 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((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; }) => { 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( theme => theme.breakpoints.down('md'),