TextInput trailing action design and API update (#2033)

* makes hover background appear inset * deprecates children prop - only icons should be used for text input inner actions * deprecates 'variant' prop for TextInputInnerAction * limits larger inner action tap target for coarse pointers * adds changeset * updates snapshots * prevents tooltip from appearing until button is actually hovered * fixes vertical alignment of SVG in IconButton when size !== 'medium' * addresses last bit of PR feedback * updates snapshots
primer · Apr 25, 2022 · 0d7a871 · 0d7a871
1 parent bbd436e
commit 0d7a871
Show file tree

Hide file tree

Showing 6 changed files with 292 additions and 235 deletions.
diff --git a/.changeset/tall-dancers-exercise.md b/.changeset/tall-dancers-exercise.md
@@ -0,0 +1,7 @@
+---
+'@primer/react': patch
+---
+
+- Text input inner action's hover bg should not touch the text input edges
+- Increases touch target area of the text input inner action button
+- Deprecated `children` and `variant` props on the `TextInputInnerAction` component, but they're **still supported for now**.
diff --git a/docs/content/TextInput.mdx b/docs/content/TextInput.mdx
@@ -111,37 +111,21 @@ render(<WithIconAndLoadingIndicator />)
 ### With trailing action
 
 ```jsx live
-<Box display="grid" sx={{gap: 3}}>
-  <FormControl>
-    <FormControl.Label>Icon action</FormControl.Label>
-    <TextInput
-      trailingAction={
-        <TextInput.Action
-          onClick={() => {
-            alert('clear input')
-          }}
-          icon={XIcon}
-          aria-label="Clear input"
-          sx={{color: 'fg.subtle'}}
-        />
-      }
-    />
-  </FormControl>
-  <FormControl>
-    <FormControl.Label>Text action</FormControl.Label>
-    <TextInput
-      trailingAction={
-        <TextInput.Action
-          onClick={() => {
-            alert('clear input')
-          }}
-        >
-          Clear
-        </TextInput.Action>
-      }
-    />
-  </FormControl>
-</Box>
+<FormControl>
+  <FormControl.Label>Icon action</FormControl.Label>
+  <TextInput
+    trailingAction={
+      <TextInput.Action
+        onClick={() => {
+          alert('clear input')
+        }}
+        icon={XIcon}
+        aria-label="Clear input"
+        sx={{color: 'fg.subtle'}}
+      />
+    }
+  />
+</FormControl>
 ```
 
 ### With error and warning states
@@ -315,7 +299,8 @@ render(<WithIconAndLoadingIndicator />)
   <PropsTableRow
     name="variant"
     type="'default' | 'primary' | 'invisible' | 'danger'"
-    description="Determine's the styles on a button"
+    description="(Deprecated so that only the 'invisible' variant is used) Determine's the styles on a button"
+    deprecated
   />
   <PropsTableBasePropRows
     elementType="button"

diff --git a/src/Button/styles.ts b/src/Button/styles.ts
@@ -230,6 +230,10 @@ export const getSizeStyles = (size = 'medium', variant: VariantType = 'default',
       fontSize = 1
   }
   if (iconOnly) {
+    // when `size !== 'medium'`, vertical alignment of the icon is thrown off
+    // because changing the font size draws an em-box that does not match the
+    // bounding box of the SVG
+    fontSize = 1
     paddingX = paddingY + 3 // to make it a square
   }
   if (variant === 'invisible') {

diff --git a/src/_TextInputInnerAction.tsx b/src/_TextInputInnerAction.tsx
@@ -5,18 +5,38 @@ import {ButtonProps} from './Button'
 import {BetterSystemStyleObject, merge, SxProp} from './sx'
 
 type TextInputActionProps = Omit<React.HTMLProps<HTMLButtonElement>, 'aria-label' | 'size'> & {
+  /** @deprecated Text input action buttons should only use icon buttons */
+  children?: React.ReactNode
   /** Text that appears in a tooltip. If an icon is passed, this is also used as the label used by assistive technologies. */
   ['aria-label']?: string
   /** The icon to render inside the button */
   icon?: React.FunctionComponent<IconProps>
   /**
+   * @deprecated Text input action buttons should only use the 'invisible' button variant
    * Determine's the styles on a button one of 'default' | 'primary' | 'invisible' | 'danger'
    */
   variant?: ButtonProps['variant']
 } & SxProp
 
 const invisibleButtonStyleOverrides = {
-  color: 'fg.default'
+  color: 'fg.default',
+  paddingTop: '2px',
+  paddingRight: '4px',
+  paddingBottom: '2px',
+  paddingLeft: '4px',
+  position: 'relative',
+
+  '@media (pointer: coarse)': {
+    ':after': {
+      content: '""',
+      position: 'absolute',
+      left: 0,
+      right: 0,
+      transform: 'translateY(-50%)',
+      top: '50%',
+      minHeight: '44px'
+    }
+  }
 }
 
 const ConditionalTooltip: React.FC<{
@@ -44,22 +64,25 @@ const ConditionalTooltip: React.FC<{
 const TextInputAction = forwardRef<HTMLButtonElement, TextInputActionProps>(
   ({'aria-label': ariaLabel, children, icon, sx: sxProp, variant, ...rest}, forwardedRef) => {
     const sx =
-      variant === 'invisible' ? merge<BetterSystemStyleObject>(invisibleButtonStyleOverrides, sxProp || {}) : sxProp
+      variant === 'invisible'
+        ? merge<BetterSystemStyleObject>(invisibleButtonStyleOverrides, sxProp || {})
+        : sxProp || {}
 
     if ((icon && !ariaLabel) || (!children && !ariaLabel)) {
       // eslint-disable-next-line no-console
       console.warn('Use the `aria-label` prop to provide an accessible label for assistive technology')
     }
 
     return (
-      <Box as="span" className="TextInput-action">
+      <Box as="span" className="TextInput-action" margin={1}>
         {icon && !children ? (
           <Tooltip aria-label={ariaLabel}>
             <IconButton
               variant={variant}
               type="button"
               icon={icon}
               aria-label={ariaLabel}
+              size="small"
               sx={sx}
               {...rest}
               ref={forwardedRef}