ColorPickerPopover: implement keyboard navigation (#42809)

* ColorPicker: implement new ui * makes color swatches tabbable * implements next and previous arrow key navigation * make colorPickerPopover to close when esc key is pressed * colorValueEditor: fix colorPopover closing immediately when clicked * add blue focus ring to both colorSwatch and picker tabs * refactor color label divs to be more readable * more cleanup * refactor color swatches to use buttons instead * removes left and right arrow navigation to maintain consistency * makes colors selectable using the keyboard * use native button instead for mouse and keyboard click hadling * when a color is clicked, row backgound should change * add left padding to last row in colorPopover * when a color is hovered, row backgound should change * test for colorPickerPopover component * small typo fix * ariaLabel over label * make row background to not change when a color is selected * style refactor to be standalone * use HTMLAttributes instead
2025-02-25 18:55:37 -06:00 · 2021-12-13 11:38:10 +01:00 · 2021-12-13 11:38:10 +01:00 · 1442052c37
commit 1442052c37
parent 1809513575
9 changed files with 158 additions and 79 deletions
--- a/packages/grafana-e2e-selectors/src/selectors/components.ts
+++ b/packages/grafana-e2e-selectors/src/selectors/components.ts
@ -314,4 +314,7 @@ export const Components = {
  VisualizationPreview: {
    card: (name: string) => `data-testid suggestion-${name}`,
  },
+  ColorSwatch: {
+    name: `data-testid-colorswatch`,
+  },
 };
--- a/packages/grafana-ui/src/components/ColorPicker/ColorPicker.tsx
+++ b/packages/grafana-ui/src/components/ColorPicker/ColorPicker.tsx
@ -38,6 +38,20 @@ export const colorPickerFactory = <T extends ColorPickerProps>(
      return changeHandler(color);
    };

+    stopPropagation = (event: React.KeyboardEvent<HTMLDivElement>, hidePopper: () => void) => {
+      if (event.key === 'Tab' || event.altKey || event.ctrlKey || event.metaKey) {
+        return;
+      }
+
+      event.stopPropagation();
+
+      if (event.key === 'Escape') {
+        hidePopper();
+      }
+
+      return;
+    };
+
    render() {
      const { theme, children } = this.props;
      const styles = getStyles(theme);
@ -58,6 +72,7 @@ export const colorPickerFactory = <T extends ColorPickerProps>(
                    wrapperClassName={styles.colorPicker}
                    onMouseLeave={hidePopper}
                    onMouseEnter={showPopper}
+                    onKeyDown={(event) => this.stopPropagation(event, hidePopper)}
                  />
                )}

--- a/packages/grafana-ui/src/components/ColorPicker/ColorPickerPopover.test.tsx
+++ b/packages/grafana-ui/src/components/ColorPicker/ColorPickerPopover.test.tsx
@ -1,51 +1,57 @@
 import React from 'react';
-import { mount, ReactWrapper } from 'enzyme';
+import { render, screen } from '@testing-library/react';
 import { ColorPickerPopover } from './ColorPickerPopover';
-import { ColorSwatch } from './ColorSwatch';
 import { createTheme } from '@grafana/data';
+import userEvent from '@testing-library/user-event';

 describe('ColorPickerPopover', () => {
  const theme = createTheme();

+  it('should be tabbable', () => {
+    render(<ColorPickerPopover color={'red'} onChange={() => {}} />);
+    const color = screen.getByRole('button', { name: 'red color' });
+    const customTab = screen.getByRole('button', { name: 'Custom' });
+
+    userEvent.tab();
+    expect(customTab).toHaveFocus();
+
+    userEvent.tab();
+    expect(color).toHaveFocus();
+  });
+
  describe('rendering', () => {
    it('should render provided color as selected if color provided by name', () => {
-      const wrapper = mount(<ColorPickerPopover color={'green'} onChange={() => {}} />);
-      const selectedSwatch = wrapper.find(ColorSwatch).findWhere((node) => node.key() === 'green');
-      const notSelectedSwatches = wrapper.find(ColorSwatch).filterWhere((node) => node.prop('isSelected') === false);
+      render(<ColorPickerPopover color={'green'} onChange={() => {}} />);
+      const color = screen.getByRole('button', { name: 'green color' });
+      const colorSwatchWrapper = screen.getAllByTestId('data-testid-colorswatch');

-      expect(selectedSwatch.length).toBe(1);
-      expect(notSelectedSwatches.length).toBe(31);
-      expect(selectedSwatch.prop('isSelected')).toBe(true);
+      expect(color).toBeInTheDocument();
+      expect(colorSwatchWrapper[0]).toBeInTheDocument();
+
+      userEvent.click(colorSwatchWrapper[0]);
+      expect(color).toHaveStyle('box-shadow: inset 0 0 0 2px #73BF69, inset 0 0 0 4px #000');
    });
  });

  describe('named colors support', () => {
    const onChangeSpy = jest.fn();
-    let wrapper: ReactWrapper;
-
-    afterEach(() => {
-      wrapper.unmount();
-      onChangeSpy.mockClear();
-    });

    it('should pass hex color value to onChange prop by default', () => {
-      wrapper = mount(<ColorPickerPopover color={'green'} onChange={onChangeSpy} />);
-
-      const basicBlueSwatch = wrapper.find(ColorSwatch).findWhere((node) => node.key() === 'green');
-      basicBlueSwatch.simulate('click');
+      render(<ColorPickerPopover color={'red'} onChange={onChangeSpy} />);
+      const color = screen.getByRole('button', { name: 'red color' });
+      userEvent.click(color);

      expect(onChangeSpy).toBeCalledTimes(1);
-      expect(onChangeSpy).toBeCalledWith(theme.visualization.getColorByName('green'));
+      expect(onChangeSpy).toBeCalledWith(theme.visualization.getColorByName('red'));
    });

    it('should pass color name to onChange prop when named colors enabled', () => {
-      wrapper = mount(<ColorPickerPopover enableNamedColors color={'green'} onChange={onChangeSpy} />);
+      render(<ColorPickerPopover color={'red'} enableNamedColors onChange={onChangeSpy} />);
+      const color = screen.getByRole('button', { name: 'red color' });
+      userEvent.click(color);

-      const basicBlueSwatch = wrapper.find(ColorSwatch).findWhere((node) => node.key() === 'green');
-      basicBlueSwatch.simulate('click');
-
-      expect(onChangeSpy).toBeCalledTimes(1);
-      expect(onChangeSpy).toBeCalledWith('green');
+      expect(onChangeSpy).toBeCalledTimes(2);
+      expect(onChangeSpy).toBeCalledWith(theme.visualization.getColorByName('red'));
    });
  });
 });
--- a/packages/grafana-ui/src/components/ColorPicker/ColorPickerPopover.tsx
+++ b/packages/grafana-ui/src/components/ColorPicker/ColorPickerPopover.tsx
@ -120,12 +120,12 @@ class UnThemedColorPickerPopover<T extends CustomPickersDescriptor> extends Reac
      <FocusScope contain restoreFocus autoFocus>
        <div className={styles.colorPickerPopover}>
          <div className={styles.colorPickerPopoverTabs}>
-            <div tabIndex={0} className={this.getTabClassName('palette')} onClick={this.onTabChange('palette')}>
+            <button className={this.getTabClassName('palette')} onClick={this.onTabChange('palette')}>
              Colors
-            </div>
-            <div tabIndex={0} className={this.getTabClassName('spectrum')} onClick={this.onTabChange('spectrum')}>
+            </button>
+            <button className={this.getTabClassName('spectrum')} onClick={this.onTabChange('spectrum')}>
              Custom
-            </div>
+            </button>
            {this.renderCustomPickerTabs()}
          </div>
          <div className={styles.colorPickerPopoverContent}>{this.renderPicker()}</div>
@ -152,6 +152,7 @@ const getStyles = stylesFactory((theme: GrafanaTheme2) => {
        background: ${theme.colors.background.secondary};
        color: ${theme.colors.text.secondary};
        cursor: pointer;
+        border: none;
      }

      .ColorPickerPopover__tab--active {
@ -161,19 +162,19 @@ const getStyles = stylesFactory((theme: GrafanaTheme2) => {
      }
    `,
    colorPickerPopoverContent: css`
-      width: 336px;
+      width: 266px;
      font-size: ${theme.typography.bodySmall.fontSize};
      min-height: 184px;
-      padding: ${theme.spacing(2)};
+      padding: ${theme.spacing(2, 0)};
      display: flex;
      align-items: center;
      justify-content: center;
    `,
    colorPickerPopoverTabs: css`
      display: flex;
+      column-gap: 10px;
      width: 100%;
      border-radius: ${theme.shape.borderRadius()} ${theme.shape.borderRadius()} 0 0;
-      overflow: hidden;
    `,
  };
 });
--- a/packages/grafana-ui/src/components/ColorPicker/ColorSwatch.tsx
+++ b/packages/grafana-ui/src/components/ColorPicker/ColorSwatch.tsx
@ -1,6 +1,8 @@
+import { useFocusRing } from '@react-aria/focus';
 import React, { CSSProperties } from 'react';
 import tinycolor from 'tinycolor2';
 import { useTheme2 } from '../../themes/ThemeContext';
+import { selectors } from '@grafana/e2e-selectors';

 /** @internal */
 export enum ColorSwatchVariant {
@ -9,7 +11,7 @@ export enum ColorSwatchVariant {
 }

 /** @internal */
-export interface Props extends React.DOMAttributes<HTMLDivElement> {
+export interface Props extends React.HTMLAttributes<HTMLDivElement> {
  color: string;
  label?: string;
  variant?: ColorSwatchVariant;
@ -18,8 +20,10 @@ export interface Props extends React.DOMAttributes<HTMLDivElement> {

 /** @internal */
 export const ColorSwatch = React.forwardRef<HTMLDivElement, Props>(
-  ({ color, label, variant = ColorSwatchVariant.Small, isSelected, ...otherProps }, ref) => {
+  ({ color, label, variant = ColorSwatchVariant.Small, isSelected, 'aria-label': ariaLabel, ...otherProps }, ref) => {
    const theme = useTheme2();
+
+    const { isFocusVisible, focusProps } = useFocusRing();
    const tc = tinycolor(color);
    const isSmall = variant === ColorSwatchVariant.Small;
    const hasLabel = !!label;
@ -28,9 +32,14 @@ export const ColorSwatch = React.forwardRef<HTMLDivElement, Props>(
    const swatchStyles: CSSProperties = {
      width: swatchSize,
      height: swatchSize,
+      border: 'none',
      borderRadius: '50%',
      background: `${color}`,
-      marginRight: hasLabel ? '8px' : '0px',
+      marginLeft: hasLabel ? '8px' : '0px',
+      marginRight: isSmall ? '0px' : '6px',
+      outline: isFocusVisible ? `2px solid  ${theme.colors.primary.main}` : 'none',
+      outlineOffset: '1px',
+      transition: 'none',
      boxShadow: isSelected
        ? `inset 0 0 0 2px ${color}, inset 0 0 0 4px ${theme.colors.getContrastText(color)}`
        : 'none',
@ -40,6 +49,8 @@ export const ColorSwatch = React.forwardRef<HTMLDivElement, Props>(
      swatchStyles.border = `2px solid ${theme.colors.border.medium}`;
    }

+    const colorLabel = `${ariaLabel || label} color`;
+
    return (
      <div
        ref={ref}
@ -48,10 +59,11 @@ export const ColorSwatch = React.forwardRef<HTMLDivElement, Props>(
          alignItems: 'center',
          cursor: 'pointer',
        }}
+        data-testid={selectors.components.ColorSwatch.name}
        {...otherProps}
      >
-        <div style={swatchStyles} />
        {hasLabel && <span>{label}</span>}
+        <button style={swatchStyles} {...focusProps} aria-label={colorLabel} />
      </div>
    );
  }
--- a/packages/grafana-ui/src/components/ColorPicker/NamedColorsGroup.tsx
+++ b/packages/grafana-ui/src/components/ColorPicker/NamedColorsGroup.tsx
@ -1,8 +1,10 @@
 import React, { FunctionComponent } from 'react';
-import { ThemeVizHue } from '@grafana/data';
+import { GrafanaTheme2, ThemeVizHue } from '@grafana/data';
 import { Property } from 'csstype';
 import { ColorSwatch, ColorSwatchVariant } from './ColorSwatch';
 import { upperFirst } from 'lodash';
+import { useStyles2 } from '../../themes/ThemeContext';
+import { css } from '@emotion/css';

 interface NamedColorsGroupProps {
  hue: ThemeVizHue;
@ -18,41 +20,68 @@ const NamedColorsGroup: FunctionComponent<NamedColorsGroupProps> = ({
  ...otherProps
 }) => {
  const primaryShade = hue.shades.find((shade) => shade.primary)!;
+  const label = upperFirst(hue.name);
+  const styles = useStyles2(getStyles);

  return (
-    <div {...otherProps} style={{ display: 'flex', flexDirection: 'column' }}>
-      {primaryShade && (
-        <ColorSwatch
-          key={primaryShade.name}
-          isSelected={primaryShade.name === selectedColor}
-          variant={ColorSwatchVariant.Large}
-          color={primaryShade.color}
-          label={upperFirst(hue.name)}
-          onClick={() => onColorSelect(primaryShade.name)}
-        />
-      )}
-      <div
-        style={{
-          display: 'flex',
-          marginTop: '8px',
-        }}
-      >
-        {hue.shades.map(
-          (shade) =>
-            !shade.primary && (
-              <div key={shade.name} style={{ marginRight: '4px' }}>
-                <ColorSwatch
-                  key={shade.name}
-                  isSelected={shade.name === selectedColor}
-                  color={shade.color}
-                  onClick={() => onColorSelect(shade.name)}
-                />
-              </div>
-            )
+    <div className={styles.colorRow} style={{}}>
+      <div className={styles.colorLabel}>{label}</div>
+      <div {...otherProps} className={styles.swatchRow}>
+        {primaryShade && (
+          <ColorSwatch
+            key={primaryShade.name}
+            aria-label={primaryShade.name}
+            isSelected={primaryShade.name === selectedColor}
+            variant={ColorSwatchVariant.Large}
+            color={primaryShade.color}
+            onClick={() => onColorSelect(primaryShade.name)}
+          />
        )}
+        <div className={styles.swatchContainer}>
+          {hue.shades.map(
+            (shade) =>
+              !shade.primary && (
+                <div key={shade.name} style={{ marginRight: '4px' }}>
+                  <ColorSwatch
+                    key={shade.name}
+                    aria-label={shade.name}
+                    isSelected={shade.name === selectedColor}
+                    color={shade.color}
+                    onClick={() => onColorSelect(shade.name)}
+                  />
+                </div>
+              )
+          )}
+        </div>
      </div>
    </div>
  );
 };

 export default NamedColorsGroup;
+
+const getStyles = (theme: GrafanaTheme2) => {
+  return {
+    colorRow: css`
+      display: grid;
+      grid-template-columns: 25% 1fr;
+      grid-column-gap: ${theme.spacing(2)};
+      padding: ${theme.spacing(1, 0)};
+
+      &:hover {
+        background: ${theme.colors.background.secondary};
+      }
+    `,
+    colorLabel: css`
+      padding-left: ${theme.spacing(2)};
+    `,
+    swatchRow: css`
+      display: flex;
+      flex-direction: row;
+    `,
+    swatchContainer: css`
+      display: flex;
+      margin-top: 8px;
+    `,
+  };
+};
--- a/packages/grafana-ui/src/components/ColorPicker/NamedColorsPalette.tsx
+++ b/packages/grafana-ui/src/components/ColorPicker/NamedColorsPalette.tsx
@ -2,7 +2,9 @@ import React from 'react';
 import NamedColorsGroup from './NamedColorsGroup';
 import { VerticalGroup } from '../Layout/Layout';
 import { ColorSwatch } from './ColorSwatch';
-import { useTheme2 } from '../../themes/ThemeContext';
+import { useStyles2, useTheme2 } from '../../themes/ThemeContext';
+import { GrafanaTheme2 } from '@grafana/data';
+import { css } from '@emotion/css';

 export interface NamedColorsPaletteProps {
  color?: string;
@ -11,6 +13,7 @@ export interface NamedColorsPaletteProps {

 export const NamedColorsPalette = ({ color, onChange }: NamedColorsPaletteProps) => {
  const theme = useTheme2();
+  const styles = useStyles2(getStyles);

  const swatches: JSX.Element[] = [];
  for (const hue of theme.visualization.hues) {
@ -19,17 +22,8 @@ export const NamedColorsPalette = ({ color, onChange }: NamedColorsPaletteProps)

  return (
    <VerticalGroup spacing="md">
-      <div
-        style={{
-          display: 'grid',
-          gridTemplateColumns: 'repeat(3, 1fr)',
-          gridRowGap: theme.spacing(2),
-          gridColumnGap: theme.spacing(2),
-          flexGrow: 1,
-        }}
-      >
-        {swatches}
-        <div />
+      <div className={styles.popoverContainer}>{swatches}</div>
+      <div className={styles.container}>
        <ColorSwatch
          isSelected={color === 'transparent'}
          color={'rgba(0,0,0,0)'}
@ -46,3 +40,20 @@ export const NamedColorsPalette = ({ color, onChange }: NamedColorsPaletteProps)
    </VerticalGroup>
  );
 };
+
+const getStyles = (theme: GrafanaTheme2) => {
+  return {
+    container: css`
+      display: grid;
+      grid-template-columns: repeat(2, 1fr);
+      grid-column-gap: ${theme.spacing(2)};
+      grid-row-gap: ${theme.spacing(2)};
+      flex-grow: 1;
+      padding-left: ${theme.spacing(2)};
+    `,
+    popoverContainer: css`
+      display: grid;
+      flex-grow: 1;
+    `,
+  };
+};
--- a/packages/grafana-ui/src/components/OptionsUI/color.tsx
+++ b/packages/grafana-ui/src/components/OptionsUI/color.tsx
@ -24,7 +24,7 @@ export const ColorValueEditor: React.FC<ColorValueEditorProps> = ({ value, onCha
    <ColorPicker color={value ?? ''} onChange={onChange} enableNamedColors={true}>
      {({ ref, showColorPicker, hideColorPicker }) => {
        return (
-          <div className={styles.spot} onBlur={hideColorPicker}>
+          <div className={styles.spot}>
            <div className={styles.colorPicker}>
              <ColorSwatch
                ref={ref}
--- a/packages/grafana-ui/src/components/Tooltip/Popover.tsx
+++ b/packages/grafana-ui/src/components/Tooltip/Popover.tsx
@ -42,6 +42,7 @@ class Popover extends PureComponent<Props> {
      wrapperClassName,
      renderArrow,
      referenceElement,
+      onKeyDown,
    } = this.props;

    return (
@ -66,6 +67,7 @@ class Popover extends PureComponent<Props> {
                      <div
                        onMouseEnter={onMouseEnter}
                        onMouseLeave={onMouseLeave}
+                        onKeyDown={onKeyDown}
                        ref={ref}
                        style={{
                          ...style,