Button/Link Focus: Use focus-visible for keyboard focus (#33066)

* outside react approach

* fixed ts issues and updated radio button

* css only solution

* Removed a bit

* Updated radio button design and fixed focus state on ToolbarButton

* Fixes

* Added missing fullWidth

packages/grafana-ui/src/components/Button/Button.story.tsx

@@ -1,7 +1,6 @@
 import React from 'react';
 import { Story } from '@storybook/react';
 import { allButtonVariants, Button, ButtonProps } from './Button';
-import { withCenteredStory, withHorizontallyCenteredStory } from '../../utils/storybook/withCenteredStory';
 import { iconOptions } from '../../utils/storybook/knobs';
 import mdx from './Button.mdx';
 import { HorizontalGroup, VerticalGroup } from '../Layout/Layout';
@@ -12,7 +11,6 @@ import { Card } from '../Card/Card';
 export default {
   title: 'Buttons/Button',
   component: Button,
-  decorators: [withCenteredStory, withHorizontallyCenteredStory],
   argTypes: {
     variant: { control: { type: 'select', options: ['primary', 'secondary', 'destructive', 'link'] } },
     size: { control: { type: 'select', options: ['sm', 'md', 'lg'] } },

packages/grafana-ui/src/components/Button/Button.tsx

@@ -5,7 +5,7 @@ import { IconName } from '../../types/icon';
 import { getPropertiesForButtonSize } from '../Forms/commonStyles';
 import { colorManipulator, GrafanaTheme, GrafanaThemeV2, ThemePaletteColor } from '@grafana/data';
 import { ComponentSize } from '../../types/size';
-import { getFocusStyles } from '../../themes/mixins';
+import { getFocusStyles, getMouseFocusStyles } from '../../themes/mixins';
 import { Icon } from '../Icon/Icon';
 
 export type ButtonVariant = 'primary' | 'secondary' | 'destructive' | 'link';
@@ -34,7 +34,7 @@ export const Button = React.forwardRef<HTMLButtonElement, ButtonProps>(
     });
 
     return (
-      <button className={cx(styles.button, className)} {...otherProps} ref={ref}>
+      <button className={cx(styles.button, className)} {...otherProps}>
         {icon && <Icon name={icon} size={size} className={styles.icon} />}
         {children && <span className={styles.content}>{children}</span>}
       </button>
@@ -47,7 +47,21 @@ Button.displayName = 'Button';
 type ButtonLinkProps = CommonProps & ButtonHTMLAttributes<HTMLButtonElement> & AnchorHTMLAttributes<HTMLAnchorElement>;
 
 export const LinkButton = React.forwardRef<HTMLAnchorElement, ButtonLinkProps>(
-  ({ variant = 'primary', size = 'md', icon, fullWidth, children, className, disabled, ...otherProps }, ref) => {
+  (
+    {
+      variant = 'primary',
+      size = 'md',
+      icon,
+      fullWidth,
+      children,
+      className,
+      onBlur,
+      onFocus,
+      disabled,
+      ...otherProps
+    },
+    ref
+  ) => {
     const theme = useTheme();
     const styles = getButtonStyles({
       theme,
@@ -60,7 +74,7 @@ export const LinkButton = React.forwardRef<HTMLAnchorElement, ButtonLinkProps>(
     const linkButtonStyles = cx(styles.button, { [styles.disabled]: disabled }, className);
 
     return (
-      <a className={linkButtonStyles} {...otherProps} ref={ref} tabIndex={disabled ? -1 : 0}>
+      <a className={linkButtonStyles} {...otherProps} tabIndex={disabled ? -1 : 0}>
         {icon && <Icon name={icon} size={size} className={styles.icon} />}
         {children && <span className={styles.content}>{children}</span>}
       </a>
@@ -99,6 +113,8 @@ export const getButtonStyles = (props: StyleProps) => {
     },
   };
 
+  const focusStyle = getFocusStyles(theme.v2);
+
   return {
     button: css({
       label: 'button',
@@ -114,6 +130,9 @@ export const getButtonStyles = (props: StyleProps) => {
       verticalAlign: 'middle',
       cursor: 'pointer',
       borderRadius: theme.v2.shape.borderRadius(1),
+      '&:focus': focusStyle,
+      '&:focus-visible': focusStyle,
+      '&:focus:not(:focus-visible)': getMouseFocusStyles(theme.v2),
       ...(fullWidth && {
         flexGrow: 1,
         justifyContent: 'center',
@@ -155,10 +174,6 @@ function getButtonVariantStyles(theme: GrafanaThemeV2, color: ThemePaletteColor)
       color: color.contrastText,
       boxShadow: theme.shadows.z2,
     },
-
-    '&:focus': {
-      ...getFocusStyles(theme),
-    },
   };
 }
 

packages/grafana-ui/src/components/Button/ToolbarButton.tsx

@@ -8,6 +8,7 @@ import { Icon } from '../Icon/Icon';
 import { getPropertiesForVariant } from './Button';
 import { isString } from 'lodash';
 import { selectors } from '@grafana/e2e-selectors';
+import { focusCss, getMouseFocusStyles } from '../../themes/mixins';
 
 export interface Props extends ButtonHTMLAttributes<HTMLButtonElement> {
   /** Icon name */
@@ -117,14 +118,20 @@ const getStyles = (theme: GrafanaTheme) => {
       border-radius: ${theme.v2.shape.borderRadius()};
       line-height: ${theme.v2.components.height.md * theme.v2.spacing.gridSize - 2}px;
       font-weight: ${theme.v2.typography.fontWeightMedium};
-      border: 1px solid ${theme.v2.palette.border1};      
+      border: 1px solid ${theme.v2.palette.border1};
       white-space: nowrap;
       transition: ${theme.v2.transitions.create(['background', 'box-shadow', 'border-color', 'color'], {
         duration: theme.v2.transitions.duration.short,
-      })},
+      })};
 
-      &:focus {
-        outline: none;
+      &:focus,
+      &:focus-visible {
+        ${focusCss(theme)}
+        z-index: 1;
+      }
+
+      &:focus:not(:focus-visible) {
+        ${getMouseFocusStyles(theme.v2)}
       }
 
       &:hover {
@@ -143,7 +150,7 @@ const getStyles = (theme: GrafanaTheme) => {
           background: ${theme.v2.palette.action.disabledBackground};
           box-shadow: none;
         }
-      }      
+      }
     `,
     default: css`
       color: ${theme.v2.palette.text.secondary};

packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButton.tsx

@@ -3,7 +3,7 @@ import { useTheme, stylesFactory } from '../../../themes';
 import { GrafanaTheme } from '@grafana/data';
 import { css, cx } from '@emotion/css';
 import { getPropertiesForButtonSize } from '../commonStyles';
-import { focusCss } from '../../../themes/mixins';
+import { getFocusStyles, getMouseFocusStyles } from '../../../themes/mixins';
 
 export type RadioButtonSize = 'sm' | 'md';
 
@@ -18,73 +18,6 @@ export interface RadioButtonProps {
   fullWidth?: boolean;
 }
 
-const getRadioButtonStyles = stylesFactory((theme: GrafanaTheme, size: RadioButtonSize, fullWidth?: boolean) => {
-  const { fontSize, height, padding } = getPropertiesForButtonSize(size, theme.v2);
-
-  const textColor = theme.v2.palette.text.secondary;
-  const textColorHover = theme.v2.palette.text.primary;
-  const textColorActive = theme.v2.palette.primary.text;
-  const borderColor = theme.v2.components.form.border;
-  const borderColorHover = theme.v2.components.form.borderHover;
-  const borderColorActive = theme.v2.components.form.border;
-  const bg = theme.colors.bodyBg;
-  const bgActive = theme.v2.palette.layer2;
-  const border = `1px solid ${borderColor}`;
-  const borderActive = `1px solid ${borderColorActive}`;
-  const borderHover = `1px solid ${borderColorHover}`;
-
-  return {
-    radio: css`
-      position: absolute;
-      opacity: 0;
-      z-index: -1000;
-
-      &:checked + label {
-        border: ${borderActive};
-        color: ${textColorActive};
-        background: ${bgActive};
-        z-index: 3;
-      }
-
-      &:focus + label {
-        ${focusCss(theme)};
-        z-index: 3;
-      }
-
-      &:disabled + label {
-        cursor: default;
-        color: ${theme.v2.palette.text.disabled};
-        cursor: not-allowed;
-      }
-    `,
-    radioLabel: css`
-      display: inline-block;
-      position: relative;
-      font-size: ${fontSize};
-      height: ${theme.v2.spacing(height)};
-      // Deduct border from line-height for perfect vertical centering on windows and linux
-      line-height: ${theme.v2.spacing.gridSize * height - 2}px;
-      color: ${textColor};
-      padding: ${theme.v2.spacing(0, padding)};
-      margin-left: -1px;
-      border-radius: ${theme.border.radius.sm};
-      border: ${border};
-      background: ${bg};
-      cursor: pointer;
-      z-index: 1;
-      flex: ${fullWidth ? `1 0 0` : 'none'};
-      text-align: center;
-      user-select: none;
-
-      &:hover {
-        color: ${textColorHover};
-        border: ${borderHover};
-        z-index: 2;
-      }
-    `,
-  };
-});
-
 export const RadioButton: React.FC<RadioButtonProps> = ({
   children,
   active = false,
@@ -118,3 +51,62 @@ export const RadioButton: React.FC<RadioButtonProps> = ({
 };
 
 RadioButton.displayName = 'RadioButton';
+
+const getRadioButtonStyles = stylesFactory((theme: GrafanaTheme, size: RadioButtonSize, fullWidth?: boolean) => {
+  const { fontSize, height, padding } = getPropertiesForButtonSize(size, theme.v2);
+
+  const textColor = theme.v2.palette.text.secondary;
+  const textColorHover = theme.v2.palette.text.primary;
+  const bg = theme.colors.bodyBg;
+
+  return {
+    radio: css`
+      position: absolute;
+      opacity: 0;
+      z-index: -1000;
+
+      &:checked + label {
+        color: ${theme.v2.palette.text.primary};
+        font-weight: ${theme.v2.typography.fontWeightMedium};
+        background: ${theme.v2.palette.action.selected};
+        z-index: 3;
+      }
+
+      &:focus + label,
+      &:focus-visible + label {
+        ${getFocusStyles(theme.v2)};
+      }
+
+      &:focus:not(:focus-visible) + label {
+        ${getMouseFocusStyles(theme.v2)}
+      }
+
+      &:disabled + label {
+        cursor: default;
+        color: ${theme.v2.palette.text.disabled};
+        cursor: not-allowed;
+      }
+    `,
+    radioLabel: css`
+      display: inline-block;
+      position: relative;
+      font-size: ${fontSize};
+      height: ${theme.v2.spacing(height)};
+      // Deduct border from line-height for perfect vertical centering on windows and linux
+      line-height: ${theme.v2.spacing.gridSize * height - 2}px;
+      color: ${textColor};
+      padding: ${theme.v2.spacing(0, padding)};
+      border-radius: ${theme.v2.shape.borderRadius()};
+      background: ${bg};
+      cursor: pointer;
+      z-index: 1;
+      flex: ${fullWidth ? `1 0 0` : 'none'};
+      text-align: center;
+      user-select: none;
+
+      &:hover {
+        color: ${textColorHover};
+      }
+    `,
+  };
+});

packages/grafana-ui/src/components/Forms/RadioButtonGroup/RadioButtonGroup.tsx

@@ -1,41 +1,11 @@
 import React, { useCallback, useRef } from 'react';
 import { css, cx } from '@emotion/css';
 import uniqueId from 'lodash/uniqueId';
-import { SelectableValue } from '@grafana/data';
+import { GrafanaTheme, SelectableValue } from '@grafana/data';
 import { RadioButtonSize, RadioButton } from './RadioButton';
 import { Icon } from '../../Icon/Icon';
 import { IconName } from '../../../types/icon';
-
-const getRadioButtonGroupStyles = () => {
-  return {
-    wrapper: css`
-      display: flex;
-      flex-direction: row;
-      flex-wrap: nowrap;
-      position: relative;
-    `,
-    radioGroup: css`
-      display: flex;
-      flex-direction: row;
-      flex-wrap: nowrap;
-
-      label {
-        border-radius: 0px;
-
-        &:first-of-type {
-          border-radius: 2px 0px 0px 2px;
-        }
-
-        &:last-of-type {
-          border-radius: 0px 2px 2px 0px;
-        }
-      }
-    `,
-    icon: css`
-      margin-right: 6px;
-    `,
-  };
-};
+import { useStyles } from '../../../themes';
 
 export interface RadioButtonGroupProps<T> {
   value?: T;
@@ -70,10 +40,10 @@ export function RadioButtonGroup<T>({
   );
   const id = uniqueId('radiogroup-');
   const groupName = useRef(id);
-  const styles = getRadioButtonGroupStyles();
+  const styles = useStyles(getStyles);
 
   return (
-    <div className={cx(styles.radioGroup, className)}>
+    <div className={cx(styles.radioGroup, fullWidth && styles.fullWidth, className)}>
       {options.map((o, i) => {
         const isItemDisabled = disabledOptions && o.value && disabledOptions.includes(o.value);
         return (
@@ -85,8 +55,8 @@ export function RadioButtonGroup<T>({
             onChange={handleOnChange(o)}
             id={`option-${o.value}-${id}`}
             name={groupName.current}
-            fullWidth={fullWidth}
             description={o.description}
+            fullWidth={fullWidth}
           >
             {o.icon && <Icon name={o.icon as IconName} className={styles.icon} />}
             {o.label}
@@ -98,3 +68,22 @@ export function RadioButtonGroup<T>({
 }
 
 RadioButtonGroup.displayName = 'RadioButtonGroup';
+
+const getStyles = (theme: GrafanaTheme) => {
+  return {
+    radioGroup: css({
+      display: 'inline-flex',
+      flexDirection: 'row',
+      flexWrap: 'nowrap',
+      border: `1px solid ${theme.v2.components.form.border}`,
+      borderRadius: theme.v2.shape.borderRadius(),
+      padding: '2px',
+    }),
+    fullWidth: css({
+      display: 'flex',
+    }),
+    icon: css`
+      margin-right: 6px;
+    `,
+  };
+};

packages/grafana-ui/src/components/IconButton/IconButton.tsx

@@ -7,7 +7,7 @@ import { useTheme } from '../../themes/ThemeContext';
 import { GrafanaTheme } from '@grafana/data';
 import { Tooltip } from '../Tooltip/Tooltip';
 import { TooltipPlacement } from '../Tooltip/PopoverController';
-import { focusCss } from '../../themes/mixins';
+import { focusCss, getMouseFocusStyles } from '../../themes/mixins';
 
 export interface Props extends React.ButtonHTMLAttributes<HTMLButtonElement> {
   /** Name of the icon **/
@@ -69,6 +69,7 @@ const getStyles = stylesFactory((theme: GrafanaTheme, size: IconSize) => {
       align-items: center;
       justify-content: center;
       position: relative;
+      border-radius: ${theme.v2.shape.borderRadius()};
       z-index: 0;
       margin-right: ${theme.spacing.xs};
 
@@ -98,10 +99,15 @@ const getStyles = stylesFactory((theme: GrafanaTheme, size: IconSize) => {
         transition-property: transform, opacity;
       }
 
-      &:focus {
+      &:focus,
+      &:focus-visible {
         ${focusCss(theme)}
       }
 
+      &:focus:not(:focus-visible) {
+        ${getMouseFocusStyles(theme.v2)}
+      }
+
       &:hover {
         color: ${theme.colors.linkHover};
 

packages/grafana-ui/src/themes/mixins.ts

@@ -46,11 +46,19 @@ export const focusCss = (theme: GrafanaTheme) => `
   transition: all 0.2s cubic-bezier(0.19, 1, 0.22, 1);
 `;
 
+export function getMouseFocusStyles(theme: GrafanaThemeV2): CSSObject {
+  return {
+    outline: 'none',
+    boxShadow: `${theme.shadows.z1}`,
+    transition: theme.transitions.create('box-shadow'),
+  };
+}
+
 export function getFocusStyles(theme: GrafanaThemeV2): CSSObject {
   return {
     outline: '2px dotted transparent',
     outlineOffset: '2px',
-    boxShadow: `0 0 0 2px ${theme.palette.layer0}, 0 0 0px 4px ${theme.palette.primary.border}`,
+    boxShadow: `0 0 0 2px ${theme.palette.layer0}, 0 0 0px 4px ${theme.palette.primary.main}`,
     transition: `all 0.2s cubic-bezier(0.19, 1, 0.22, 1)`,
   };
 }

public/app/core/components/AppNotifications/AppNotificationList.tsx

@@ -11,6 +11,7 @@ import {
 } from '../../copy/appNotification';
 import { AppEvents } from '@grafana/data';
 import { connect, ConnectedProps } from 'react-redux';
+import { VerticalGroup } from '@grafana/ui';
 
 export interface OwnProps {}
 
@@ -45,15 +46,17 @@ export class AppNotificationListUnConnected extends PureComponent<Props> {
 
     return (
       <div className="page-alert-list">
-        {appNotifications.map((appNotification, index) => {
-          return (
-            <AppNotificationItem
-              key={`${appNotification.id}-${index}`}
-              appNotification={appNotification}
-              onClearNotification={(id) => this.onClearAppNotification(id)}
-            />
-          );
-        })}
+        <VerticalGroup>
+          {appNotifications.map((appNotification, index) => {
+            return (
+              <AppNotificationItem
+                key={`${appNotification.id}-${index}`}
+                appNotification={appNotification}
+                onClearNotification={(id) => this.onClearAppNotification(id)}
+              />
+            );
+          })}
+        </VerticalGroup>
       </div>
     );
   }

public/app/core/components/QueryOperationRow/QueryOperationRow.tsx

@@ -130,6 +130,10 @@ const getQueryOperationRowStyles = stylesFactory((theme: GrafanaTheme) => {
       align-items: center;
       justify-content: space-between;
       white-space: nowrap;
+
+      &:focus {
+        outline: none;
+      }
     `,
     dragIcon: css`
       cursor: drag;

public/app/core/core.ts

@@ -11,7 +11,6 @@ import './jquery_extended';
 import './partials';
 import './components/jsontree/jsontree';
 import './components/code_editor/code_editor';
-import './utils/outline';
 import './components/colorpicker/spectrum_picker';
 import './services/search_srv';
 import './services/ng_react';

public/app/core/utils/outline.ts

@@ -1,34 +0,0 @@
-// based on http://www.paciellogroup.com/blog/2012/04/how-to-remove-css-outlines-in-an-accessible-manner/
-function outlineFixer() {
-  const d: any = document;
-
-  const styleElement = d.createElement('STYLE');
-  const domEvents = 'addEventListener' in d;
-
-  const addEventListener = (type: string, callback: { (): void; (): void }) => {
-    // Basic cross-browser event handling
-    if (domEvents) {
-      d.addEventListener(type, callback);
-    } else {
-      d.attachEvent('on' + type, callback);
-    }
-  };
-
-  const setCss = (cssText: string) => {
-    // Handle setting of <style> element contents in IE8
-    !!styleElement.styleSheet ? (styleElement.styleSheet.cssText = cssText) : (styleElement.innerHTML = cssText);
-  };
-
-  d.getElementsByTagName('HEAD')[0].appendChild(styleElement);
-
-  // Using mousedown instead of mouseover, so that previously focused elements don't lose focus ring on mouse move
-  addEventListener('mousedown', () => {
-    setCss(':focus{outline:0 !important}::-moz-focus-inner{border:0;}');
-  });
-
-  addEventListener('keydown', () => {
-    setCss('');
-  });
-}
-
-outlineFixer();