From 0cf08f325347ad176689201a71618aa1d33e6c23 Mon Sep 17 00:00:00 2001
From: Josh Black <joshblack@github.com>
Date: Thu, 20 Jun 2024 11:42:26 -0500
Subject: [PATCH] fix(Spinner): use aria-label when defined, falling back to
 srText (#4685)

* fix(Spinner): use aria-label when defined, falling back to srText

* test(Spinner): add tests for aria-label support

* docs: update Spinner API docs

* chore: fix eslint warnings

---------

Co-authored-by: Josh Black <joshblack@users.noreply.github.com>
---
 packages/react/src/Spinner/Spinner.docs.json  |  2 +-
 packages/react/src/Spinner/Spinner.tsx        | 11 ++++++-----
 packages/react/src/__tests__/Spinner.test.tsx |  9 +++++++--
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/packages/react/src/Spinner/Spinner.docs.json b/packages/react/src/Spinner/Spinner.docs.json
index 7d7bd6d014a..9acb22c3ee6 100644
--- a/packages/react/src/Spinner/Spinner.docs.json
+++ b/packages/react/src/Spinner/Spinner.docs.json
@@ -19,7 +19,7 @@
     },
     {
       "name": "aria-label",
-      "type": "string | null",
+      "type": "string",
       "description": "Sets the text conveyed by assistive technologies such as screen readers.",
       "deprecated": true
     },
diff --git a/packages/react/src/Spinner/Spinner.tsx b/packages/react/src/Spinner/Spinner.tsx
index 5cb003acebc..c980c506be8 100644
--- a/packages/react/src/Spinner/Spinner.tsx
+++ b/packages/react/src/Spinner/Spinner.tsx
@@ -18,14 +18,14 @@ export type SpinnerProps = {
   /** Sets the text conveyed by assistive technologies such as screen readers. Set to `null` if the loading state is displayed in a text node somewhere else on the page. */
   srText?: string | null
   /** @deprecated Use `srText` instead. */
-  'aria-label'?: string | null
+  'aria-label'?: string
 } & HTMLDataAttributes &
   SxProp
 
 function Spinner({size: sizeKey = 'medium', srText = 'Loading', 'aria-label': ariaLabel, ...props}: SpinnerProps) {
   const size = sizeMap[sizeKey]
-  const hasSrAnnouncement = Boolean(srText || ariaLabel)
-  const ariaLabelId = useId()
+  const hasHiddenLabel = srText !== null && ariaLabel === undefined
+  const labelId = useId()
 
   return (
     /* inline-flex removes the extra line height */
@@ -36,7 +36,8 @@ function Spinner({size: sizeKey = 'medium', srText = 'Loading', 'aria-label': ar
         viewBox="0 0 16 16"
         fill="none"
         aria-hidden
-        aria-labelledby={ariaLabelId}
+        aria-label={ariaLabel ?? undefined}
+        aria-labelledby={hasHiddenLabel ? labelId : undefined}
         {...props}
       >
         <circle
@@ -56,7 +57,7 @@ function Spinner({size: sizeKey = 'medium', srText = 'Loading', 'aria-label': ar
           vectorEffect="non-scaling-stroke"
         />
       </svg>
-      {hasSrAnnouncement ? <VisuallyHidden id={ariaLabelId}>{srText || ariaLabel}</VisuallyHidden> : null}
+      {hasHiddenLabel ? <VisuallyHidden id={labelId}>{srText}</VisuallyHidden> : null}
     </Box>
   )
 }
diff --git a/packages/react/src/__tests__/Spinner.test.tsx b/packages/react/src/__tests__/Spinner.test.tsx
index 47151e13b4d..568c563e4a5 100644
--- a/packages/react/src/__tests__/Spinner.test.tsx
+++ b/packages/react/src/__tests__/Spinner.test.tsx
@@ -3,7 +3,7 @@ import axe from 'axe-core'
 import type {SpinnerProps} from '..'
 import {Spinner} from '..'
 import {behavesAsComponent, checkExports} from '../utils/testing'
-import {render as HTMLRender} from '@testing-library/react'
+import {render as HTMLRender, screen} from '@testing-library/react'
 
 describe('Spinner', () => {
   behavesAsComponent({
@@ -26,12 +26,17 @@ describe('Spinner', () => {
     expect(getByLabelText('Custom loading text')).toBeInTheDocument()
   })
 
-  it('should not label the spinner with with loading text when `srText` is set to `null`', async () => {
+  it('should not label the spinner with with loading text when `srText` is set to `null`', () => {
     const {getByLabelText} = HTMLRender(<Spinner srText={null} />)
 
     expect(() => getByLabelText('Loading')).toThrow()
   })
 
+  it('should use `aria-label` over `srText` if `aria-label` is provided', () => {
+    HTMLRender(<Spinner aria-label="Test label" />)
+    expect(screen.getByLabelText('Test label')).toBeInTheDocument()
+  })
+
   it('should have no axe violations', async () => {
     const {container} = HTMLRender(<Spinner />)
     const results = await axe.run(container)