Finish updating global focus styles (#2050)

* updates focus styles that do not match the Primer CSS implementation * updates ThemeSwitcher to render ActionMenu instead of DropdownMenu * explicitly sets outline-offset in global focus styles instead of adding a new style that 'beats' the default 2px * update snapshots * fixes cross-browser issues for global focus styles * fixes unintentional laggy focus style update on SubNav * updates focus styles for UnderlineNav * DRY button focus styles * updates snapshots * adds changeset * fixes type error * revert themePreval snapshot update
primer · May 5, 2022 · 0f9edca · 0f9edca
1 parent 1c2eeb9
commit 0f9edca
Show file tree

Hide file tree

Showing 29 changed files with 643 additions and 322 deletions.
diff --git a/.changeset/silent-falcons-exist.md b/.changeset/silent-falcons-exist.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+Finishes updating components with the global focus styles defined in Primer CSS ([this PR](https://github.com/primer/css/pull/1744))
diff --git a/docs/content/SubNav.md b/docs/content/SubNav.md
@@ -39,7 +39,7 @@ This ensures that the NavLink gets `activeClassName='selected'`
     <SubNav.Link href="#support">Support</SubNav.Link>
   </SubNav.Links>
 
-  <TextInput type="search" icon={SearchIcon} sx={{width: 320}} />
+  <TextInput type="search" leadingVisual={SearchIcon} sx={{width: 320}} />
 </SubNav>
 ```
 
@@ -64,7 +64,7 @@ This ensures that the NavLink gets `activeClassName='selected'`
         </ActionList>
       </ActionMenu.Overlay>
     </ActionMenu>
-    <TextInput type="search" icon={SearchIcon} width={320} />
+    <TextInput type="search" leadingVisual={SearchIcon} width={320} />
   </FilteredSearch>
   <SubNav.Links>
     <SubNav.Link href="#home" selected>

diff --git a/docs/src/@primer/gatsby-theme-doctocat/components/live-preview-wrapper.js b/docs/src/@primer/gatsby-theme-doctocat/components/live-preview-wrapper.js
@@ -1,24 +1,33 @@
-import {BaseStyles, Box, ThemeProvider, useTheme} from '@primer/react'
-import {DropdownMenu, DropdownButton} from '@primer/react/deprecated'
 import React from 'react'
+import {ActionMenu, ActionList, BaseStyles, Box, ThemeProvider, useTheme} from '@primer/react'
 
 function ThemeSwitcher() {
   const {theme, dayScheme, setDayScheme} = useTheme()
-  const items = Object.keys(theme.colorSchemes).map(scheme => ({text: scheme.replace(/_/g, ' '), key: scheme}))
+  const items = Object.keys(theme.colorSchemes).map(scheme => ({name: scheme.replace(/_/g, ' '), key: scheme}))
   const selectedItem = React.useMemo(() => items.find(item => item.key === dayScheme), [items, dayScheme])
+  const itemsKeys = items.map(item => item.key)
+  const [selectedIndex, setSelectedIndex] = React.useState(itemsKeys.indexOf(dayScheme))
+
   return (
-    <DropdownMenu
-      renderAnchor={({children, ...anchorProps}) => (
-        <DropdownButton {...anchorProps} sx={{variant: 'small'}}>
-          {children}
-        </DropdownButton>
-      )}
-      items={items}
-      selectedItem={selectedItem}
-      onChange={item => {
-        setDayScheme(item.key)
-      }}
-    />
+    <ActionMenu>
+      <ActionMenu.Button aria-label="Select field type">{selectedItem?.name}</ActionMenu.Button>
+      <ActionMenu.Overlay width="medium">
+        <ActionList selectionVariant="single">
+          {items.map((type, index) => (
+            <ActionList.Item
+              key={index}
+              selected={index === selectedIndex}
+              onSelect={() => {
+                setSelectedIndex(index)
+                setDayScheme(items[index].key)
+              }}
+            >
+              {type.name}
+            </ActionList.Item>
+          ))}
+        </ActionList>
+      </ActionMenu.Overlay>
+    </ActionMenu>
   )
 }
 

diff --git a/src/Button/styles.ts b/src/Button/styles.ts
@@ -2,17 +2,6 @@ import {VariantType} from './types'
 import {Theme} from '../ThemeProvider'
 
 export const TEXT_ROW_HEIGHT = '20px' // custom value off the scale
-const focusOutlineStyles = {
-  outline: '2px solid',
-  outlineColor: 'accent.fg',
-  outlineOffset: '-2px'
-}
-const fallbackFocus = {
-  ...focusOutlineStyles,
-  ':not(:focus-visible)': {
-    outline: 'solid 1px transparent'
-  }
-}
 
 export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme) => {
   const style = {
@@ -23,11 +12,6 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme
       '&:hover:not([disabled])': {
         backgroundColor: 'btn.hoverBg'
       },
-      // focus must come before :active so that the active box shadow overrides
-      '&:focus:not([disabled])': {
-        ...fallbackFocus
-      },
-      '&:focus-visible:not([disabled])': focusOutlineStyles,
       '&:active:not([disabled])': {
         backgroundColor: 'btn.activeBg',
         borderColor: 'btn.activeBorder'
@@ -52,13 +36,10 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme
         color: 'btn.primary.hoverText',
         backgroundColor: 'btn.primary.hoverBg'
       },
-      // focus must come before :active so that the active box shadow overrides
       '&:focus:not([disabled])': {
-        boxShadow: 'inset 0 0 0 3px',
-        ...fallbackFocus
+        boxShadow: 'inset 0 0 0 3px'
       },
       '&:focus-visible:not([disabled])': {
-        ...focusOutlineStyles,
         boxShadow: 'inset 0 0 0 3px'
       },
       '&:active:not([disabled])': {
@@ -95,11 +76,6 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme
           color: 'btn.danger.hoverText'
         }
       },
-      // focus must come before :active so that the active box shadow overrides
-      '&:focus:not([disabled])': {
-        ...fallbackFocus
-      },
-      '&:focus-visible:not([disabled])': focusOutlineStyles,
       '&:active:not([disabled])': {
         color: 'btn.danger.selectedText',
         backgroundColor: 'btn.danger.selectedBg',
@@ -134,11 +110,6 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme
       '&:hover:not([disabled])': {
         backgroundColor: 'btn.hoverBg'
       },
-      // focus must come before :active so that the active box shadow overrides
-      '&:focus:not([disabled])': {
-        ...fallbackFocus
-      },
-      '&:focus-visible:not([disabled])': focusOutlineStyles,
       '&:active:not([disabled])': {
         backgroundColor: 'btn.selectedBg'
       },
@@ -168,11 +139,6 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme
           color: 'inherit'
         }
       },
-      // focus must come before :active so that the active box shadow overrides
-      '&:focus:not([disabled])': {
-        ...fallbackFocus
-      },
-      '&:focus-visible:not([disabled])': focusOutlineStyles,
       '&:active:not([disabled])': {
         color: 'btn.outline.selectedText',
         backgroundColor: 'btn.outline.selectedBg',

diff --git a/src/Button/types.ts b/src/Button/types.ts
@@ -2,8 +2,12 @@ import React, {HTMLAttributes, ComponentPropsWithRef} from 'react'
 import styled from 'styled-components'
 import {IconProps} from '@primer/octicons-react'
 import sx, {SxProp} from '../sx'
+import getGlobalFocusStyles from '../_getGlobalFocusStyles'
 
-export const StyledButton = styled.button<SxProp>(sx)
+export const StyledButton = styled.button<SxProp>`
+  ${getGlobalFocusStyles('-2px')};
+  ${sx};
+`
 
 export type VariantType = 'default' | 'primary' | 'invisible' | 'danger' | 'outline'
 

diff --git a/src/Checkbox.tsx b/src/Checkbox.tsx
@@ -4,6 +4,7 @@ import React, {ChangeEventHandler, InputHTMLAttributes, ReactElement, useContext
 import sx, {SxProp} from './sx'
 import {FormValidationStatus} from './utils/types/FormValidationStatus'
 import {CheckboxGroupContext} from './CheckboxGroup'
+import getGlobalFocusStyles from './_getGlobalFocusStyles'
 
 export type CheckboxProps = {
   /**
@@ -38,6 +39,7 @@ const StyledCheckbox = styled.input`
   cursor: pointer;
 
   ${props => props.disabled && `cursor: not-allowed;`}
+  ${getGlobalFocusStyles(0)};
 
   ${sx}
 `

diff --git a/src/Pagination/Pagination.tsx b/src/Pagination/Pagination.tsx
@@ -3,6 +3,7 @@ import styled from 'styled-components'
 import Box from '../Box'
 import {get} from '../constants'
 import sx, {SxProp} from '../sx'
+import getGlobalFocusStyles from '../_getGlobalFocusStyles'
 import {buildComponentData, buildPaginationModel} from './model'
 
 const Page = styled.a`
@@ -37,6 +38,8 @@ const Page = styled.a`
     transition-duration: 0.1s;
   }
 
+  ${getGlobalFocusStyles(0)};
+
   &:active {
     border-color: ${get('colors.border.muted')};
   }

diff --git a/src/Radio.tsx b/src/Radio.tsx
@@ -3,6 +3,7 @@ import React, {ChangeEventHandler, InputHTMLAttributes, ReactElement, useContext
 import sx, {SxProp} from './sx'
 import {FormValidationStatus} from './utils/types/FormValidationStatus'
 import {RadioGroupContext} from './RadioGroup'
+import getGlobalFocusStyles from './_getGlobalFocusStyles'
 
 export type RadioProps = {
   /**
@@ -41,6 +42,7 @@ const StyledRadio = styled.input`
   cursor: pointer;
 
   ${props => props.disabled && `cursor: not-allowed;`}
+  ${getGlobalFocusStyles(0)};
 
   ${sx}
 `

diff --git a/src/SubNav.tsx b/src/SubNav.tsx
@@ -96,7 +96,7 @@ const SubNavLink = styled.a.attrs<StyledSubNavLinkProps>(props => ({
   &:focus {
     text-decoration: none;
     background-color: ${get('colors.canvas.subtle')};
-    transition: 0.2s ease;
+    transition: background-color 0.2s ease;
 
     .SubNav-octicon {
       color: ${get('colors.fg.muted')};

diff --git a/src/TabNav.tsx b/src/TabNav.tsx
@@ -5,6 +5,7 @@ import styled from 'styled-components'
 import {get} from './constants'
 import sx, {SxProp} from './sx'
 import {ComponentProps} from './utils/types'
+import getGlobalFocusStyles from './_getGlobalFocusStyles'
 
 const ITEM_CLASS = 'TabNav-item'
 const SELECTED_CLASS = 'selected'
@@ -49,6 +50,8 @@ const TabNavLink = styled.a.attrs<StyledTabNavLinkProps>(props => ({
   border: 1px solid transparent;
   border-bottom: 0;
 
+  ${getGlobalFocusStyles('-6px')};
+
   &:hover,
   &:focus {
     color: ${get('colors.fg.default')};

diff --git a/src/UnderlineNav.tsx b/src/UnderlineNav.tsx
@@ -5,6 +5,7 @@ import styled from 'styled-components'
 import {get} from './constants'
 import sx, {SxProp} from './sx'
 import {ComponentProps} from './utils/types'
+import getGlobalFocusStyles from './_getGlobalFocusStyles'
 
 const ITEM_CLASS = 'PRC-UnderlineNav-item'
 const SELECTED_CLASS = 'PRC-selected'
@@ -86,7 +87,7 @@ const UnderlineNavLink = styled.a.attrs<StyledUnderlineNavLinkProps>(props => ({
     color: ${get('colors.fg.default')};
     text-decoration: none;
     border-bottom-color: ${get('colors.neutral.muted')};
-    transition: 0.2s ease;
+    transition: border-bottom-color 0.2s ease;
 
     .PRC-UnderlineNav-octicon {
       color: ${get('colors.fg.muted')};
@@ -102,6 +103,8 @@ const UnderlineNavLink = styled.a.attrs<StyledUnderlineNavLinkProps>(props => ({
     }
   }
 
+  ${getGlobalFocusStyles('-8px')};
+
   ${sx};
 `
 

diff --git a/src/_TextInputWrapper.tsx b/src/_TextInputWrapper.tsx
@@ -65,20 +65,22 @@ export type StyledWrapperProps = {
 const textInputBasePadding = '12px'
 export const textInputHorizPadding = textInputBasePadding
 
-// TODO: figure out how to type a themed CSS function (e.g.: css`color: blue;`)
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-const renderFocusStyles = (hasTrailingAction: boolean, isInputFocused: boolean, focusStyles: any) => {
+const renderFocusStyles = (hasTrailingAction: boolean, isInputFocused: boolean) => {
   if (hasTrailingAction) {
     return (
       isInputFocused &&
       css`
-        ${focusStyles}
+        border-color: ${get('colors.accent.fg')};
+        outline: none;
+        box-shadow: inset 0 0 0 1px ${get('colors.accent.fg')};
       `
     )
   }
   return css`
     &:focus-within {
-      ${focusStyles}
+      border-color: ${get('colors.accent.fg')};
+      outline: none;
+      box-shadow: inset 0 0 0 1px ${get('colors.accent.fg')};
     }
   `
 }
@@ -110,15 +112,7 @@ export const TextInputBaseWrapper = styled.span<StyledBaseWrapperProps>`
     color: ${get('colors.fg.subtle')};
   }
 
-  ${props =>
-    renderFocusStyles(
-      Boolean(props.hasTrailingAction),
-      Boolean(props.isInputFocused),
-      css`
-        border-color: ${get('colors.accent.emphasis')};
-        box-shadow: ${get('shadows.primer.shadow.focus')};
-      `
-    )}
+  ${props => renderFocusStyles(Boolean(props.hasTrailingAction), Boolean(props.isInputFocused))}
 
   > textarea {
     padding: ${textInputBasePadding};
@@ -154,29 +148,14 @@ export const TextInputBaseWrapper = styled.span<StyledBaseWrapperProps>`
     props.validationStatus === 'error' &&
     css`
       border-color: ${get('colors.danger.emphasis')};
-      ${renderFocusStyles(
-        Boolean(props.hasTrailingAction),
-        Boolean(props.isInputFocused),
-        css`
-          border-color: ${get('colors.danger.emphasis')};
-          box-shadow: ${get('shadows.btn.danger.focusShadow')};
-        `
-      )}
+      ${renderFocusStyles(Boolean(props.hasTrailingAction), Boolean(props.isInputFocused))}
     `}
 
 
   ${props =>
     props.validationStatus === 'success' &&
     css`
       border-color: ${get('colors.success.emphasis')};
-      ${renderFocusStyles(
-        Boolean(props.hasTrailingAction),
-        Boolean(props.isInputFocused),
-        css`
-          border-color: ${get('colors.success.emphasis')};
-          box-shadow: 0 0 0 3px ${get('colors.success.muted')};
-        `
-      )}
     `}
 
   ${props =>
@@ -230,14 +209,7 @@ const TextInputWrapper = styled(TextInputBaseWrapper)<StyledWrapperProps>`
     props.validationStatus === 'warning' &&
     css`
       border-color: ${get('colors.attention.emphasis')};
-      ${renderFocusStyles(
-        Boolean(props.hasTrailingAction),
-        Boolean(props.isInputFocused),
-        css`
-          border-color: ${get('colors.attention.emphasis')};
-          box-shadow: 0 0 0 3px ${get('colors.attention.muted')};
-        `
-      )}
+      ${renderFocusStyles(Boolean(props.hasTrailingAction), Boolean(props.isInputFocused))}
     `}
 
   ${sx};

diff --git a/src/__tests__/Pagination/__snapshots__/Pagination.test.tsx.snap b/src/__tests__/Pagination/__snapshots__/Pagination.test.tsx.snap
@@ -43,6 +43,22 @@ exports[`Pagination renders consistently 1`] = `
   transition-duration: 0.1s;
 }
 
+.c2:focus:not(:disabled) {
+  box-shadow: none;
+  outline: 2px solid #0969da;
+  outline-offset: 0;
+}
+
+.c2:focus:not(:disabled):not(:focus-visible) {
+  outline: solid 1px transparent;
+}
+
+.c2:focus-visible:not(:disabled) {
+  box-shadow: none;
+  outline: 2px solid #0969da;
+  outline-offset: 0;
+}
+
 .c2:active {
   border-color: hsla(210,18%,87%,1);
 }