Fix icon adornment outline (#1664)

Previously, it was too tight around the icon, so we removed it entirely. That causes accessibility issues, though, so this PR adds it back and adds an offset so that it doesn't look cramped.
penumbra-zone · Aug 9, 2024 · 6d644f8 · 6d644f8
1 parent 2a76fce
commit 6d644f8
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 13 deletions.
diff --git a/packages/ui/src/Button/index.stories.tsx b/packages/ui/src/Button/index.stories.tsx
@@ -1,16 +1,16 @@
 import type { Meta, StoryObj } from '@storybook/react';
 
 import { Button } from '.';
-import { ArrowLeftRight, Check } from 'lucide-react';
+import { ArrowLeftRight, Check, Copy } from 'lucide-react';
 
 const meta: Meta<typeof Button> = {
   component: Button,
   tags: ['autodocs', '!dev', 'density'],
   argTypes: {
     icon: {
       control: 'select',
-      options: ['None', 'Check', 'ArrowLeftRight'],
-      mapping: { None: undefined, Check, ArrowLeftRight },
+      options: ['None', 'Copy', 'Check', 'ArrowLeftRight'],
+      mapping: { None: undefined, Copy, Check, ArrowLeftRight },
     },
     iconOnly: {
       options: ['true', 'false', 'adornment'],
@@ -28,7 +28,7 @@ export const Basic: Story = {
     children: 'Save',
     actionType: 'default',
     disabled: false,
-    icon: Check,
+    icon: Copy,
     iconOnly: false,
     type: 'button',
   },

diff --git a/packages/ui/src/Button/index.tsx b/packages/ui/src/Button/index.tsx
@@ -53,6 +53,7 @@ interface StyledButtonProps {
   $priority: Priority;
   $density: Density;
   $getFocusOutlineColor: (theme: DefaultTheme) => string;
+  $getFocusOutlineOffset?: (theme: DefaultTheme) => string | undefined;
   $getBorderRadius: (theme: DefaultTheme) => string;
 }
 
@@ -72,7 +73,6 @@ const StyledButton = styled.button<StyledButtonProps>`
   align-items: center;
   justify-content: center;
   color: ${props => props.theme.color.neutral.contrast};
-  overflow: hidden;
   position: relative;
 
   ${props =>
@@ -82,12 +82,12 @@ const StyledButton = styled.button<StyledButtonProps>`
         ? sparse
         : compact}
 
-  ${focusOutline}
-  ${overlays}
-
   &::after {
     outline-offset: -2px;
   }
+
+  ${focusOutline}
+  ${overlays}
 `;
 
 interface BaseButtonProps {
@@ -188,11 +188,8 @@ export const Button = ({
       onClick={onClick}
       aria-label={iconOnly ? children : undefined}
       title={iconOnly ? children : undefined}
-      $getFocusOutlineColor={theme =>
-        iconOnly === 'adornment'
-          ? theme.color.base.transparent
-          : theme.color.action[outlineColorByActionType[actionType]]
-      }
+      $getFocusOutlineColor={theme => theme.color.action[outlineColorByActionType[actionType]]}
+      $getFocusOutlineOffset={() => (iconOnly === 'adornment' ? '0px' : undefined)}
       $getBorderRadius={theme =>
         density === 'sparse' && iconOnly !== 'adornment'
           ? theme.borderRadius.sm

diff --git a/packages/ui/src/utils/button.ts b/packages/ui/src/utils/button.ts
@@ -13,6 +13,7 @@ export const buttonBase = css`
 /** Adds a focus outline to a button using the `:focus-within` pseudoclass. */
 export const focusOutline = css<{
   $getFocusOutlineColor: (theme: DefaultTheme) => string;
+  $getFocusOutlineOffset?: (theme: DefaultTheme) => string | undefined;
   $getBorderRadius: (theme: DefaultTheme) => string;
 }>`
   position: relative;
@@ -26,6 +27,9 @@ export const focusOutline = css<{
     outline-width: 2px;
     outline-style: solid;
     outline-color: transparent;
+    ${props =>
+      props.$getFocusOutlineOffset?.(props.theme) &&
+      `outline-offset: ${props.$getFocusOutlineOffset(props.theme)};`}
     border-radius: ${props => props.$getBorderRadius(props.theme)};
 
     transition: outline-color 0.15s;