From 06878bf8108793261dc3976f2ee4a0b59f68f165 Mon Sep 17 00:00:00 2001 From: hiba-aldalaty Date: Fri, 26 Nov 2021 11:23:46 +0000 Subject: [PATCH] Revert "Move styling from themes/palette and make optional" This reverts commit 04e1b87a894d0c4a7c28e13e89a0cba75159586d. Signed-off-by: hiba-aldalaty --- packages/core-components/api-report.md | 2 - .../src/layout/Sidebar/Items.tsx | 212 +++++++++--------- .../src/layout/Sidebar/SidebarSubmenu.tsx | 8 +- packages/theme/api-report.md | 6 + packages/theme/src/themes.ts | 12 + packages/theme/src/types.ts | 6 + 6 files changed, 130 insertions(+), 116 deletions(-) diff --git a/packages/core-components/api-report.md b/packages/core-components/api-report.md index 0c2f009092..f11cb6f3cc 100644 --- a/packages/core-components/api-report.md +++ b/packages/core-components/api-report.md @@ -1990,7 +1990,6 @@ export type SidebarSubItemDropDownItem = { // @public export const SidebarSubmenu: ({ title, - backgroundColor, children, }: PropsWithChildren) => JSX.Element; @@ -2010,7 +2009,6 @@ export type SidebarSubmenuItemProps = { // @public export type SidebarSubmenuProps = { title?: string; - backgroundColor?: string; children: ReactNode; }; diff --git a/packages/core-components/src/layout/Sidebar/Items.tsx b/packages/core-components/src/layout/Sidebar/Items.tsx index f4e2124ab6..544d7010fe 100644 --- a/packages/core-components/src/layout/Sidebar/Items.tsx +++ b/packages/core-components/src/layout/Sidebar/Items.tsx @@ -61,114 +61,112 @@ export type SidebarItemClassKey = | 'secondaryAction' | 'selected'; -const useStyles = (props: { hoverBackgroundColor?: string }) => - makeStyles( - theme => { - const { - selectedIndicatorWidth, - drawerWidthClosed, - drawerWidthOpen, - iconContainerWidth, - } = sidebarConfig; - return { - root: { - color: theme.palette.navigation.color, - display: 'flex', - flexFlow: 'row nowrap', - alignItems: 'center', - height: 48, - cursor: 'pointer', +const useStyles = makeStyles( + theme => { + const { + selectedIndicatorWidth, + drawerWidthClosed, + drawerWidthOpen, + iconContainerWidth, + } = sidebarConfig; + return { + root: { + color: theme.palette.navigation.color, + display: 'flex', + flexFlow: 'row nowrap', + alignItems: 'center', + height: 48, + cursor: 'pointer', + }, + buttonItem: { + background: 'none', + border: 'none', + width: 'auto', + margin: 0, + padding: 0, + textAlign: 'inherit', + font: 'inherit', + }, + closed: { + width: drawerWidthClosed, + justifyContent: 'center', + }, + open: { + width: drawerWidthOpen, + }, + highlightable: { + '&:hover': { + background: theme.palette.navigation.navItem.hoverBackground, }, - buttonItem: { - background: 'none', - border: 'none', - width: 'auto', - margin: 0, - padding: 0, - textAlign: 'inherit', - font: 'inherit', + }, + highlighted: { + background: theme.palette.navigation.navItem.hoverBackground, + }, + 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: { + boxSizing: 'border-box', + height: '100%', + width: 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 ${theme.spacing(2)}`, + }, + searchContainer: { + width: drawerWidthOpen - iconContainerWidth, + }, + secondaryAction: { + width: theme.spacing(6), + textAlign: 'center', + marginRight: theme.spacing(1), + }, + closedItemIcon: { + width: '100%', + justifyContent: 'center', + }, + submenuArrow: { + position: 'absolute', + right: 0, + }, + selected: { + '&$root': { + borderLeft: `solid ${selectedIndicatorWidth}px ${theme.palette.navigation.indicator}`, + color: theme.palette.navigation.selectedColor, }, - closed: { + '&$closed': { width: drawerWidthClosed, - justifyContent: 'center', }, - open: { - width: drawerWidthOpen, + '& $closedItemIcon': { + paddingRight: selectedIndicatorWidth, }, - highlightable: { - '&:hover': { - // Ideally this would be added to the pallette but would cause a breaking change - background: props.hoverBackgroundColor || '#404040', - }, + '& $iconContainer': { + marginLeft: -selectedIndicatorWidth, }, - highlighted: { - background: props.hoverBackgroundColor || '#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: { - boxSizing: 'border-box', - height: '100%', - width: 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 ${theme.spacing(2)}`, - }, - searchContainer: { - width: drawerWidthOpen - iconContainerWidth, - }, - secondaryAction: { - width: theme.spacing(6), - textAlign: 'center', - marginRight: theme.spacing(1), - }, - closedItemIcon: { - width: '100%', - justifyContent: 'center', - }, - submenuArrow: { - position: 'absolute', - right: 0, - }, - selected: { - '&$root': { - borderLeft: `solid ${selectedIndicatorWidth}px ${theme.palette.navigation.indicator}`, - color: theme.palette.navigation.selectedColor, - }, - '&$closed': { - width: drawerWidthClosed, - }, - '& $closedItemIcon': { - paddingRight: selectedIndicatorWidth, - }, - '& $iconContainer': { - marginLeft: -selectedIndicatorWidth, - }, - }, - }; - }, - { name: 'BackstageSidebarItem' }, - ); + }, + }; + }, + { name: 'BackstageSidebarItem' }, +); function isItemWithSubmenuActive(submenu: ReactNode, locationPathname: string) { // Item is active if any of submenu items have active paths @@ -199,11 +197,10 @@ function isItemWithSubmenuActive(submenu: ReactNode, locationPathname: string) { const ItemWithSubmenu = ({ text, hasNotifications = false, - hoverBackgroundColor, icon: Icon, children, }: PropsWithChildren) => { - const classes = useStyles({ hoverBackgroundColor })(); + const classes = useStyles(); const [isHoveredOn, setIsHoveredOn] = useState(false); const { pathname: locationPathname } = useLocation(); const isActive = isItemWithSubmenuActive(children, locationPathname); @@ -280,7 +277,6 @@ type SidebarItemBaseProps = { text?: string; hasNotifications?: boolean; disableHighlight?: boolean; - hoverBackgroundColor?: string; className?: string; }; @@ -370,13 +366,12 @@ export const SidebarItem = forwardRef((props, ref) => { text, hasNotifications = false, disableHighlight = false, - hoverBackgroundColor, onClick, children, className, ...navLinkProps } = props; - const classes = useStyles({ hoverBackgroundColor })(); + const classes = useStyles(); // 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 @@ -454,7 +449,6 @@ export const SidebarItem = forwardRef((props, ref) => { text={text} icon={Icon} hasNotifications={hasNotifications} - hoverBackgroundColor={hoverBackgroundColor} > {submenu} @@ -491,7 +485,7 @@ type SidebarSearchFieldProps = { export function SidebarSearchField(props: SidebarSearchFieldProps) { const [input, setInput] = useState(''); - const classes = useStyles({})(); + const classes = useStyles(); const Icon = props.icon ? props.icon : SearchIcon; const search = () => { diff --git a/packages/core-components/src/layout/Sidebar/SidebarSubmenu.tsx b/packages/core-components/src/layout/Sidebar/SidebarSubmenu.tsx index a53bf2328d..9b8b9ca145 100644 --- a/packages/core-components/src/layout/Sidebar/SidebarSubmenu.tsx +++ b/packages/core-components/src/layout/Sidebar/SidebarSubmenu.tsx @@ -25,7 +25,7 @@ import { } from './config'; import { BackstageTheme } from '@backstage/theme'; -const useStyles = (props: { left: number; backgroundColor?: string }) => +const useStyles = (props: { left: number }) => makeStyles(theme => ({ root: { zIndex: 1000, @@ -42,7 +42,7 @@ const useStyles = (props: { left: number; backgroundColor?: string }) => top: 0, bottom: 0, padding: 0, - background: props.backgroundColor || '#404040', + background: theme.palette.navigation.submenu.background, overflowX: 'hidden', msOverflowStyle: 'none', scrollbarWidth: 'none', @@ -75,7 +75,6 @@ const useStyles = (props: { left: number; backgroundColor?: string }) => */ export type SidebarSubmenuProps = { title?: string; - backgroundColor?: string; children: ReactNode; }; @@ -86,14 +85,13 @@ export type SidebarSubmenuProps = { */ export const SidebarSubmenu = ({ title, - backgroundColor, children, }: PropsWithChildren) => { const { isOpen } = useContext(SidebarContext); const left = isOpen ? sidebarConfig.drawerWidthOpen : sidebarConfig.drawerWidthClosed; - const props = { left, backgroundColor }; + const props = { left: left }; const classes = useStyles(props)(); const { isHoveredOn } = useContext(ItemWithSubmenuContext); diff --git a/packages/theme/api-report.md b/packages/theme/api-report.md index 2dc6ab7b55..4331278e60 100644 --- a/packages/theme/api-report.md +++ b/packages/theme/api-report.md @@ -41,6 +41,12 @@ export type BackstagePaletteAdditions = { indicator: string; color: string; selectedColor: string; + navItem: { + hoverBackground: string; + }; + submenu: { + background: string; + }; }; tabbar: { indicator: string; diff --git a/packages/theme/src/themes.ts b/packages/theme/src/themes.ts index 7e99ce8e8a..b048c70fec 100644 --- a/packages/theme/src/themes.ts +++ b/packages/theme/src/themes.ts @@ -76,6 +76,12 @@ export const lightTheme = createTheme({ indicator: '#9BF0E1', color: '#b5b5b5', selectedColor: '#FFF', + navItem: { + hoverBackground: '#404040', + }, + submenu: { + background: '#404040', + }, }, pinSidebarButton: { icon: '#181818', @@ -151,6 +157,12 @@ export const darkTheme = createTheme({ indicator: '#9BF0E1', color: '#b5b5b5', selectedColor: '#FFF', + navItem: { + hoverBackground: '#404040', + }, + submenu: { + background: '#404040', + }, }, pinSidebarButton: { icon: '#404040', diff --git a/packages/theme/src/types.ts b/packages/theme/src/types.ts index e1acba3a01..44d9165417 100644 --- a/packages/theme/src/types.ts +++ b/packages/theme/src/types.ts @@ -53,6 +53,12 @@ export type BackstagePaletteAdditions = { indicator: string; color: string; selectedColor: string; + navItem: { + hoverBackground: string; + }; + submenu: { + background: string; + }; }; tabbar: { indicator: string;