Merge branch 'main' into icon-button-keyshortcuts

.changeset/bright-islands-kick.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+Add support for providing icons as an element to UnderlineNavItem

.changeset/clean-fireants-type.md

@@ -0,0 +1,8 @@
+---
+"@primer/react": minor
+---
+
+IconButton: Enable tooltips by default in icon buttons by updating the default value of `unsafeDisableTooltip` to `false`.
+
+This is a behaviour change in icon buttons, please upgrade with a caution.  
+

.github/workflows/add_staff_label.yml

@@ -0,0 +1,23 @@
+name: Add Staff Label
+
+on:
+  issues:
+    types: [opened, reopened]
+  pull_request:
+    types: [opened, reopened]
+
+jobs:
+  add-staff-label:
+    if: ${{ github.repository == 'primer/react' }}
+    runs-on: ubuntu-latest
+    env:
+      GH_TOKEN: ${{ github.token }}
+    steps:
+      - name: Add label to issue
+        if: ${{ github.event.issue.author_association == 'MEMBER' }}
+        run: |
+          gh issue edit ${{github.event.issue.html_url}} --add-label staff
+      - name: Add label to pull_request
+        if: ${{ github.event.pull_request.author_association == 'MEMBER' }}
+        run: |
+          gh pr edit ${{github.event.pull_request.html_url}} --add-label staff

e2e/components/drafts/ActionBar.test.ts

@@ -44,8 +44,8 @@ test.describe('ActionBar', () => {
           await expect(page.locator(toolbarButtonSelector)).toHaveCount(10)
           await page.setViewportSize({width: viewports['primer.breakpoint.xs'], height: 768})
           await expect(page.locator(toolbarButtonSelector)).toHaveCount(6)
-          const moreButtonSelector = `button[aria-label="More Comment box toolbar items"]`
-          await page.locator(moreButtonSelector).click()
+          const moreButtonSelector = page.getByLabel('More Comment box toolbar items')
+          await moreButtonSelector.click()
           await expect(page.locator('ul[role="menu"]>li')).toHaveCount(5)
         })
       })

examples/app-router/package.json

@@ -9,7 +9,7 @@
     "type-check": "tsc --noEmit"
   },
   "dependencies": {
-    "@primer/react": "36.24.0",
+    "@primer/react": "36.25.0",
     "next": "^14.1.0",
     "react": "^18.3.1",
     "react-dom": "^18.3.1",

examples/codesandbox/package.json

@@ -20,7 +20,7 @@
     "@typescript-eslint/eslint-plugin": "^7.11.0",
     "@typescript-eslint/parser": "^7.3.1",
     "@vitejs/plugin-react": "^4.2.1",
-    "@primer/react": "36.24.0",
+    "@primer/react": "36.25.0",
     "eslint": "^8.56.0",
     "eslint-plugin-react-hooks": "^4.6.0",
     "eslint-plugin-react-refresh": "^0.4.7",

examples/nextjs/package.json

@@ -10,7 +10,7 @@
   },
   "dependencies": {
     "@primer/octicons-react": "19.x",
-    "@primer/react": "36.24.0",
+    "@primer/react": "36.25.0",
     "next": "^14.1.0",
     "react": "^18.3.1",
     "react-dom": "^18.3.1",

packages/react/CHANGELOG.md

@@ -1,5 +1,21 @@
 # @primer/react
 
+## 36.25.0
+
+### Minor Changes
+
+- [#4051](https://github.com/primer/react/pull/4051) [`7e644b70359fcba07810560abcb8b1fbe785668a`](https://github.com/primer/react/commit/7e644b70359fcba07810560abcb8b1fbe785668a) Thanks [@mperrotti](https://github.com/mperrotti)! - Adds a loading state to ActionList items. Also allows the Spinner component to accept screenreader text.
+
+  <!-- Changed components: ActionList, ActionMenu, NavList, Spinners -->
+
+- [#4697](https://github.com/primer/react/pull/4697) [`a7d1e4f37cd8fd01c86250178ef6ae748d786e03`](https://github.com/primer/react/commit/a7d1e4f37cd8fd01c86250178ef6ae748d786e03) Thanks [@khiga8](https://github.com/khiga8)! - Add TrailingAction support to NavList
+
+### Patch Changes
+
+- [#4706](https://github.com/primer/react/pull/4706) [`71859edc30664e259c855ffdc3732cda8dc6d169`](https://github.com/primer/react/commit/71859edc30664e259c855ffdc3732cda8dc6d169) Thanks [@TylerJDev](https://github.com/TylerJDev)! - (Behind feature flag) ActionList: Fix for "full" variant when using button semantics
+
+- [#4711](https://github.com/primer/react/pull/4711) [`199e3840af17d8ea7c75dbba60cdfbaaf7ef4021`](https://github.com/primer/react/commit/199e3840af17d8ea7c75dbba60cdfbaaf7ef4021) Thanks [@TylerJDev](https://github.com/TylerJDev)! - Removes live region from `FormControl` validation
+
 ## 36.24.0
 
 ### Minor Changes

packages/react/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@primer/react",
-  "version": "36.24.0",
+  "version": "36.25.0",
   "description": "An implementation of GitHub's Primer Design System using React",
   "main": "lib/index.js",
   "module": "lib-esm/index.js",

packages/react/src/ActionBar/ActionBar.tsx

@@ -299,7 +299,7 @@ export const ActionBarIconButton = forwardRef((props: ActionBarIconButtonProps,
     const domRect = (ref as MutableRefObject<HTMLElement>).current.getBoundingClientRect()
     setChildrenWidth({text, width: domRect.width})
   }, [ref, setChildrenWidth])
-  return <IconButton ref={ref} size={size} {...props} variant="invisible" unsafeDisableTooltip={false} />
+  return <IconButton ref={ref} size={size} {...props} variant="invisible" />
 })
 
 const sizeToHeight = {

packages/react/src/ActionList/TrailingAction.tsx

@@ -48,7 +48,6 @@ export const TrailingAction = forwardRef(({as = 'button', icon, label, href = nu
           aria-label={label}
           icon={icon}
           variant="invisible"
-          unsafeDisableTooltip={false}
           tooltipDirection="w"
           href={href}
           // @ts-expect-error StyledButton wants both Anchor and Button refs

packages/react/src/Button/IconButton.tsx

@@ -17,7 +17,7 @@ const IconButton = forwardRef(
       disabled,
       tooltipDirection,
       // This is planned to be a temporary prop until the default tooltip on icon buttons are fully rolled out.
-      unsafeDisableTooltip = true,
+      unsafeDisableTooltip = false,
       keyshortcuts,
       ...props
     },

packages/react/src/Button/__tests__/Button.test.tsx

@@ -131,11 +131,12 @@ describe('Button', () => {
     const tooltipEl = getByText('Love is all around')
     expect(triggerEL).toHaveAttribute('aria-describedby', tooltipEl.id)
   })
-  it('should not render tooltip on an icon button by default', () => {
-    const {getByRole} = render(<IconButton icon={HeartIcon} aria-label="Heart" />)
+  it('should render tooltip on an icon button by default', () => {
+    const {getByRole, getByText} = render(<IconButton icon={HeartIcon} aria-label="Heart" />)
     const triggerEl = getByRole('button')
-    expect(triggerEl).not.toHaveAttribute('aria-labelledby')
-    expect(triggerEl).toHaveAttribute('aria-label', 'Heart')
+    const tooltipEl = getByText('Heart')
+    expect(triggerEl).toHaveAttribute('aria-labelledby', tooltipEl.id)
+    expect(triggerEl).not.toHaveAttribute('aria-label')
   })
   it('should render aria-keyshorts on an icon button when keyshortcuts prop is passed', () => {
     const {getByRole} = render(

packages/react/src/ButtonGroup/ButtonGroup.features.stories.tsx

@@ -11,7 +11,8 @@ export default {
 
 export const IconButtons = () => (
   <ButtonGroup>
-    <IconButton icon={PlusIcon} aria-label="Add" />
-    <IconButton icon={DashIcon} aria-label="Subtract" />
+    {/* We can remove these unsafe props after we resolve https://github.com/primer/react/issues/4129 */}
+    <IconButton unsafeDisableTooltip={true} icon={PlusIcon} aria-label="Add" />
+    <IconButton unsafeDisableTooltip={true} icon={DashIcon} aria-label="Subtract" />
   </ButtonGroup>
 )

packages/react/src/Dialog.tsx

@@ -11,6 +11,7 @@ import type {ComponentProps} from './utils/types'
 import {useRefObjectAsForwardedRef} from './hooks/useRefObjectAsForwardedRef'
 import {XIcon} from '@primer/octicons-react'
 
+// Dialog v1
 const noop = () => null
 
 type StyledDialogBaseProps = {

packages/react/src/UnderlineNav/UnderlineNav.features.stories.tsx

@@ -16,9 +16,11 @@ import type {Meta} from '@storybook/react'
 import {UnderlineNav} from './index'
 import {INITIAL_VIEWPORTS} from '@storybook/addon-viewport'
 
-export default {
+const meta = {
   title: 'Components/UnderlineNav/Features',
-} as Meta
+} satisfies Meta<typeof UnderlineNav>
+
+export default meta
 
 export const Default = () => {
   return (
@@ -33,28 +35,28 @@ export const Default = () => {
 export const WithIcons = () => {
   return (
     <UnderlineNav aria-label="Repository with icons">
-      <UnderlineNav.Item icon={CodeIcon}>Code</UnderlineNav.Item>
-      <UnderlineNav.Item icon={EyeIcon} counter={6}>
+      <UnderlineNav.Item icon={<CodeIcon />}>Code</UnderlineNav.Item>
+      <UnderlineNav.Item icon={<EyeIcon />} counter={6}>
         Issues
       </UnderlineNav.Item>
-      <UnderlineNav.Item aria-current="page" icon={GitPullRequestIcon}>
+      <UnderlineNav.Item aria-current="page" icon={<GitPullRequestIcon />}>
         Pull Requests
       </UnderlineNav.Item>
-      <UnderlineNav.Item icon={CommentDiscussionIcon} counter={7}>
+      <UnderlineNav.Item icon={<CommentDiscussionIcon />} counter={7}>
         Discussions
       </UnderlineNav.Item>
-      <UnderlineNav.Item icon={ProjectIcon}>Projects</UnderlineNav.Item>
+      <UnderlineNav.Item icon={<ProjectIcon />}>Projects</UnderlineNav.Item>
     </UnderlineNav>
   )
 }
 
 export const WithCounterLabels = () => {
   return (
     <UnderlineNav aria-label="Repository with counters">
-      <UnderlineNav.Item aria-current="page" icon={CodeIcon} counter="11K">
+      <UnderlineNav.Item aria-current="page" icon={<CodeIcon />} counter="11K">
         Code
       </UnderlineNav.Item>
-      <UnderlineNav.Item icon={IssueOpenedIcon} counter={12}>
+      <UnderlineNav.Item icon={<IssueOpenedIcon />} counter={12}>
         Issues
       </UnderlineNav.Item>
     </UnderlineNav>

packages/react/src/UnderlineNav/UnderlineNav.stories.tsx

@@ -5,10 +5,9 @@ import {UnderlineNavItem} from './UnderlineNavItem'
 
 const excludedControlKeys = ['sx', 'as', 'variant', 'align', 'afterSelect']
 
-export default {
+const meta: Meta<typeof UnderlineNav> = {
   title: 'Components/UnderlineNav',
   component: UnderlineNav,
-  subcomponents: {UnderlineNavItem},
   parameters: {
     controls: {
       expanded: true,
@@ -33,7 +32,9 @@ export default {
     'aria-label': 'Repository',
     loadingCounters: false,
   },
-} as Meta<typeof UnderlineNav>
+}
+
+export default meta
 
 export const Default: StoryFn<typeof UnderlineNav> = () => {
   const children = ['Code', 'Pull requests', 'Actions', 'Projects', 'Wiki']

packages/react/src/UnderlineNav/UnderlineNav.test.tsx

@@ -1,5 +1,5 @@
 import React from 'react'
-import {render} from '@testing-library/react'
+import {render, screen} from '@testing-library/react'
 import userEvent from '@testing-library/user-event'
 import type {IconProps} from '@primer/octicons-react'
 import {
@@ -77,22 +77,26 @@ describe('UnderlineNav', () => {
     default: undefined,
     UnderlineNav,
   })
+
   it('renders aria-current attribute to be pages when an item is selected', () => {
     const {getByRole} = render(<ResponsiveUnderlineNav />)
     const selectedNavLink = getByRole('link', {name: 'Code'})
     expect(selectedNavLink.getAttribute('aria-current')).toBe('page')
   })
+
   it('renders aria-label attribute correctly', () => {
     const {container, getByRole} = render(<ResponsiveUnderlineNav />)
     expect(container.getElementsByTagName('nav').length).toEqual(1)
     const nav = getByRole('navigation')
     expect(nav.getAttribute('aria-label')).toBe('Repository')
   })
+
   it('renders icons correctly', () => {
     const {getByRole} = render(<ResponsiveUnderlineNav />)
     const nav = getByRole('navigation')
     expect(nav.getElementsByTagName('svg').length).toEqual(7)
   })
+
   it('fires onSelect on click', async () => {
     const onSelect = jest.fn()
     const {getByRole} = render(
@@ -107,6 +111,7 @@ describe('UnderlineNav', () => {
     await user.click(item)
     expect(onSelect).toHaveBeenCalledTimes(1)
   })
+
   it('fires onSelect on keypress', async () => {
     const onSelect = jest.fn()
     const {getByRole} = render(
@@ -128,27 +133,31 @@ describe('UnderlineNav', () => {
     await user.keyboard(' ') // space
     expect(onSelect).toHaveBeenCalledTimes(3)
   })
+
   it('respects counter prop', () => {
     const {getByRole} = render(<ResponsiveUnderlineNav />)
     const item = getByRole('link', {name: 'Issues (120)'})
     const counter = item.getElementsByTagName('span')[3]
     expect(counter.textContent).toBe('120')
     expect(counter).toHaveAttribute('aria-hidden', 'true')
   })
+
   it('renders the content of visually hidden span properly for screen readers', () => {
     const {getByRole} = render(<ResponsiveUnderlineNav />)
     const item = getByRole('link', {name: 'Issues (120)'})
     const counter = item.getElementsByTagName('span')[4]
     // non breaking space unified code
     expect(counter.textContent).toBe('\u00A0(120)')
   })
+
   it('respects loadingCounters prop', () => {
     const {getByRole} = render(<ResponsiveUnderlineNav loadingCounters={true} />)
     const item = getByRole('link', {name: 'Actions'})
     const loadingCounter = item.getElementsByTagName('span')[2]
     expect(loadingCounter.className).toContain('LoadingCounter')
     expect(loadingCounter.textContent).toBe('')
   })
+
   it('renders a visually hidden h2 heading for screen readers when aria-label is present', () => {
     const {getByRole} = render(<ResponsiveUnderlineNav />)
     const heading = getByRole('heading', {name: 'Repository navigation'})
@@ -157,6 +166,7 @@ describe('UnderlineNav', () => {
     expect(heading.className).toContain('VisuallyHidden')
     expect(heading.textContent).toBe('Repository navigation')
   })
+
   it('throws an error when there are multiple items that have aria-current', () => {
     const spy = jest.spyOn(console, 'error').mockImplementation()
     expect(() => {
@@ -186,6 +196,22 @@ describe('UnderlineNav', () => {
     // We are expecting a left value back, that way we know the `getAnchoredPosition` ran.
     expect(results).toEqual(expect.objectContaining({left: 0}))
   })
+
+  it('should support icons passed in as an element', () => {
+    render(
+      <UnderlineNav aria-label="Repository">
+        <UnderlineNav.Item aria-current="page" icon={<CodeIcon aria-label="Page one icon" />}>
+          Page one
+        </UnderlineNav.Item>
+        <UnderlineNav.Item icon={<IssueOpenedIcon aria-label="Page two icon" />}>Page two</UnderlineNav.Item>
+        <UnderlineNav.Item icon={<GitPullRequestIcon aria-label="Page three icon" />}>Page three</UnderlineNav.Item>
+      </UnderlineNav>,
+    )
+
+    expect(screen.getByLabelText('Page one icon')).toBeInTheDocument()
+    expect(screen.getByLabelText('Page two icon')).toBeInTheDocument()
+    expect(screen.getByLabelText('Page three icon')).toBeInTheDocument()
+  })
 })
 
 describe('Keyboard Navigation', () => {

packages/react/src/UnderlineNav/UnderlineNavItem.tsx

@@ -38,7 +38,7 @@ export type UnderlineNavItemProps = {
   /**
    *  Icon before the text
    */
-  icon?: React.FunctionComponent<IconProps>
+  icon?: React.FunctionComponent<IconProps> | React.ReactElement
   /**
    * Renders `UnderlineNav.Item` as given component i.e. react-router's Link
    **/