ColorPicker: Refine design of color picker (#43275)

* ColorPicker: Refine design of the color picker popover * One more tweak * removed unneeded div and add hover style * Fixing test * Reverse order from dark to brighter
2025-02-25 18:55:37 -06:00 · 2021-12-20 10:50:48 +01:00 · 2021-12-20 10:50:48 +01:00 · b867ecb515
commit b867ecb515
parent 85f134969e
5 changed files with 111 additions and 95 deletions
--- a/packages/grafana-ui/src/components/ColorPicker/ColorPickerPopover.test.tsx
+++ b/packages/grafana-ui/src/components/ColorPicker/ColorPickerPopover.test.tsx
@ -1,5 +1,5 @@
 import React from 'react';
-import { render, screen } from '@testing-library/react';
+import { act, render, screen } from '@testing-library/react';
 import { ColorPickerPopover } from './ColorPickerPopover';
 import { createTheme } from '@grafana/data';
 import userEvent from '@testing-library/user-event';
@ -9,13 +9,17 @@ describe('ColorPickerPopover', () => {

  it('should be tabbable', () => {
    render(<ColorPickerPopover color={'red'} onChange={() => {}} />);
-    const color = screen.getByRole('button', { name: 'red color' });
+    const color = screen.getByRole('button', { name: 'super-light-red color' });
    const customTab = screen.getByRole('button', { name: 'Custom' });

-    userEvent.tab();
+    act(() => {
+      userEvent.tab();
+    });
    expect(customTab).toHaveFocus();

-    userEvent.tab();
+    act(() => {
+      userEvent.tab();
+    });
    expect(color).toHaveFocus();
  });

@ -28,8 +32,10 @@ describe('ColorPickerPopover', () => {
      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 #000000');
+      act(() => {
+        userEvent.click(colorSwatchWrapper[0]);
+      });
+      expect(color).toHaveStyle('box-shadow: inset 0 0 0 2px #73BF69,inset 0 0 0 4px #000000');
    });
  });

@ -39,7 +45,9 @@ describe('ColorPickerPopover', () => {
    it('should pass hex color value to onChange prop by default', () => {
      render(<ColorPickerPopover color={'red'} onChange={onChangeSpy} />);
      const color = screen.getByRole('button', { name: 'red color' });
-      userEvent.click(color);
+      act(() => {
+        userEvent.click(color);
+      });

      expect(onChangeSpy).toBeCalledTimes(1);
      expect(onChangeSpy).toBeCalledWith(theme.visualization.getColorByName('red'));
@ -48,7 +56,9 @@ describe('ColorPickerPopover', () => {
    it('should pass color name to onChange prop when named colors enabled', () => {
      render(<ColorPickerPopover color={'red'} enableNamedColors onChange={onChangeSpy} />);
      const color = screen.getByRole('button', { name: 'red color' });
-      userEvent.click(color);
+      act(() => {
+        userEvent.click(color);
+      });

      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
@ -144,6 +144,7 @@ const getStyles = stylesFactory((theme: GrafanaTheme2) => {
      border-radius: ${theme.shape.borderRadius()};
      box-shadow: ${theme.shadows.z3};
      background: ${theme.colors.background.primary};
+      border: 1px solid ${theme.colors.border.medium};

      .ColorPickerPopover__tab {
        width: 50%;
@ -151,8 +152,14 @@ const getStyles = stylesFactory((theme: GrafanaTheme2) => {
        padding: ${theme.spacing(1, 0)};
        background: ${theme.colors.background.secondary};
        color: ${theme.colors.text.secondary};
+        font-size: ${theme.typography.bodySmall.fontSize};
        cursor: pointer;
        border: none;
+
+        &:focus:not(:focus-visible) {
+          outline: none;
+          box-shadow: none;
+        }
      }

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

 /** @internal */
 export enum ColorSwatchVariant {
@ -22,51 +24,64 @@ export interface Props extends React.HTMLAttributes<HTMLDivElement> {
 export const ColorSwatch = React.forwardRef<HTMLDivElement, Props>(
  ({ 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 styles = getStyles(theme, variant, color, isFocusVisible, isSelected);
    const hasLabel = !!label;
-    const swatchSize = isSmall ? '16px' : '32px';
-
-    const swatchStyles: CSSProperties = {
-      width: swatchSize,
-      height: swatchSize,
-      border: 'none',
-      borderRadius: '50%',
-      background: `${color}`,
-      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',
-    };
-
-    if (tc.getAlpha() < 0.1) {
-      swatchStyles.border = `2px solid ${theme.colors.border.medium}`;
-    }
-
    const colorLabel = `${ariaLabel || label} color`;

    return (
-      <div
-        ref={ref}
-        style={{
-          display: 'flex',
-          alignItems: 'center',
-          cursor: 'pointer',
-        }}
-        data-testid={selectors.components.ColorSwatch.name}
-        {...otherProps}
-      >
-        {hasLabel && <span>{label}</span>}
-        <button style={swatchStyles} {...focusProps} aria-label={colorLabel} />
+      <div ref={ref} className={styles.wrapper} data-testid={selectors.components.ColorSwatch.name} {...otherProps}>
+        {hasLabel && <span className={styles.label}>{label}</span>}
+        <button className={styles.swatch} {...focusProps} aria-label={colorLabel} />
      </div>
    );
  }
 );

+const getStyles = (
+  theme: GrafanaTheme2,
+  variant: ColorSwatchVariant,
+  color: string,
+  isFocusVisible: boolean,
+  isSelected?: boolean
+) => {
+  const tc = tinycolor(color);
+  const isSmall = variant === ColorSwatchVariant.Small;
+  const swatchSize = isSmall ? '16px' : '32px';
+  let border = 'none';
+
+  if (tc.getAlpha() < 0.1) {
+    border = `2px solid ${theme.colors.border.medium}`;
+  }
+
+  return {
+    wrapper: css({
+      display: 'flex',
+      alignItems: 'center',
+      cursor: 'pointer',
+    }),
+    label: css({
+      marginRight: theme.spacing(1),
+    }),
+    swatch: css({
+      width: swatchSize,
+      height: swatchSize,
+      background: `${color}`,
+      border,
+      borderRadius: '50%',
+      outlineOffset: '1px',
+      outline: isFocusVisible ? `2px solid  ${theme.colors.primary.main}` : 'none',
+      boxShadow: isSelected
+        ? `inset 0 0 0 2px ${color}, inset 0 0 0 4px ${theme.colors.getContrastText(color)}`
+        : 'none',
+      transition: theme.transitions.create(['transform'], {
+        duration: theme.transitions.duration.short,
+      }),
+      '&:hover': {
+        transform: 'scale(1.1)',
+      },
+    }),
+  };
+};
+
 ColorSwatch.displayName = 'ColorSwatch';
--- a/packages/grafana-ui/src/components/ColorPicker/NamedColorsGroup.tsx
+++ b/packages/grafana-ui/src/components/ColorPicker/NamedColorsGroup.tsx
@ -19,40 +19,23 @@ const NamedColorsGroup: FunctionComponent<NamedColorsGroupProps> = ({
  onColorSelect,
  ...otherProps
 }) => {
-  const primaryShade = hue.shades.find((shade) => shade.primary)!;
  const label = upperFirst(hue.name);
  const styles = useStyles2(getStyles);

  return (
-    <div className={styles.colorRow} style={{}}>
+    <div className={styles.colorRow}>
      <div className={styles.colorLabel}>{label}</div>
      <div {...otherProps} className={styles.swatchRow}>
-        {primaryShade && (
+        {hue.shades.map((shade) => (
          <ColorSwatch
-            key={primaryShade.name}
-            aria-label={primaryShade.name}
-            isSelected={primaryShade.name === selectedColor}
-            variant={ColorSwatchVariant.Large}
-            color={primaryShade.color}
-            onClick={() => onColorSelect(primaryShade.name)}
+            key={shade.name}
+            aria-label={shade.name}
+            variant={shade.primary ? ColorSwatchVariant.Large : ColorSwatchVariant.Small}
+            isSelected={shade.name === selectedColor}
+            color={shade.color}
+            onClick={() => onColorSelect(shade.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>
  );
@ -66,7 +49,7 @@ const getStyles = (theme: GrafanaTheme2) => {
      display: grid;
      grid-template-columns: 25% 1fr;
      grid-column-gap: ${theme.spacing(2)};
-      padding: ${theme.spacing(1, 0)};
+      padding: ${theme.spacing(0.5, 0)};

      &:hover {
        background: ${theme.colors.background.secondary};
@ -74,14 +57,15 @@ const getStyles = (theme: GrafanaTheme2) => {
    `,
    colorLabel: css`
      padding-left: ${theme.spacing(2)};
+      display: flex;
+      align-items: center;
    `,
    swatchRow: css`
      display: flex;
-      flex-direction: row;
-    `,
-    swatchContainer: css`
-      display: flex;
-      margin-top: 8px;
+      gap: ${theme.spacing(1)};
+      align-items: center;
+      justify-content: space-around;
+      flex-direction: row-reverse;
    `,
  };
 };
--- a/packages/grafana-ui/src/components/ColorPicker/NamedColorsPalette.tsx
+++ b/packages/grafana-ui/src/components/ColorPicker/NamedColorsPalette.tsx
@ -1,6 +1,5 @@
 import React from 'react';
 import NamedColorsGroup from './NamedColorsGroup';
-import { VerticalGroup } from '../Layout/Layout';
 import { ColorSwatch } from './ColorSwatch';
 import { useStyles2, useTheme2 } from '../../themes/ThemeContext';
 import { GrafanaTheme2 } from '@grafana/data';
@ -21,9 +20,9 @@ export const NamedColorsPalette = ({ color, onChange }: NamedColorsPaletteProps)
  }

  return (
-    <VerticalGroup spacing="md">
-      <div className={styles.popoverContainer}>{swatches}</div>
-      <div className={styles.container}>
+    <>
+      <div className={styles.swatches}>{swatches}</div>
+      <div className={styles.extraColors}>
        <ColorSwatch
          isSelected={color === 'transparent'}
          color={'rgba(0,0,0,0)'}
@ -37,21 +36,24 @@ export const NamedColorsPalette = ({ color, onChange }: NamedColorsPaletteProps)
          onClick={() => onChange('text')}
        />
      </div>
-    </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)};
+      display: flex;
+      flex-direction: column;
    `,
-    popoverContainer: css`
+    extraColors: css`
+      display: flex;
+      align-items: center;
+      justify-content: space-around;
+      gap: ${theme.spacing(1)};
+      padding: ${theme.spacing(1, 0)};
+    `,
+    swatches: css`
      display: grid;
      flex-grow: 1;
    `,