fixing colorpicker accessibility

pinterest · Jun 6, 2024 · 7637031 · 7637031
1 parent 605e34f
commit 7637031
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 62 deletions.
diff --git a/docs/examples/buttontoggle/changeLabelsDo.tsx b/docs/examples/buttontoggle/changeLabelsDo.tsx
@@ -9,7 +9,7 @@ export default function Example() {
       <ButtonGroup>
         <ButtonToggle
           color="red"
-          onClick={(value) => setSaved(!value)}
+          onClick={() => setSaved((value) => !value)}
           selected={saved}
           size="lg"
           text={saved ? 'Saved' : 'Save'}

diff --git a/docs/examples/buttontoggle/colorpicker.tsx b/docs/examples/buttontoggle/colorpicker.tsx
@@ -8,7 +8,7 @@ export default function Example() {
     <Flex alignItems="center" height="100%" justifyContent="center" width="100%">
       <ButtonToggle
         color={['#F0E3DC', '#F8D7D8', '#F2D7BE', '#F7C3AF']}
-        onClick={(value) => setSelected(!value)}
+        onClick={() => setSelected((value) => !value)}
         selected={selected}
         size="lg"
       />

diff --git a/packages/gestalt/src/ButtonToggle.css b/packages/gestalt/src/ButtonToggle.css
@@ -32,6 +32,8 @@
 .colorPickerButton {
   composes: noBorder from "./Borders.css";
   background-color: var(--color-transparent);
+  border-radius: var(--rounding-pill);
+  padding: 0;
 }
 
 .childrenDiv {

diff --git a/packages/gestalt/src/ButtonToggle.tsx b/packages/gestalt/src/ButtonToggle.tsx
@@ -171,15 +171,36 @@ const ButtonToggleWithForwardRef = forwardRef<HTMLButtonElement, Props>(function
   const bgColor: 'red' | 'transparent' = color instanceof Array ? 'red' : color;
   if (color instanceof Array) {
     return (
-      <ColorPickerButton
-        colors={color}
-        isSelected={selected}
+      <button
+        ref={innerRef}
+        aria-controls={accessibilityControls}
+        aria-label={accessibilityLabel}
+        aria-pressed={selected}
+        className={classnames(sharedTypeClasses, styles.colorPickerButton)}
+        data-test-id={dataTestId}
+        disabled={disabled}
+        onBlur={(event) => {
+          onBlur?.({ event });
+        }}
         onClick={(event) => {
           buttonToggleHandlers?.onClick?.();
-          onClick?.(event);
+          onClick?.({ event });
+        }}
+        onFocus={(event) => {
+          onFocus?.({ event });
         }}
-        size={size}
-      />
+        onMouseDown={handleMouseDown}
+        onMouseUp={handleMouseUp}
+        onTouchCancel={handleTouchCancel}
+        onTouchEnd={handleTouchEnd}
+        // @ts-expect-error - TS2322 - Type '(arg1: TouchEvent<HTMLDivElement>) => void' is not assignable to type 'TouchEventHandler<HTMLButtonElement>'.
+        onTouchMove={handleTouchMove}
+        // @ts-expect-error - TS2322 - Type '(arg1: TouchEvent<HTMLDivElement>) => void' is not assignable to type 'TouchEventHandler<HTMLButtonElement>'.
+        onTouchStart={handleTouchStart}
+        type="button"
+      >
+        <ColorPickerButton colors={color} selected={selected} size={size} />
+      </button>
     );
   }
 

diff --git a/packages/gestalt/src/ButtonToggle/ColorPickerButton.tsx b/packages/gestalt/src/ButtonToggle/ColorPickerButton.tsx
@@ -1,8 +1,5 @@
-import { ComponentProps, useState } from 'react';
+import { useState } from 'react';
 import Box from '../Box';
-import ButtonToggle from '../ButtonToggle';
-import Flex from '../Flex';
-import TapArea from '../TapArea';
 
 const BORDER_OFFSET_PX = 4;
 
@@ -18,81 +15,70 @@ const widths = {
   lg: 88,
 };
 
-function getBorderColor(isHovered: boolean, isSelected: boolean) {
+function getBorderColor(hovered: boolean, selected: boolean) {
   // Selection state takes precedence
-  if (isSelected) {
+  if (selected) {
     return 'selected';
   }
-  if (isHovered) {
+  if (hovered) {
     return 'tertiary';
   }
   return undefined;
 }
 
 export type Props = {
   colors: ReadonlyArray<string>;
-  isSelected: boolean;
-  onClick?: ComponentProps<typeof ButtonToggle>['onClick'];
+  selected: boolean;
   size: 'sm' | 'md' | 'lg';
 };
 
-export default function ColorPickerButton({ colors, isSelected, onClick, size }: Props) {
+export default function ColorPickerButton({ colors, selected, size }: Props) {
   const filtersContainerHeightPx = heights[size] + BORDER_OFFSET_PX * 2;
   const filtersContainerWidthPx = widths[size] + BORDER_OFFSET_PX * 2;
-
   const [hovered, setHovered] = useState(false);
 
   return (
-    <Flex alignItems="center" height={filtersContainerHeightPx} justifyContent="start">
-      <TapArea
-        mouseCursor="pointer"
-        onMouseEnter={() => setHovered(true)}
-        onMouseLeave={() => setHovered(false)}
-        // @ts-expect-error - TS2322 - Type '(arg1: TouchEvent<HTMLDivElement>) => void' is not assignable to type 'TouchEventHandler<HTMLButtonElement>'.
-        onTap={onClick}
+    <Box
+      alignItems="center"
+      color={getBorderColor(hovered, selected)}
+      display="flex"
+      height={filtersContainerHeightPx}
+      justifyContent="center"
+      onMouseEnter={() => setHovered(true)}
+      onMouseLeave={() => setHovered(false)}
+      rounding="pill"
+      width={filtersContainerWidthPx}
+    >
+      <Box
+        alignItems="center"
+        color="default"
+        display="flex"
+        height={filtersContainerHeightPx - BORDER_OFFSET_PX}
+        justifyContent="center"
         rounding="pill"
+        width={filtersContainerWidthPx - BORDER_OFFSET_PX}
       >
         <Box
-          alignItems="center"
-          color={getBorderColor(hovered, isSelected)}
           display="flex"
-          height={filtersContainerHeightPx}
-          justifyContent="center"
+          height={heights[size]}
+          overflow="hidden"
           rounding="pill"
-          width={filtersContainerWidthPx}
+          width={widths[size]}
+          wrap
         >
-          <Box
-            alignItems="center"
-            color="default"
-            display="flex"
-            height={filtersContainerHeightPx - BORDER_OFFSET_PX}
-            justifyContent="center"
-            rounding="pill"
-            width={filtersContainerWidthPx - BORDER_OFFSET_PX}
-          >
+          {colors.map((colorHex, index) => (
             <Box
-              display="flex"
-              height={heights[size]}
-              overflow="hidden"
-              rounding="pill"
-              width={widths[size]}
-              wrap
-            >
-              {colors.map((colorHex, index) => (
-                <Box
-                  // eslint-disable-next-line react/no-array-index-key
-                  key={`${colorHex}-${index}`}
-                  dangerouslySetInlineStyle={{
-                    __style: { backgroundColor: colorHex },
-                  }}
-                  height={heights[size] / 2}
-                  width={widths[size] / 2}
-                />
-              ))}
-            </Box>
-          </Box>
+              // eslint-disable-next-line react/no-array-index-key
+              key={`${colorHex}-${index}`}
+              dangerouslySetInlineStyle={{
+                __style: { backgroundColor: colorHex },
+              }}
+              height={heights[size] / 2}
+              width={widths[size] / 2}
+            />
+          ))}
         </Box>
-      </TapArea>
-    </Flex>
+      </Box>
+    </Box>
   );
 }