Merge branch 'main' of github.com:primer/react into next-major

.changeset/giant-bobcats-deliver.md

@@ -0,0 +1,7 @@
+---
+'@primer/react': patch
+---
+
+Update ActionList to place `id` on item with an ARIA role
+
+<!-- Changed components: ActionList -->

.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/ActionList/Item.tsx

@@ -190,10 +190,10 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
       [onSelect, disabled, afterSelect],
     )
 
-    // use props.id if provided, otherwise generate one.
-    const labelId = useId(id)
-    const inlineDescriptionId = useId(id && `${id}--inline-description`)
-    const blockDescriptionId = useId(id && `${id}--block-description`)
+    const itemId = useId(id)
+    const labelId = `${itemId}--label`
+    const inlineDescriptionId = `${itemId}--inline-description`
+    const blockDescriptionId = `${itemId}--block-description`
 
     const ItemWrapper = _PrivateItemWrapper || React.Fragment
 
@@ -208,6 +208,7 @@ export const Item = React.forwardRef<HTMLLIElement, ActionListItemProps>(
       'aria-describedby': slots.description?.props.variant === 'block' ? blockDescriptionId : undefined,
       ...(selectionAttribute && {[selectionAttribute]: selected}),
       role: role || itemRole,
+      id: itemId,
     }
 
     const containerProps = _PrivateItemWrapper ? {role: role || itemRole ? 'none' : undefined} : menuItemProps

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',
@@ -64,8 +65,11 @@ export const TestSxProp = () => {
       >
         Red
       </Button>
-      <Button leadingVisual={SearchIcon} variant="invisible" sx={{color: 'firebrick'}}>
-        Red
+      <Button variant="invisible" sx={{color: 'firebrick'}}>
+        Invariant color overridden
+      </Button>
+      <Button leadingVisual={IssueClosedIcon} sx={{color: 'done.fg'}}>
+        <Text sx={{color: 'fg.default'}}>Close issue</Text>
       </Button>
       <Button
         size="small"

src/Button/Button.tsx

@@ -3,18 +3,23 @@ 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 => {
   const {block, size, leadingVisual, trailingVisual, trailingAction} = props
   let sxStyles = sxProp
+  const style: CSSCustomProperties = {}
 
   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>
   )
@@ -67,11 +72,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/Dialog/__snapshots__/Dialog.test.tsx.snap

@@ -291,6 +291,9 @@ exports[`Dialog renders consistently 1`] = `
 
 .c1[data-no-visuals] {
   color: #0969da;
+  position: absolute;
+  top: 8px;
+  right: 16px;
 }
 
 .c1:has([data-component="ButtonCounter"]) {
@@ -305,12 +308,6 @@ exports[`Dialog renders consistently 1`] = `
   color: inherit;
 }
 
-.c1[data-no-visuals="true"] {
-  position: absolute;
-  top: 8px;
-  right: 16px;
-}
-
 .c4 {
   font-size: 14px;
   font-weight: 600;

src/NavList/__snapshots__/NavList.test.tsx.snap

@@ -306,9 +306,10 @@ exports[`NavList renders a simple list 1`] = `
       >
         <a
           aria-current="page"
-          aria-labelledby="react-aria-2 "
+          aria-labelledby="react-aria-2--label "
           class="c3"
           href="/"
+          id="react-aria-2"
           tabindex="0"
         >
           <div
@@ -317,7 +318,7 @@ exports[`NavList renders a simple list 1`] = `
           >
             <span
               class="c5"
-              id="react-aria-2"
+              id="react-aria-2--label"
             >
               Home
             </span>
@@ -328,9 +329,10 @@ exports[`NavList renders a simple list 1`] = `
         class="c1 c6"
       >
         <a
-          aria-labelledby="react-aria-5 "
+          aria-labelledby="react-aria-3--label "
           class="c3"
           href="/about"
+          id="react-aria-3"
           tabindex="0"
         >
           <div
@@ -339,7 +341,7 @@ exports[`NavList renders a simple list 1`] = `
           >
             <span
               class="c5"
-              id="react-aria-5"
+              id="react-aria-3--label"
             >
               About
             </span>
@@ -350,9 +352,10 @@ exports[`NavList renders a simple list 1`] = `
         class="c1 c6"
       >
         <a
-          aria-labelledby="react-aria-8 "
+          aria-labelledby="react-aria-4--label "
           class="c3"
           href="/contact"
+          id="react-aria-4"
           tabindex="0"
         >
           <div
@@ -361,7 +364,7 @@ exports[`NavList renders a simple list 1`] = `
           >
             <span
               class="c5"
-              id="react-aria-8"
+              id="react-aria-4--label"
             >
               Contact
             </span>
@@ -736,9 +739,10 @@ exports[`NavList renders with groups 1`] = `
           >
             <a
               aria-current="page"
-              aria-labelledby="react-aria-3 "
+              aria-labelledby="react-aria-3--label "
               class="c7"
               href="/getting-started"
+              id="react-aria-3"
               tabindex="0"
             >
               <div
@@ -747,7 +751,7 @@ exports[`NavList renders with groups 1`] = `
               >
                 <span
                   class="c9"
-                  id="react-aria-3"
+                  id="react-aria-3--label"
                 >
                   Getting started
                 </span>
@@ -770,22 +774,23 @@ exports[`NavList renders with groups 1`] = `
           role="presentation"
         >
           <span
-            id="react-aria-6"
+            id="react-aria-4"
           >
             Components
           </span>
         </div>
         <ul
-          aria-labelledby="react-aria-6"
+          aria-labelledby="react-aria-4"
           class="c4"
         >
           <li
             class="c5 c10"
           >
             <a
-              aria-labelledby="react-aria-7 "
+              aria-labelledby="react-aria-5--label "
               class="c7"
               href="/Avatar"
+              id="react-aria-5"
               tabindex="0"
             >
               <div
@@ -794,7 +799,7 @@ exports[`NavList renders with groups 1`] = `
               >
                 <span
                   class="c9"
-                  id="react-aria-7"
+                  id="react-aria-5--label"
                 >
                   Avatar
                 </span>
@@ -1177,8 +1182,9 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav
         <button
           aria-controls="react-aria-3"
           aria-expanded="true"
-          aria-labelledby="react-aria-2 "
+          aria-labelledby="react-aria-2--label "
           class="c3 c4"
+          id="react-aria-2"
           tabindex="0"
         >
           <div
@@ -1190,7 +1196,7 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav
             >
               <span
                 class="c1 c7"
-                id="react-aria-2"
+                id="react-aria-2--label"
               >
                 Item
               </span>
@@ -1227,9 +1233,10 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav
             >
               <a
                 aria-current="page"
-                aria-labelledby="react-aria-4 "
+                aria-labelledby="react-aria-4--label "
                 class="c12"
                 href="#"
+                id="react-aria-4"
                 tabindex="0"
               >
                 <div
@@ -1238,7 +1245,7 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav
                 >
                   <span
                     class="c1 c7"
-                    id="react-aria-4"
+                    id="react-aria-4--label"
                   >
                     Sub Item
                   </span>
@@ -1651,8 +1658,9 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t
         <button
           aria-controls="react-aria-3"
           aria-expanded="false"
-          aria-labelledby="react-aria-2 "
+          aria-labelledby="react-aria-2--label "
           class="c3 c4"
+          id="react-aria-2"
           tabindex="0"
         >
           <div
@@ -1664,7 +1672,7 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t
             >
               <span
                 class="c1 c7"
-                id="react-aria-2"
+                id="react-aria-2--label"
               >
                 Item
               </span>
@@ -1701,9 +1709,10 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t
             >
               <a
                 aria-current="page"
-                aria-labelledby="react-aria-4 "
+                aria-labelledby="react-aria-4--label "
                 class="c12"
                 href="#"
+                id="react-aria-4"
                 tabindex="0"
               >
                 <div
@@ -1712,7 +1721,7 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t
                 >
                   <span
                     class="c1 c7"
-                    id="react-aria-4"
+                    id="react-aria-4--label"
                   >
                     Sub Item
                   </span>