Button alignment and size bug fixes

.changeset/angry-spoons-grin.md

@@ -0,0 +1,9 @@
+---
+"@primer/react": patch
+---
+
+- Add a block prop for full width
+- Add alignContent prop to align content to center or start
+- Use control sizing CSS variable values (not using CSS vars just yet)
+- Use height over padding for more control over sizing
+

src/Button/Button.features.stories.tsx

@@ -1,4 +1,4 @@
-import {EyeIcon, HeartIcon} from '@primer/octicons-react'
+import {EyeIcon, TriangleDownIcon, HeartIcon} from '@primer/octicons-react'
 import React, {useState} from 'react'
 import {Button} from '.'
 
@@ -30,6 +30,10 @@ export const TrailingCounter = () => {
   )
 }
 
+export const TrailingAction = () => <Button trailingAction={TriangleDownIcon}>Trailing action</Button>
+
+export const Block = () => <Button block>Default</Button>
+
 export const Disabled = () => <Button disabled>Default</Button>
 
 export const Small = () => <Button size="small">Default</Button>

src/Button/Button.stories.tsx

@@ -1,5 +1,5 @@
 import React from 'react'
-import {EyeClosedIcon, EyeIcon, SearchIcon, XIcon, HeartIcon} from '@primer/octicons-react'
+import {EyeClosedIcon, EyeIcon, SearchIcon, TriangleDownIcon, XIcon, HeartIcon} from '@primer/octicons-react'
 import {Story, Meta} from '@storybook/react'
 import {Button} from '.'
 import {OcticonArgType} from '../utils/story-helpers'
@@ -21,18 +21,39 @@ export default {
     variant: {
       control: {
         type: 'radio',
-        options: ['default', 'primary', 'danger', 'invisible', 'outline'],
+        options: ['default', 'primary', 'danger', 'invisible'],
+      },
+    },
+    alignContent: {
+      control: {
+        type: 'radio',
+        options: ['center', 'start'],
+      },
+    },
+    block: {
+      control: {
+        type: 'boolean',
+      },
+    },
+    leadingVisual: OcticonArgType([EyeClosedIcon, EyeIcon, SearchIcon, XIcon, HeartIcon]),
+    trailingVisual: OcticonArgType([EyeClosedIcon, EyeIcon, SearchIcon, XIcon, HeartIcon]),
+    trailingAction: OcticonArgType([TriangleDownIcon]),
+    trailingVisualCount: {
+      control: {
+        type: 'number',
       },
     },
-    leadingIcon: OcticonArgType([EyeClosedIcon, EyeIcon, SearchIcon, XIcon, HeartIcon]),
-    trailingIcon: OcticonArgType([EyeClosedIcon, EyeIcon, SearchIcon, XIcon, HeartIcon]),
   },
   args: {
+    block: false,
     size: 'medium',
     disabled: false,
     variant: 'default',
-    trailingIcon: null,
-    leadingIcon: null,
+    alignContent: 'center',
+    trailingVisual: null,
+    leadingVisual: null,
+    trailingAction: null,
+    trailingVisualCount: undefined,
   },
 } as Meta<typeof Button>
 

src/Button/ButtonBase.tsx

@@ -4,32 +4,38 @@ import Box from '../Box'
 import {merge, SxProp} from '../sx'
 import {useTheme} from '../ThemeProvider'
 import {ButtonProps, StyledButton} from './types'
-import {getVariantStyles, getSizeStyles, getButtonStyles} from './styles'
+import {getVariantStyles, getButtonStyles, getAlignContentSize} from './styles'
 import {useRefObjectAsForwardedRef} from '../hooks/useRefObjectAsForwardedRef'
 declare let __DEV__: boolean
 
 const defaultSxProp = {}
-const iconWrapStyles = {
-  display: 'inline-block',
-}
-const trailingIconStyles = {
-  ...iconWrapStyles,
-  ml: 2,
-}
-
 const ButtonBase = forwardRef<HTMLElement, ButtonProps>(
   ({children, as: Component = 'button', sx: sxProp = defaultSxProp, ...props}, forwardedRef): JSX.Element => {
-    const {leadingIcon: LeadingIcon, trailingIcon: TrailingIcon, variant = 'default', size = 'medium', ...rest} = props
+    const {
+      leadingIcon: LeadingIcon,
+      trailingIcon: TrailingIcon,
+      trailingAction: TrailingAction,
+      variant = 'default',
+      size = 'medium',
+      alignContent = 'center',
+      block = false,
+      ...rest
+    } = props
+
     const innerRef = React.useRef<HTMLElement>(null)
     useRefObjectAsForwardedRef(forwardedRef, innerRef)
 
     const {theme} = useTheme()
     const baseStyles = useMemo(() => {
-      return merge.all([getButtonStyles(theme), getSizeStyles(size, variant, false), getVariantStyles(variant, theme)])
-    }, [theme, size, variant])
+      return merge.all([getButtonStyles(theme), getVariantStyles(variant, theme)])
+    }, [theme, variant])
     const sxStyles = useMemo(() => {
       return merge(baseStyles, sxProp as SxProp)
     }, [baseStyles, sxProp])
+    const iconWrapStyles = {
+      display: 'flex',
+      pointerEvents: 'none',
+    }
 
     React.useEffect(() => {
       if (!(innerRef.current instanceof HTMLButtonElement) && !(innerRef.current instanceof HTMLAnchorElement)) {
@@ -41,16 +47,30 @@ const ButtonBase = forwardRef<HTMLElement, ButtonProps>(
     }, [innerRef])
 
     return (
-      <StyledButton as={Component} sx={sxStyles} {...rest} ref={innerRef}>
-        {LeadingIcon && (
-          <Box as="span" data-component="leadingIcon" sx={iconWrapStyles}>
-            <LeadingIcon />
-          </Box>
-        )}
-        {children && <span data-component="text">{children}</span>}
-        {TrailingIcon && (
-          <Box as="span" data-component="trailingIcon" sx={trailingIconStyles}>
-            <TrailingIcon />
+      <StyledButton
+        as={Component}
+        sx={sxStyles}
+        {...rest}
+        ref={innerRef}
+        data-component={block ? 'block' : null}
+        data-size={size === 'small' || size === 'large' ? size : undefined}
+      >
+        <Box as="span" data-component="buttonContent" sx={getAlignContentSize(alignContent)}>
+          {LeadingIcon && (
+            <Box as="span" data-component="leadingVisual" sx={{...iconWrapStyles}}>
+              <LeadingIcon />
+            </Box>
+          )}
+          {children && <span data-component="text">{children}</span>}
+          {TrailingIcon && (
+            <Box as="span" data-component="trailingVisual" sx={{...iconWrapStyles}}>
+              <TrailingIcon />
+            </Box>
+          )}
+        </Box>
+        {TrailingAction && (
+          <Box as="span" data-component="trailingAction" sx={{...iconWrapStyles}}>
+            <TrailingAction />
           </Box>
         )}
       </StyledButton>

src/Button/IconButton.tsx

@@ -1,24 +1,22 @@
 import React, {forwardRef} from 'react'
 import {merge, SxProp} from '../sx'
 import {useTheme} from '../ThemeProvider'
-import Box from '../Box'
 import {IconButtonProps, StyledButton} from './types'
-import {getBaseStyles, getSizeStyles, getVariantStyles} from './styles'
+import {getBaseStyles, getVariantStyles} from './styles'
 
 const IconButton = forwardRef<HTMLButtonElement, IconButtonProps>((props, forwardedRef): JSX.Element => {
   const {variant = 'default', size = 'medium', sx: sxProp = {}, icon: Icon, ...rest} = props
   const {theme} = useTheme()
-  const sxStyles = merge.all([
-    getBaseStyles(theme),
-    getSizeStyles(size, variant, true),
-    getVariantStyles(variant, theme),
-    sxProp as SxProp,
-  ])
+  const sxStyles = merge.all([getBaseStyles(theme), getVariantStyles(variant, theme), sxProp as SxProp])
   return (
-    <StyledButton sx={sxStyles} {...rest} ref={forwardedRef}>
-      <Box as="span" sx={{display: 'inline-block'}}>
-        <Icon />
-      </Box>
+    <StyledButton
+      data-component="IconButton"
+      data-size={size === 'small' || size === 'large' ? size : undefined}
+      sx={sxStyles}
+      {...rest}
+      ref={forwardedRef}
+    >
+      <Icon />
     </StyledButton>
   )
 })

src/Button/LinkButton.features.stories.tsx

@@ -1,4 +1,4 @@
-import {EyeIcon, HeartIcon} from '@primer/octicons-react'
+import {EyeIcon, ChevronRightIcon, HeartIcon} from '@primer/octicons-react'
 import React, {forwardRef} from 'react'
 import {Button} from '.'
 
@@ -48,6 +48,18 @@ export const TrailingVisual = () => (
   </Button>
 )
 
+export const TrailingAction = () => (
+  <Button as="a" href="/" trailingAction={ChevronRightIcon}>
+    Trailing action
+  </Button>
+)
+
+export const Block = () => (
+  <Button as="a" href="/" block>
+    Default
+  </Button>
+)
+
 export const Small = () => (
   <Button as="a" href="/" size="small">
     Default

src/Button/LinkButton.stories.tsx

@@ -1,5 +1,5 @@
 import React from 'react'
-import {EyeClosedIcon, EyeIcon, SearchIcon, XIcon, HeartIcon} from '@primer/octicons-react'
+import {EyeClosedIcon, EyeIcon, SearchIcon, XIcon, HeartIcon, ChevronRightIcon} from '@primer/octicons-react'
 import {Story, Meta} from '@storybook/react'
 import {Button} from '.'
 import {OcticonArgType} from '../utils/story-helpers'
@@ -19,15 +19,34 @@ export default {
         options: ['default', 'primary', 'danger', 'invisible', 'outline'],
       },
     },
-    leadingIcon: OcticonArgType([EyeClosedIcon, EyeIcon, SearchIcon, XIcon, HeartIcon]),
-    trailingIcon: OcticonArgType([EyeClosedIcon, EyeIcon, SearchIcon, XIcon, HeartIcon]),
+    alignContent: {
+      control: {
+        type: 'radio',
+        options: ['center', 'start'],
+      },
+    },
+    block: {
+      control: {
+        type: 'boolean',
+      },
+    },
+    leadingVisual: OcticonArgType([EyeClosedIcon, EyeIcon, SearchIcon, XIcon, HeartIcon]),
+    trailingVisual: OcticonArgType([EyeClosedIcon, EyeIcon, SearchIcon, XIcon, HeartIcon]),
+    trailingAction: OcticonArgType([ChevronRightIcon]),
+    trailingVisualCount: {
+      control: {
+        type: 'number',
+      },
+    },
     href: {control: 'text'},
   },
   args: {
+    block: false,
     size: 'medium',
     variant: 'default',
-    trailingIcon: null,
-    leadingIcon: null,
+    alignContent: 'center',
+    trailingVisual: null,
+    leadingVisual: null,
     href: '/',
   },
 } as Meta<typeof Button>

src/Button/LinkButton.tsx

@@ -8,20 +8,7 @@ type MyProps = LinkButtonProps & ButtonBaseProps
 
 const LinkButton = forwardRef<HTMLElement, MyProps>(
   ({children, as: Component = 'a', sx = {}, ...props}, forwardedRef): JSX.Element => {
-    const style = {
-      width: 'fit-content',
-      '&:hover:not([disabled])': {
-        textDecoration: 'underline',
-      },
-      // focus must come before :active so that the active box shadow overrides
-      '&:focus:not([disabled])': {
-        textDecoration: 'underline',
-      },
-      '&:active:not([disabled])': {
-        textDecoration: 'underline',
-      },
-    }
-    const sxStyle = merge.all([style, sx as SxProp])
+    const sxStyle = merge.all([sx as SxProp])
     return (
       <ButtonBase as={Component} ref={forwardedRef} sx={sxStyle} {...props}>
         {children}