Fix Button loading announcements

.changeset/bright-buttons-announce.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+Button: Ensure loading state messages are announced when entering loading, including in React Strict Mode

packages/react/src/Button/Button.dev.stories.tsx

@@ -1,4 +1,5 @@
 import {SearchIcon, TriangleDownIcon, EyeIcon, HeartFillIcon} from '@primer/octicons-react'
+import {useState} from 'react'
 import {Button, IconButton} from '.'
 import {Stack} from '../Stack'
 
@@ -86,6 +87,27 @@ export const DisabledButtonVariants = () => {
   )
 }
 
+export const LoadingAnnouncement = () => {
+  const [isLoading, setIsLoading] = useState(false)
+
+  return (
+    <Stack gap="normal" align="start">
+      <p>
+        With a screen reader running, activate “Export repository”. The button enters its loading state and announces
+        “Exporting repository”. Reset the loading state to repeat the test.
+      </p>
+      <Stack direction="horizontal">
+        <Button loading={isLoading} loadingAnnouncement="Exporting repository" onClick={() => setIsLoading(true)}>
+          Export repository
+        </Button>
+        <Button disabled={!isLoading} onClick={() => setIsLoading(false)}>
+          Reset loading state
+        </Button>
+      </Stack>
+    </Stack>
+  )
+}
+
 export const LinkVariantWithUnderlinePreference = () => {
   return (
     <Stack gap="spacious">

packages/react/src/Button/ButtonBase.tsx

@@ -200,7 +200,9 @@ const ButtonBase = forwardRef(({children, as: Component = 'button', ...props}, f
       </Component>
       {loading && (
         <VisuallyHidden>
-          <AriaStatus id={loadingAnnouncementID}>{loadingAnnouncement}</AriaStatus>
+          <AriaStatus id={loadingAnnouncementID} announceOnShow>
+            {loadingAnnouncement}
+          </AriaStatus>
         </VisuallyHidden>
       )}
     </ConditionalWrapper>

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

@@ -7,6 +7,7 @@ import {IconButton, Button, LinkButton} from '../../Button'
 import classes from '../ButtonBase.module.css'
 import {implementsClassName} from '../../utils/testing'
 import {FeatureFlags} from '../../FeatureFlags'
+import {getLiveRegion} from '../../live-region/__tests__/test-helpers'
 
 type StatefulLoadingButtonProps = {
   children?: React.ReactNode
@@ -295,6 +296,25 @@ describe('Button', () => {
       .toBe('Action loading')
   })
 
+  it('announces the loading announcement when entering the loading state', async () => {
+    for (const region of document.querySelectorAll('live-region')) {
+      region.remove()
+    }
+    const liveRegion = document.createElement('live-region')
+    document.body.appendChild(liveRegion)
+
+    render(
+      <StatefulLoadingButton loadingAnnouncement="Action loading">
+        <span>content</span>
+      </StatefulLoadingButton>,
+    )
+
+    await userEvent.click(page.getByRole('button'))
+
+    await expect.poll(() => getLiveRegion().getMessage('polite')).toBe('Action loading')
+    liveRegion.remove()
+  })
+
   it('should be described by loading announcement AND whatever is passed to aria-describedby', async () => {
     const buttonDescriptionId = 'button-description'
     const buttonId = 'loading-button'

packages/react/src/live-region/Announce.tsx

@@ -1,7 +1,6 @@
 import {announceFromElement} from '@primer/live-region-element'
 import type React from 'react'
 import {useEffect, useRef, type ElementRef} from 'react'
-import {useEffectOnce} from '../internal/hooks/useEffectOnce'
 import {useEffectCallback} from '../internal/hooks/useEffectCallback'
 import type {PolymorphicProps} from '../utils/modern-polymorphic'
 
@@ -52,6 +51,7 @@ export function Announce<As extends React.ElementType = 'div'>(props: AnnouncePr
     ...rest
   } = props
   const ref = useRef<ElementRef<'div'>>(null)
+
   // Tracked in a ref rather than state because it is only used to dedupe
   // announcements and is never rendered. Using state here would trigger a React
   // commit on every content change (e.g. one per keystroke when tied to an input).
@@ -99,14 +99,16 @@ export function Announce<As extends React.ElementType = 'div'>(props: AnnouncePr
     previousAnnouncementText.current = textContent
   })
 
-  // Announce the initial message, this is wrapped in `useEffectOnce` so that it
-  // does not announce twice in StrictMode
-  useEffectOnce(() => {
+  const announceOnMount = useEffectCallback(() => {
     if (announceOnShow) {
       announce()
     }
   })
 
+  useEffect(() => {
+    announceOnMount()
+  }, [announceOnMount])
+
   useEffect(() => {
     const {current: container} = ref
     if (container === null) {
@@ -131,10 +133,9 @@ export function Announce<As extends React.ElementType = 'div'>(props: AnnouncePr
 
   useEffect(() => {
     return () => {
-      if (savedAnnouncement.current !== null) {
-        savedAnnouncement.current.cancel()
-        savedAnnouncement.current = null
-      }
+      savedAnnouncement.current?.cancel()
+      savedAnnouncement.current = null
+      previousAnnouncementText.current = null
     }
   }, [])
 

packages/react/src/live-region/__tests__/Announce.test.tsx

@@ -1,5 +1,6 @@
 import {describe, expect, it, beforeEach, afterEach} from 'vitest'
 import {render, screen, act, waitFor} from '@testing-library/react'
+import {StrictMode} from 'react'
 import {Announce} from '../Announce'
 import {getLiveRegion} from './test-helpers'
 import {implementsClassName} from '../../utils/testing'
@@ -34,6 +35,37 @@ describe('Announce', () => {
     expect(liveRegion.getMessage('polite')).toBe('')
   })
 
+  it('announces delayed content when rendered in StrictMode', async () => {
+    render(
+      <StrictMode>
+        <Announce delayMs={50}>test</Announce>
+      </StrictMode>,
+    )
+
+    await waitFor(() => {
+      expect(getLiveRegion().getMessage('polite')).toBe('test')
+    })
+  })
+
+  it('does not announce when announceOnShow changes after mount', () => {
+    const {rerender} = render(<Announce announceOnShow={false}>test</Announce>)
+
+    rerender(<Announce announceOnShow>test</Announce>)
+
+    expect(getLiveRegion().getMessage('polite')).toBe('')
+  })
+
+  it('cancels delayed content after unmounting', async () => {
+    const {unmount} = render(<Announce delayMs={50}>test</Announce>)
+
+    unmount()
+    await act(async () => {
+      await new Promise(resolve => setTimeout(resolve, 75))
+    })
+
+    expect(getLiveRegion().getMessage('polite')).toBe('')
+  })
+
   it('should pass additional props to the container element', () => {
     const {container} = render(<Announce data-testid="container">test</Announce>)