fix(input-components): Improve focus ring styles (uber#2834)

* fix(button): improve focus-ring styles * fix(checkbox): improve focus-ring styles * fix(toggle): improve focus-ring styles * fix(radio): improve focus-ring styles and focus mngmnt * fix(radio): wip tests * fix(radio): tests * fix(radio): lint fix * fix(yard): add unique names for radio knobs * test(vrt): update visual snapshots for e5b87dd [skip ci] (uber#2835) Co-authored-by: UberOpenSourceBot <[email protected]>
refrigeratorstory · Feb 12, 2020 · d13c5cd · d13c5cd
1 parent 20aef6d
commit d13c5cd
Show file tree

Hide file tree

Showing 39 changed files with 448 additions and 111 deletions.
diff --git a/documentation-site/components/yard/config/radio.ts b/documentation-site/components/yard/config/radio.ts
@@ -72,6 +72,13 @@ const RadioGroupConfig: TConfig = {
         'baseui/radio': {named: ['Radio']},
       },
     },
+    name: {
+      value: 'number',
+      type: PropTypes.String,
+      description:
+        'String value for the name of RadioGroup, it is used to group buttons. If missed default is random ID string.',
+      hidden: false,
+    },
     align: {
       value: 'ALIGN.vertical',
       type: PropTypes.Enum,
@@ -94,13 +101,6 @@ const RadioGroupConfig: TConfig = {
       type: PropTypes.Boolean,
       description: 'Sets radio group into error state.',
     },
-    name: {
-      value: undefined,
-      type: PropTypes.String,
-      description:
-        'String value for the name of RadioGroup, it is used to group buttons. If missed default is random ID string.',
-      hidden: true,
-    },
     required: {
       value: false,
       type: PropTypes.Boolean,

diff --git a/documentation-site/components/yard/editor.tsx b/documentation-site/components/yard/editor.tsx
@@ -84,6 +84,7 @@ const Editor: React.FC<TEditorProps> = ({
         }}
       />
       <SimpleEditor
+        ignoreTabKey={true}
         value={code || ''}
         placeholder={placeholder}
         highlight={code => highlightCode(code, editorTheme, transformToken)}

diff --git a/documentation-site/components/yard/index.tsx b/documentation-site/components/yard/index.tsx
@@ -16,7 +16,7 @@ import {
   ThemeProvider,
 } from 'baseui';
 import {Card} from 'baseui/card';
-import {Spinner} from 'baseui/spinner';
+import {StyledSpinnerNext as Spinner} from 'baseui/spinner';
 import {useRouter} from 'next/router';
 
 import {useView, Compiler, Error} from 'react-view';

diff --git a/documentation-site/components/yard/knob.tsx b/documentation-site/components/yard/knob.tsx
@@ -161,7 +161,7 @@ const Knob: React.SFC<{
           <Label tooltip={getTooltip(description, type, name)}>{name}</Label>
           {numberOfOptions < 7 ? (
             <RadioGroup
-              name="radio group"
+              name={`radio-${name}`}
               align="horizontal"
               overrides={{
                 RadioGroupRoot: {

diff --git a/documentation-site/examples/radio/basic.js b/documentation-site/examples/radio/basic.js
@@ -6,7 +6,7 @@ export default () => {
   const [value, setValue] = React.useState('1');
   return (
     <RadioGroup
-      name="radio group"
+      name="basic usage"
       onChange={e => setValue(e.target.value)}
       value={value}
     >

diff --git a/documentation-site/examples/radio/basic.tsx b/documentation-site/examples/radio/basic.tsx
@@ -5,7 +5,7 @@ export default () => {
   const [value, setValue] = React.useState('1');
   return (
     <RadioGroup
-      name="radio group"
+      name="basic usage"
       onChange={e => setValue(e.target.value)}
       value={value}
     >

diff --git a/documentation-site/examples/radio/disabled.js b/documentation-site/examples/radio/disabled.js
@@ -3,7 +3,7 @@ import * as React from 'react';
 import {Radio, RadioGroup} from 'baseui/radio';
 
 export default () => (
-  <RadioGroup disabled name="radio group" value="1">
+  <RadioGroup disabled name="disabled" value="1">
     <Radio value="1">Checked</Radio>
     <Radio value="2">Unchecked</Radio>
   </RadioGroup>

diff --git a/documentation-site/examples/radio/disabled.tsx b/documentation-site/examples/radio/disabled.tsx
@@ -2,7 +2,7 @@ import * as React from 'react';
 import {Radio, RadioGroup} from 'baseui/radio';
 
 export default () => (
-  <RadioGroup disabled name="radio group" value="1">
+  <RadioGroup disabled name="disabled" value="1">
     <Radio value="1">Checked</Radio>
     <Radio value="2">Unchecked</Radio>
   </RadioGroup>

diff --git a/documentation-site/examples/radio/error.js b/documentation-site/examples/radio/error.js
@@ -7,7 +7,7 @@ export default () => {
   return (
     <RadioGroup
       isError
-      name="radio group"
+      name="error"
       onChange={e => setValue(e.target.value)}
       value={value}
     >

diff --git a/documentation-site/examples/radio/error.tsx b/documentation-site/examples/radio/error.tsx
@@ -6,7 +6,7 @@ export default () => {
   return (
     <RadioGroup
       isError
-      name="radio group"
+      name="error"
       onChange={e => setValue(e.target.value)}
       value={value}
     >

diff --git a/documentation-site/examples/radio/horizontal-align.js b/documentation-site/examples/radio/horizontal-align.js
@@ -7,7 +7,7 @@ export default () => {
   return (
     <RadioGroup
       align="horizontal"
-      name="radio group"
+      name="horizontal"
       onChange={e => setValue(e.target.value)}
       value={value}
     >

diff --git a/documentation-site/examples/radio/horizontal-align.tsx b/documentation-site/examples/radio/horizontal-align.tsx
@@ -6,7 +6,7 @@ export default () => {
   return (
     <RadioGroup
       align="horizontal"
-      name="radio group"
+      name="horizontal"
       onChange={e => setValue(e.target.value)}
       value={value}
     >

diff --git a/documentation-site/examples/radio/overrides.js b/documentation-site/examples/radio/overrides.js
@@ -7,7 +7,7 @@ export default () => {
   const [value, setValue] = React.useState('1');
   return (
     <RadioGroup
-      name="radio group"
+      name="overrides"
       onChange={e => setValue(e.target.value)}
       value={value}
     >

diff --git a/documentation-site/examples/radio/overrides.tsx b/documentation-site/examples/radio/overrides.tsx
@@ -6,7 +6,7 @@ export default () => {
   const [value, setValue] = React.useState('1');
   return (
     <RadioGroup
-      name="radio group"
+      name="overrides"
       onChange={e => setValue(e.target.value)}
       value={value}
     >

diff --git a/documentation-site/examples/radio/stateful.js b/documentation-site/examples/radio/stateful.js
@@ -3,10 +3,7 @@ import * as React from 'react';
 import {Radio, StatefulRadioGroup} from 'baseui/radio';
 
 export default () => (
-  <StatefulRadioGroup
-    name="choose one option"
-    initialState={{value: '2'}}
-  >
+  <StatefulRadioGroup name="stateful" initialState={{value: '2'}}>
     <Radio value="1">First</Radio>
     <Radio value="2">Second</Radio>
     <Radio value="3">Third</Radio>

diff --git a/documentation-site/examples/radio/stateful.tsx b/documentation-site/examples/radio/stateful.tsx
@@ -2,10 +2,7 @@ import * as React from 'react';
 import {Radio, StatefulRadioGroup} from 'baseui/radio';
 
 export default () => (
-  <StatefulRadioGroup
-    name="choose one option"
-    initialState={{value: '2'}}
-  >
+  <StatefulRadioGroup name="stateful" initialState={{value: '2'}}>
     <Radio value="1">First</Radio>
     <Radio value="2">Second</Radio>
     <Radio value="3">Third</Radio>

diff --git a/package.json b/package.json
@@ -80,7 +80,9 @@
     "@octokit/rest": "^16.33.1",
     "@storybook/react": "^5.0.0",
     "@svgr/cli": "^4.3.2",
+    "@testing-library/jest-dom": "^5.1.1",
     "@testing-library/react": "^9.1.4",
+    "@testing-library/user-event": "^8.1.0",
     "@types/babel__code-frame": "^7.0.1",
     "@types/babel__core": "^7.1.3",
     "@types/babel__generator": "^7.0.2",

diff --git a/src/button/button.js b/src/button/button.js
@@ -15,12 +15,17 @@ import {getSharedProps} from './utils.js';
 import ButtonInternals from './button-internals.js';
 import {defaultProps} from './default-props.js';
 import {getOverrides} from '../helpers/overrides.js';
+import {isFocusVisible, forkFocus, forkBlur} from '../utils/focusVisible.js';
 
 import type {ButtonPropsT} from './types.js';
 
-// eslint-disable-next-line flowtype/no-weak-types
-class Button extends React.Component<ButtonPropsT & {forwardedRef: any}> {
+class Button extends React.Component<
+  // eslint-disable-next-line flowtype/no-weak-types
+  ButtonPropsT & {forwardedRef: any},
+  {isFocusVisible: boolean},
+> {
   static defaultProps = defaultProps;
+  state = {isFocusVisible: false};
 
   internalOnClick = (...args: *) => {
     const {isLoading, onClick} = this.props;
@@ -30,6 +35,18 @@ class Button extends React.Component<ButtonPropsT & {forwardedRef: any}> {
     onClick && onClick(...args);
   };
 
+  handleFocus = (event: SyntheticEvent<>) => {
+    if (isFocusVisible(event)) {
+      this.setState({isFocusVisible: true});
+    }
+  };
+
+  handleBlur = (event: SyntheticEvent<>) => {
+    if (this.state.isFocusVisible !== false) {
+      this.setState({isFocusVisible: false});
+    }
+  };
+
   render() {
     const {
       overrides = {},
@@ -61,7 +78,10 @@ class Button extends React.Component<ButtonPropsT & {forwardedRef: any}> {
       overrides.LoadingSpinnerContainer,
       StyledLoadingSpinnerContainer,
     );
-    const sharedProps = getSharedProps(this.props);
+    const sharedProps = {
+      ...getSharedProps(this.props),
+      $isFocusVisible: this.state.isFocusVisible,
+    };
     return (
       <BaseButton
         ref={forwardedRef}
@@ -71,6 +91,8 @@ class Button extends React.Component<ButtonPropsT & {forwardedRef: any}> {
         {...baseButtonProps}
         // Applies last to override passed in onClick
         onClick={this.internalOnClick}
+        onFocus={forkFocus(baseButtonProps, this.handleFocus)}
+        onBlur={forkBlur(baseButtonProps, this.handleBlur)}
       >
         {isLoading ? (
           <React.Fragment>

diff --git a/src/button/styled-components.js b/src/button/styled-components.js
@@ -11,7 +11,16 @@ import type {SharedStylePropsT} from './types.js';
 
 export const BaseButton = styled<SharedStylePropsT>(
   'button',
-  ({$theme, $size, $kind, $shape, $isLoading, $isSelected, $disabled}) => ({
+  ({
+    $theme,
+    $size,
+    $kind,
+    $shape,
+    $isLoading,
+    $isSelected,
+    $disabled,
+    $isFocusVisible,
+  }) => ({
     display: 'inline-flex',
     // need to maintain button width while showing loading spinner
     flexDirection: $isLoading ? 'column' : 'row',
@@ -25,8 +34,9 @@ export const BaseButton = styled<SharedStylePropsT>(
     borderTopStyle: 'none',
     borderRightStyle: 'none',
     borderBottomStyle: 'none',
+    outline: $isFocusVisible ? `3px solid ${$theme.colors.accent}` : 'none',
+    outlineOffset: '-3px',
     textDecoration: 'none',
-    outline: 'none',
     WebkitAppearance: 'none',
     transitionProperty: 'background',
     transitionDuration: $theme.animation.timing100,
@@ -258,11 +268,6 @@ function getKindStyles({$theme, $isLoading, $isSelected, $kind, $disabled}) {
             ? $theme.colors.buttonPrimaryActive
             : $theme.colors.buttonPrimaryHover,
         },
-        ':focus': {
-          backgroundColor: $isLoading
-            ? $theme.colors.buttonPrimaryActive
-            : $theme.colors.buttonPrimaryHover,
-        },
         ':active': {
           backgroundColor: $theme.colors.buttonPrimaryActive,
         },
@@ -282,11 +287,6 @@ function getKindStyles({$theme, $isLoading, $isSelected, $kind, $disabled}) {
             ? $theme.colors.buttonSecondaryActive
             : $theme.colors.buttonSecondaryHover,
         },
-        ':focus': {
-          backgroundColor: $isLoading
-            ? $theme.colors.buttonSecondaryActive
-            : $theme.colors.buttonSecondaryHover,
-        },
         ':active': {
           backgroundColor: $theme.colors.buttonSecondaryActive,
         },
@@ -306,11 +306,6 @@ function getKindStyles({$theme, $isLoading, $isSelected, $kind, $disabled}) {
             ? $theme.colors.buttonTertiaryActive
             : $theme.colors.buttonTertiaryHover,
         },
-        ':focus': {
-          backgroundColor: $isLoading
-            ? $theme.colors.buttonTertiaryActive
-            : $theme.colors.buttonTertiaryHover,
-        },
         ':active': {
           backgroundColor: $theme.colors.buttonTertiaryActive,
         },
@@ -330,11 +325,6 @@ function getKindStyles({$theme, $isLoading, $isSelected, $kind, $disabled}) {
             ? $theme.colors.buttonMinimalActive
             : $theme.colors.buttonMinimalHover,
         },
-        ':focus': {
-          backgroundColor: $isLoading
-            ? $theme.colors.buttonMinimalActive
-            : $theme.colors.buttonMinimalHover,
-        },
         ':active': {
           backgroundColor: $theme.colors.buttonMinimalActive,
         },

diff --git a/src/button/types.js b/src/button/types.js
@@ -45,4 +45,5 @@ export type SharedStylePropsT = {
   $size?: $Keys<typeof SIZE>,
   $isLoading?: boolean,
   $disabled?: boolean,
+  $isFocusVisible: boolean,
 };
diff --git a/src/checkbox/checkbox.js b/src/checkbox/checkbox.js
@@ -18,6 +18,7 @@ import {
   ToggleTrack as StyledToggleTrack,
 } from './styled-components.js';
 import {STYLE_TYPE} from './constants.js';
+import {isFocusVisible} from '../utils/focusVisible.js';
 
 class StatelessCheckbox extends React.Component<PropsT, StatelessStateT> {
   static defaultProps: DefaultPropsT = {
@@ -41,6 +42,7 @@ class StatelessCheckbox extends React.Component<PropsT, StatelessStateT> {
 
   state = {
     isFocused: this.props.autoFocus || false,
+    isFocusVisible: false,
     isHovered: false,
     isActive: false,
   };
@@ -83,11 +85,17 @@ class StatelessCheckbox extends React.Component<PropsT, StatelessStateT> {
   onFocus = (e: SyntheticInputEvent<HTMLInputElement>) => {
     this.setState({isFocused: true});
     this.props.onFocus(e);
+    if (isFocusVisible(e)) {
+      this.setState({isFocusVisible: true});
+    }
   };
 
   onBlur = (e: SyntheticInputEvent<HTMLInputElement>) => {
     this.setState({isFocused: false});
     this.props.onBlur(e);
+    if (this.state.isFocusVisible !== false) {
+      this.setState({isFocusVisible: false});
+    }
   };
 
   isToggle = () => {
@@ -147,6 +155,7 @@ class StatelessCheckbox extends React.Component<PropsT, StatelessStateT> {
     };
     const sharedProps = {
       $isFocused: this.state.isFocused,
+      $isFocusVisible: this.state.isFocusVisible,
       $isHovered: this.state.isHovered,
       $isActive: this.state.isActive,
       $isError: isError,