From c1991318aa7ee5d021f458db26cd2597279cc4ba Mon Sep 17 00:00:00 2001 From: Dusty Greif Date: Thu, 20 May 2021 14:03:14 -0700 Subject: [PATCH] Background styles for focused action list items (#1158) --- .changeset/orange-berries-protect.md | 5 ++ package.json | 2 +- src/ActionList/Item.tsx | 74 ++++++++++++++++--- src/ThemeProvider.tsx | 2 +- src/__tests__/ThemeProvider.tsx | 13 ++-- .../__snapshots__/CircleOcticon.tsx.snap | 2 +- src/__tests__/__snapshots__/Dialog.tsx.snap | 2 +- yarn.lock | 8 +- 8 files changed, 83 insertions(+), 25 deletions(-) create mode 100644 .changeset/orange-berries-protect.md diff --git a/.changeset/orange-berries-protect.md b/.changeset/orange-berries-protect.md new file mode 100644 index 00000000000..ea198d2ca20 --- /dev/null +++ b/.changeset/orange-berries-protect.md @@ -0,0 +1,5 @@ +--- +"@primer/components": patch +--- + +Add background styles for focused action list items, instead of using default `outline` diff --git a/package.json b/package.json index 334cd084266..9595664803b 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "author": "GitHub, Inc.", "license": "MIT", "dependencies": { - "@primer/octicons-react": "^11.3.0", + "@primer/octicons-react": "^13.0.0", "@primer/primitives": "0.0.0-202121782215", "@styled-system/css": "5.1.5", "@styled-system/props": "5.1.4", diff --git a/src/ActionList/Item.tsx b/src/ActionList/Item.tsx index 99868183779..eb5fd34b8e9 100644 --- a/src/ActionList/Item.tsx +++ b/src/ActionList/Item.tsx @@ -7,6 +7,37 @@ import {ItemInput} from './List' import styled from 'styled-components' import {StyledHeader} from './Header' import {StyledDivider} from './Divider' +import {useColorSchemeVar, useTheme} from '../ThemeProvider' + +/** + * These colors are not yet in our default theme. Need to remove this once they are added. + */ +const customItemThemes = { + default: { + hover: { + light: 'rgba(46, 77, 108, 0.06)', + dark: 'rgba(201, 206, 212, 0.12)', + dark_dimmed: 'rgba(201, 206, 212, 0.12)' + }, + focus: { + light: 'rgba(54, 77, 100, 0.16)', + dark: 'rgba(201, 206, 212, 0.24)', + dark_dimmed: 'rgba(201, 206, 212, 0.24)' + } + }, + danger: { + hover: { + light: 'rgba(234, 74, 90, 0.08)', + dark: 'rgba(248, 81, 73, 0.16)', + dark_dimmed: 'rgba(248, 81, 73, 0.16)' + }, + focus: { + light: 'rgba(234, 74, 90, 0.14)', + dark: 'rgba(248, 81, 73, 0.24)', + dark_dimmed: 'rgba(248, 81, 73, 0.24)' + } + } +} as const /** * Contract for props passed to the `Item` component. @@ -95,7 +126,6 @@ const getItemVariant = (variant = 'default', disabled?: boolean) => { color: get('colors.text.disabled'), iconColor: get('colors.text.disabled'), annotationColor: get('colors.text.disabled'), - hoverBackground: 'inherit', hoverCursor: 'default' } } @@ -106,15 +136,13 @@ const getItemVariant = (variant = 'default', disabled?: boolean) => { color: get('colors.text.danger'), iconColor: get('colors.icon.danger'), annotationColor: get('colors.text.disabled'), - hoverBackground: get('colors.bg.danger'), hoverCursor: 'pointer' } default: return { color: 'inherit', - iconColor: get('colors.text.disabled'), - annotationColor: get('colors.text.disabled'), - hoverBackground: get('colors.selectMenu.tapHighlight'), + iconColor: get('colors.text.secondary'), + annotationColor: get('colors.text.secondary'), hoverCursor: 'pointer' } } @@ -125,7 +153,13 @@ const StyledItemContent = styled.div` ` const StyledItem = styled.div< - {variant: ItemProps['variant']; showDivider: ItemProps['showDivider']; item?: ItemInput} & SxProp + { + variant: ItemProps['variant'] + showDivider: ItemProps['showDivider'] + item?: ItemInput + hoverBackground: string + focusBackground: string + } & SxProp >` /* 6px vertical padding + 20px line height = 32px total height * @@ -139,7 +173,7 @@ const StyledItem = styled.div< @media (hover: hover) and (pointer: fine) { :hover { - background: ${({variant, item}) => getItemVariant(variant, item?.disabled).hoverBackground}; + background: ${({hoverBackground}) => hoverBackground}; cursor: ${({variant, item}) => getItemVariant(variant, item?.disabled).hoverCursor}; } } @@ -159,6 +193,11 @@ const StyledItem = styled.div< } } + &:focus { + background: ${({focusBackground}) => focusBackground}; + outline: none; + } + ${sx} ` @@ -176,18 +215,21 @@ const BaseVisualContainer = styled.div<{variant?: ItemProps['variant']; disabled flex-shrink: 0; display: flex; flex-direction: column; + flex-shrink: 0; justify-content: center; margin-right: ${get('space.2')}; +` +const ColoredVisualContainer = styled(BaseVisualContainer)` svg { fill: ${({variant, disabled}) => getItemVariant(variant, disabled).iconColor}; font-size: ${get('fontSizes.0')}; } ` -const LeadingVisualContainer = styled(BaseVisualContainer)`` +const LeadingVisualContainer = styled(ColoredVisualContainer)`` -const TrailingVisualContainer = styled(BaseVisualContainer)` +const TrailingVisualContainer = styled(ColoredVisualContainer)` color: ${({variant, disabled}) => getItemVariant(variant, disabled).annotationColor}}; margin-left: auto; margin-right: 0; @@ -260,6 +302,12 @@ export function Item(itemProps: Partial & {item?: ItemInput}): JSX.El [onAction, disabled, itemProps, onClick] ) + const customItemTheme = customItemThemes[variant] + const hoverBackground = useColorSchemeVar(customItemTheme.hover, 'inherit') + const focusBackground = useColorSchemeVar(customItemTheme.focus, 'inherit') + + const {theme} = useTheme() + return ( & {item?: ItemInput}): JSX.El data-id={id} onKeyPress={keyPressHandler} onClick={clickHandler} + hoverBackground={disabled ? 'inherit' : hoverBackground} + focusBackground={disabled ? 'inherit' : focusBackground} > {!!selected === selected && ( - + {selectionVariant === 'multiple' ? ( <> {/* @@ -282,9 +332,9 @@ export function Item(itemProps: Partial & {item?: ItemInput}): JSX.El ) : ( - selected && + selected && )} - + )} {LeadingVisual && ( diff --git a/src/ThemeProvider.tsx b/src/ThemeProvider.tsx index a32ba5c96b4..238ebe82d42 100644 --- a/src/ThemeProvider.tsx +++ b/src/ThemeProvider.tsx @@ -90,7 +90,7 @@ export function useTheme() { return React.useContext(ThemeContext) } -export function useColorSchemeVar(values: Partial>, fallback?: string) { +export function useColorSchemeVar(values: Partial>, fallback: string) { const {colorScheme = ''} = useTheme() return values[colorScheme] ?? fallback } diff --git a/src/__tests__/ThemeProvider.tsx b/src/__tests__/ThemeProvider.tsx index 1ce77dd1ec5..cece1ea1b91 100644 --- a/src/__tests__/ThemeProvider.tsx +++ b/src/__tests__/ThemeProvider.tsx @@ -377,11 +377,14 @@ describe('useColorSchemeVar', () => { } function CustomBg() { - const customBg = useColorSchemeVar({ - light: 'red', - dark: 'blue', - dark_dimmed: 'green' - }) + const customBg = useColorSchemeVar( + { + light: 'red', + dark: 'blue', + dark_dimmed: 'green' + }, + 'inherit' + ) return Hello } diff --git a/src/__tests__/__snapshots__/CircleOcticon.tsx.snap b/src/__tests__/__snapshots__/CircleOcticon.tsx.snap index 8c22a56c6d8..8dae7f477b8 100644 --- a/src/__tests__/__snapshots__/CircleOcticon.tsx.snap +++ b/src/__tests__/__snapshots__/CircleOcticon.tsx.snap @@ -44,7 +44,7 @@ exports[`CircleOcticon renders consistently 1`] = ` >