Button: Make button visuals overridable by sx (#3683)

* Button: Make button visuals overridable by sx

* add changeset

* update comments

* test(vrt): update snapshots

* add tests

* update bits

* same color name as storybook

* test(vrt): update snapshots

* override icon styles with css custom var

* stage the export

* update snapshots

---------

Co-authored-by: broccolinisoup <[email protected]>

.changeset/purple-panthers-accept.md

@@ -0,0 +1,7 @@
+---
+'@primer/react': patch
+---
+
+Button: Allow leadingIcon, trailingIcon, trailingAction to be overridable with sx
+
+<!-- Changed components: Button -->

src/Button/Button.dev.stories.tsx

@@ -1,6 +1,7 @@
-import {SearchIcon, TriangleDownIcon, EyeIcon} from '@primer/octicons-react'
+import {SearchIcon, TriangleDownIcon, EyeIcon, IssueClosedIcon} from '@primer/octicons-react'
 import React from 'react'
 import {Button, IconButton} from '.'
+import {default as Text} from '../Text'
 
 export default {
   title: 'Components/Button/DevOnly',
@@ -67,8 +68,11 @@ export const TestSxProp = () => {
       >
         Red
       </Button>
-      <Button leadingIcon={SearchIcon} variant="invisible" sx={{color: 'firebrick'}}>
-        Red
+      <Button variant="invisible" sx={{color: 'firebrick'}}>
+        Invariant color overridden
+      </Button>
+      <Button leadingIcon={IssueClosedIcon} sx={{color: 'done.fg'}}>
+        <Text sx={{color: 'fg.default'}}>Close issue</Text>
       </Button>
       <Button
         size="small"

src/Button/Button.tsx

@@ -3,10 +3,11 @@ import {ButtonProps} from './types'
 import {ButtonBase} from './ButtonBase'
 import {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic'
 import {defaultSxProp} from '../utils/defaultSxProp'
-import {BetterSystemStyleObject} from '../sx'
+import {BetterSystemStyleObject, CSSCustomProperties} from '../sx'
 
 const ButtonComponent = forwardRef(({children, sx: sxProp = defaultSxProp, ...props}, forwardedRef): JSX.Element => {
   let sxStyles = sxProp
+  const style: CSSCustomProperties = {}
   const leadingVisual = props.leadingVisual ?? props.leadingIcon
   const trailingVisual = props.trailingVisual ?? props.trailingIcon
 
@@ -15,10 +16,14 @@ const ButtonComponent = forwardRef(({children, sx: sxProp = defaultSxProp, ...pr
 
   if (sxProp !== null && Object.keys(sxProp).length > 0) {
     sxStyles = generateCustomSxProp({block, size, leadingVisual, trailingVisual, trailingAction}, sxProp)
+
+    // @ts-ignore sxProp can have color attribute
+    const {color} = sxProp
+    if (color) style['--button-color'] = color
   }
 
   return (
-    <ButtonBase ref={forwardedRef} as="button" sx={sxStyles} type="button" {...props}>
+    <ButtonBase ref={forwardedRef} as="button" sx={sxStyles} style={style} type="button" {...props}>
       {children}
     </ButtonBase>
   )
@@ -71,11 +76,10 @@ export function generateCustomSxProp(
   // Possible data attributes: data-size, data-block, data-no-visuals
   const size = props.size && props.size !== 'medium' ? `[data-size="${props.size}"]` : '' // medium is a default size therefore it doesn't have a data attribute that used for styling
   const block = props.block ? `[data-block="block"]` : ''
-  const noVisuals =
-    props.leadingVisual || props.trailingVisual || props.trailingAction ? '' : '[data-no-visuals="true"]'
+  const noVisuals = props.leadingVisual || props.trailingVisual || props.trailingAction ? '' : '[data-no-visuals]'
 
-  // this is custom selector. We need to make sure we add the data attributes to the base css class (& -> &[data-attributename="value"]])
-  const cssSelector = `&${size}${block}${noVisuals}` // &[data-size="small"][data-block="block"][data-no-visuals="true"]
+  // this is a custom selector. We need to make sure we add the data attributes to the base css class (& -> &[data-attributename="value"]])
+  const cssSelector = `&${size}${block}${noVisuals}` // &[data-size="small"][data-block="block"][data-no-visuals]
 
   const customSxProp: {
     [key: string]: BetterSystemStyleObject

src/Button/__tests__/__snapshots__/Button.test.tsx.snap

@@ -212,7 +212,7 @@ exports[`Button renders consistently 1`] = `
 .c0 [data-component="leadingVisual"],
 .c0 [data-component="trailingVisual"],
 .c0 [data-component="trailingAction"] {
-  color: #656d76;
+  color: var(--button-color,#656d76);
 }
 
 @media (forced-colors:active) {
@@ -225,6 +225,7 @@ exports[`Button renders consistently 1`] = `
   className="c0"
   data-block={null}
   data-no-visuals={true}
+  style={{}}
   type="button"
 >
   <span
@@ -445,7 +446,7 @@ exports[`Button respects block prop 1`] = `
 .c0 [data-component="leadingVisual"],
 .c0 [data-component="trailingVisual"],
 .c0 [data-component="trailingAction"] {
-  color: fg.muted;
+  color: var(--button-color,undefined);
 }
 
 @media (forced-colors:active) {
@@ -684,7 +685,7 @@ exports[`Button respects the alignContent prop 1`] = `
 .c0 [data-component="leadingVisual"],
 .c0 [data-component="trailingVisual"],
 .c0 [data-component="trailingAction"] {
-  color: fg.muted;
+  color: var(--button-color,undefined);
 }
 
 @media (forced-colors:active) {
@@ -922,7 +923,7 @@ exports[`Button respects the large size prop 1`] = `
 .c0 [data-component="leadingVisual"],
 .c0 [data-component="trailingVisual"],
 .c0 [data-component="trailingAction"] {
-  color: fg.muted;
+  color: var(--button-color,undefined);
 }
 
 @media (forced-colors:active) {
@@ -1161,7 +1162,7 @@ exports[`Button respects the small size prop 1`] = `
 .c0 [data-component="leadingVisual"],
 .c0 [data-component="trailingVisual"],
 .c0 [data-component="trailingAction"] {
-  color: fg.muted;
+  color: var(--button-color,undefined);
 }
 
 @media (forced-colors:active) {

src/Button/styles.ts

@@ -26,7 +26,7 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme
         borderColor: 'btn.activeBorder',
       },
       '[data-component="leadingVisual"], [data-component="trailingVisual"], [data-component="trailingAction"]': {
-        color: 'fg.muted',
+        color: `var(--button-color, ${theme?.colors.fg.muted})`,
       },
     },
     primary: {

src/__tests__/__snapshots__/ActionMenu.test.tsx.snap

@@ -226,7 +226,7 @@ exports[`ActionMenu renders consistently 1`] = `
 .c1 [data-component="leadingVisual"],
 .c1 [data-component="trailingVisual"],
 .c1 [data-component="trailingAction"] {
-  color: #656d76;
+  color: var(--button-color,#656d76);
 }
 
 @media (forced-colors:active) {
@@ -251,6 +251,7 @@ exports[`ActionMenu renders consistently 1`] = `
     id="react-aria-1"
     onClick={[Function]}
     onKeyDown={[Function]}
+    style={{}}
     tabIndex={0}
     type="button"
   >

src/__tests__/__snapshots__/TextInput.test.tsx.snap

@@ -1868,15 +1868,15 @@ exports[`TextInput renders trailingAction icon button 1`] = `
   color: inherit;
 }
 
-.c4[data-size="small"][data-no-visuals="true"] {
+.c4[data-size="small"][data-no-visuals] {
   padding-top: 2px;
   padding-right: 4px;
   padding-bottom: 2px;
   padding-left: 4px;
   position: relative;
 }
 
-.c4[data-size="small"][data-no-visuals="true"][data-component="IconButton"] {
+.c4[data-size="small"][data-no-visuals][data-component="IconButton"] {
   width: var(--inner-action-size);
   height: var(--inner-action-size);
 }
@@ -2202,7 +2202,7 @@ exports[`TextInput renders trailingAction icon button 1`] = `
 }
 
 @media (pointer:coarse) {
-  .c4[data-size="small"][data-no-visuals="true"]:after {
+  .c4[data-size="small"][data-no-visuals]:after {
     content: "";
     position: absolute;
     left: 0;
@@ -2505,6 +2505,16 @@ exports[`TextInput renders trailingAction text button 1`] = `
 
 .c3[data-no-visuals] {
   color: #0969da;
+  padding-top: 2px;
+  padding-right: 4px;
+  padding-bottom: 2px;
+  padding-left: 4px;
+  position: relative;
+}
+
+.c3[data-no-visuals][data-component="IconButton"] {
+  width: var(--inner-action-size);
+  height: var(--inner-action-size);
 }
 
 .c3:has([data-component="ButtonCounter"]) {
@@ -2519,19 +2529,6 @@ exports[`TextInput renders trailingAction text button 1`] = `
   color: inherit;
 }
 
-.c3[data-no-visuals="true"] {
-  padding-top: 2px;
-  padding-right: 4px;
-  padding-bottom: 2px;
-  padding-left: 4px;
-  position: relative;
-}
-
-.c3[data-no-visuals="true"][data-component="IconButton"] {
-  width: var(--inner-action-size);
-  height: var(--inner-action-size);
-}
-
 .c0 {
   font-size: 14px;
   line-height: 20px;
@@ -2628,7 +2625,7 @@ exports[`TextInput renders trailingAction text button 1`] = `
 }
 
 @media (pointer:coarse) {
-  .c3[data-no-visuals="true"]:after {
+  .c3[data-no-visuals]:after {
     content: "";
     position: absolute;
     left: 0;
@@ -2669,6 +2666,7 @@ exports[`TextInput renders trailingAction text button 1`] = `
       data-block={null}
       data-no-visuals={true}
       onClick={[MockFunction]}
+      style={{}}
       type="button"
     >
       <span
@@ -2910,6 +2908,16 @@ exports[`TextInput renders trailingAction text button with a tooltip 1`] = `
 
 .c4[data-no-visuals] {
   color: #0969da;
+  padding-top: 2px;
+  padding-right: 4px;
+  padding-bottom: 2px;
+  padding-left: 4px;
+  position: relative;
+}
+
+.c4[data-no-visuals][data-component="IconButton"] {
+  width: var(--inner-action-size);
+  height: var(--inner-action-size);
 }
 
 .c4:has([data-component="ButtonCounter"]) {
@@ -2924,19 +2932,6 @@ exports[`TextInput renders trailingAction text button with a tooltip 1`] = `
   color: inherit;
 }
 
-.c4[data-no-visuals="true"] {
-  padding-top: 2px;
-  padding-right: 4px;
-  padding-bottom: 2px;
-  padding-left: 4px;
-  position: relative;
-}
-
-.c4[data-no-visuals="true"][data-component="IconButton"] {
-  width: var(--inner-action-size);
-  height: var(--inner-action-size);
-}
-
 .c0 {
   font-size: 14px;
   line-height: 20px;
@@ -3259,7 +3254,7 @@ exports[`TextInput renders trailingAction text button with a tooltip 1`] = `
 }
 
 @media (pointer:coarse) {
-  .c4[data-no-visuals="true"]:after {
+  .c4[data-no-visuals]:after {
     content: "";
     position: absolute;
     left: 0;
@@ -3305,6 +3300,7 @@ exports[`TextInput renders trailingAction text button with a tooltip 1`] = `
         data-block={null}
         data-no-visuals={true}
         onClick={[MockFunction]}
+        style={{}}
         type="button"
       >
         <span

src/sx.ts

@@ -14,7 +14,7 @@ export type BetterCssProperties = {
 }
 
 // Support CSS custom properties in the `sx` prop
-type CSSCustomProperties = {
+export type CSSCustomProperties = {
   [key: `--${string}`]: string | number
 }