Background styles for focused action list items (#1158)

.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`

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",

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<ItemProps> & {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 (
     <StyledItem
       tabIndex={disabled ? undefined : -1}
@@ -270,9 +318,11 @@ export function Item(itemProps: Partial<ItemProps> & {item?: ItemInput}): JSX.El
       data-id={id}
       onKeyPress={keyPressHandler}
       onClick={clickHandler}
+      hoverBackground={disabled ? 'inherit' : hoverBackground}
+      focusBackground={disabled ? 'inherit' : focusBackground}
     >
       {!!selected === selected && (
-        <LeadingVisualContainer>
+        <BaseVisualContainer>
           {selectionVariant === 'multiple' ? (
             <>
               {/*
@@ -282,9 +332,9 @@ export function Item(itemProps: Partial<ItemProps> & {item?: ItemInput}): JSX.El
               <input type="checkbox" checked={selected} aria-label={text} readOnly aria-readonly="false" />
             </>
           ) : (
-            selected && <CheckIcon />
+            selected && <CheckIcon fill={theme?.colors.text.primary} />
           )}
-        </LeadingVisualContainer>
+        </BaseVisualContainer>
       )}
       {LeadingVisual && (
         <LeadingVisualContainer variant={variant} disabled={disabled}>

src/ThemeProvider.tsx

@@ -90,7 +90,7 @@ export function useTheme() {
   return React.useContext(ThemeContext)
 }
 
-export function useColorSchemeVar(values: Partial<Record<string, string>>, fallback?: string) {
+export function useColorSchemeVar(values: Partial<Record<string, string>>, fallback: string) {
   const {colorScheme = ''} = useTheme()
   return values[colorScheme] ?? fallback
 }

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 <Text bg={customBg}>Hello</Text>
     }

src/__tests__/__snapshots__/CircleOcticon.tsx.snap

@@ -44,7 +44,7 @@ exports[`CircleOcticon renders consistently 1`] = `
   >
     <svg
       aria-hidden="true"
-      className="octicon"
+      className="octicon octicon-check"
       dangerouslySetInnerHTML={
         Object {
           "__html": "<path fill-rule=\\"evenodd\\" d=\\"M21.03 5.72a.75.75 0 010 1.06l-11.5 11.5a.75.75 0 01-1.072-.012l-5.5-5.75a.75.75 0 111.084-1.036l4.97 5.195L19.97 5.72a.75.75 0 011.06 0z\\"></path>",

src/__tests__/__snapshots__/Dialog.tsx.snap

@@ -160,7 +160,7 @@ Array [
     >
       <svg
         aria-hidden="true"
-        className="octicon"
+        className="octicon octicon-x"
         dangerouslySetInnerHTML={
           Object {
             "__html": "<path fill-rule=\\"evenodd\\" d=\\"M3.72 3.72a.75.75 0 011.06 0L8 6.94l3.22-3.22a.75.75 0 111.06 1.06L9.06 8l3.22 3.22a.75.75 0 11-1.06 1.06L8 9.06l-3.22 3.22a.75.75 0 01-1.06-1.06L6.94 8 3.72 4.78a.75.75 0 010-1.06z\\"></path>",

yarn.lock

@@ -1833,10 +1833,10 @@
   resolved "https://registry.yarnpkg.com/@popperjs/core/-/core-2.7.2.tgz#00644825ce478065101954bdb2f7f2d062365b7f"
   integrity sha512-3LC1/M2ZFcmDrSkD1byT/hZzoPehZkDucbDShLnZ+/Gwkr6CAxiQ2kWy1UtJeLGADe58hTWkx22YEHqjPGUKzw==
 
-"@primer/octicons-react@^11.3.0":
-  version "11.3.0"
-  resolved "https://registry.yarnpkg.com/@primer/octicons-react/-/octicons-react-11.3.0.tgz#794641d95ff5749a9438a2e0c201956b2a377b60"
-  integrity sha512-4sVhkrBKuj3h+PFw69yOyO/l3nQB/mm95V+Kz7LRSlIrbZr6hZarZD5Ft4ewdONPROkIHQM/6KSK90+OAimxsQ==
+"@primer/octicons-react@^13.0.0":
+  version "13.0.0"
+  resolved "https://registry.yarnpkg.com/@primer/octicons-react/-/octicons-react-13.0.0.tgz#a7f2288fd9cf9cabc1e75553a0dd9f00d74b68c1"
+  integrity sha512-j5XppNRCvgaMZLPsVvvmp6GSh7P5pq6PUbsfLNBWi2Kz3KYDeoGDWbPr5MjoxFOGUn6Hjnt6qjHPRxahd11vLQ==
 
 "@primer/primitives@0.0.0-202121782215":
   version "0.0.0-202121782215"