From 25c10ddae4bd7266822a3af62f504cc749bc8ed5 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 12 Oct 2026 22:47:59 +0200 Subject: [PATCH 1/3] Fix Button loading announcements Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .changeset/bright-buttons-announce.md | 5 ++++ .../react/src/Button/Button.dev.stories.tsx | 22 +++++++++++++++++ packages/react/src/Button/ButtonBase.tsx | 4 +++- .../src/Button/__tests__/Button.test.tsx | 21 ++++++++++++++++ packages/react/src/live-region/Announce.tsx | 15 ++++++++---- .../live-region/__tests__/Announce.test.tsx | 24 +++++++++++++++++++ 6 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 .changeset/bright-buttons-announce.md diff --git a/.changeset/bright-buttons-announce.md b/.changeset/bright-buttons-announce.md new file mode 100644 index 00000000000..a8ff3db8b8e --- /dev/null +++ b/.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 diff --git a/packages/react/src/Button/Button.dev.stories.tsx b/packages/react/src/Button/Button.dev.stories.tsx index 8d23de9c81f..f1099d1cc0e 100644 --- a/packages/react/src/Button/Button.dev.stories.tsx +++ b/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 ( + +

+ 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. +

+ + + + + + ) +} + export const LinkVariantWithUnderlinePreference = () => { return ( diff --git a/packages/react/src/Button/ButtonBase.tsx b/packages/react/src/Button/ButtonBase.tsx index b6b362565f3..6f0975a75c7 100644 --- a/packages/react/src/Button/ButtonBase.tsx +++ b/packages/react/src/Button/ButtonBase.tsx @@ -199,7 +199,9 @@ const ButtonBase = forwardRef(({children, as: Component = 'button', ...props}, f {loading && ( - {loadingAnnouncement} + + {loadingAnnouncement} + )} diff --git a/packages/react/src/Button/__tests__/Button.test.tsx b/packages/react/src/Button/__tests__/Button.test.tsx index f4f4ded2486..2a5c8380daf 100644 --- a/packages/react/src/Button/__tests__/Button.test.tsx +++ b/packages/react/src/Button/__tests__/Button.test.tsx @@ -6,6 +6,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 @@ -242,6 +243,26 @@ describe('Button', () => { expect(loadingAnnouncement?.textContent).toBe('Action loading') }) + it('announces the loading announcement when entering the loading state', () => { + for (const region of document.querySelectorAll('live-region')) { + region.remove() + } + const liveRegion = document.createElement('live-region') + document.body.appendChild(liveRegion) + + render( + + content + , + ) + + fireEvent.click(screen.getByRole('button')) + + const message = getLiveRegion().getMessage('polite') + liveRegion.remove() + expect(message).toBe('Action loading') + }) + it('should be described by loading announcement AND whatever is passed to aria-describedby', () => { const buttonDescriptionId = 'button-description' const buttonId = 'loading-button' diff --git a/packages/react/src/live-region/Announce.tsx b/packages/react/src/live-region/Announce.tsx index ae67d2fc51a..3b8b0425e37 100644 --- a/packages/react/src/live-region/Announce.tsx +++ b/packages/react/src/live-region/Announce.tsx @@ -52,11 +52,13 @@ export function Announce(props: AnnouncePr ...rest } = props const ref = useRef>(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). const previousAnnouncementText = useRef(null) const savedAnnouncement = useRef | null>(null) + const cleanupTimeout = useRef | null>(null) const announce = useEffectCallback(() => { const {current: element} = ref if (!element) { @@ -130,11 +132,16 @@ export function Announce(props: AnnouncePr }, [announce]) useEffect(() => { + const element = ref.current return () => { - if (savedAnnouncement.current !== null) { - savedAnnouncement.current.cancel() - savedAnnouncement.current = null - } + // StrictMode reconnects the rendered element before the next task. + // eslint-disable-next-line @eslint-react/web-api/no-leaked-timeout -- this task must outlive effect cleanup + cleanupTimeout.current = setTimeout(() => { + if (!element?.isConnected && savedAnnouncement.current !== null) { + savedAnnouncement.current.cancel() + savedAnnouncement.current = null + } + }, 0) } }, []) diff --git a/packages/react/src/live-region/__tests__/Announce.test.tsx b/packages/react/src/live-region/__tests__/Announce.test.tsx index 5512adec0ef..a96a7d1d50c 100644 --- a/packages/react/src/live-region/__tests__/Announce.test.tsx +++ b/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,29 @@ describe('Announce', () => { expect(liveRegion.getMessage('polite')).toBe('') }) + it('announces delayed content when rendered in StrictMode', async () => { + render( + + test + , + ) + + await waitFor(() => { + expect(getLiveRegion().getMessage('polite')).toBe('test') + }) + }) + + it('cancels delayed content after unmounting', async () => { + const {unmount} = render(test) + + 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(test) From 9642198637b1cc9ff4082abeac393c8ae08d66c3 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Tue, 20 Oct 2026 16:01:29 +0200 Subject: [PATCH 2/3] Address live region review feedback Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- packages/react/src/live-region/Announce.tsx | 21 ++++++--------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/packages/react/src/live-region/Announce.tsx b/packages/react/src/live-region/Announce.tsx index 3b8b0425e37..dc5f5bafc8e 100644 --- a/packages/react/src/live-region/Announce.tsx +++ b/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' @@ -58,7 +57,6 @@ export function Announce(props: AnnouncePr // commit on every content change (e.g. one per keystroke when tied to an input). const previousAnnouncementText = useRef(null) const savedAnnouncement = useRef | null>(null) - const cleanupTimeout = useRef | null>(null) const announce = useEffectCallback(() => { const {current: element} = ref if (!element) { @@ -101,13 +99,12 @@ export function Announce(props: AnnouncePr previousAnnouncementText.current = textContent }) - // Announce the initial message, this is wrapped in `useEffectOnce` so that it - // does not announce twice in StrictMode - useEffectOnce(() => { + useEffect(() => { + // eslint-disable-next-line react-you-might-not-need-an-effect/no-event-handler -- announce after the element mounts if (announceOnShow) { announce() } - }) + }, [announce, announceOnShow]) useEffect(() => { const {current: container} = ref @@ -132,16 +129,10 @@ export function Announce(props: AnnouncePr }, [announce]) useEffect(() => { - const element = ref.current return () => { - // StrictMode reconnects the rendered element before the next task. - // eslint-disable-next-line @eslint-react/web-api/no-leaked-timeout -- this task must outlive effect cleanup - cleanupTimeout.current = setTimeout(() => { - if (!element?.isConnected && savedAnnouncement.current !== null) { - savedAnnouncement.current.cancel() - savedAnnouncement.current = null - } - }, 0) + savedAnnouncement.current?.cancel() + savedAnnouncement.current = null + previousAnnouncementText.current = null } }, []) From 51bd8fda92f7c9d97183dd3f53e705444db7629f Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Fri, 23 Oct 2026 14:29:29 +0200 Subject: [PATCH 3/3] Preserve announceOnShow mount semantics Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- packages/react/src/live-region/Announce.tsx | 9 ++++++--- .../react/src/live-region/__tests__/Announce.test.tsx | 8 ++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/react/src/live-region/Announce.tsx b/packages/react/src/live-region/Announce.tsx index dc5f5bafc8e..f095c95ef1b 100644 --- a/packages/react/src/live-region/Announce.tsx +++ b/packages/react/src/live-region/Announce.tsx @@ -99,12 +99,15 @@ export function Announce(props: AnnouncePr previousAnnouncementText.current = textContent }) - useEffect(() => { - // eslint-disable-next-line react-you-might-not-need-an-effect/no-event-handler -- announce after the element mounts + const announceOnMount = useEffectCallback(() => { if (announceOnShow) { announce() } - }, [announce, announceOnShow]) + }) + + useEffect(() => { + announceOnMount() + }, [announceOnMount]) useEffect(() => { const {current: container} = ref diff --git a/packages/react/src/live-region/__tests__/Announce.test.tsx b/packages/react/src/live-region/__tests__/Announce.test.tsx index a96a7d1d50c..98d71c1ba2b 100644 --- a/packages/react/src/live-region/__tests__/Announce.test.tsx +++ b/packages/react/src/live-region/__tests__/Announce.test.tsx @@ -47,6 +47,14 @@ describe('Announce', () => { }) }) + it('does not announce when announceOnShow changes after mount', () => { + const {rerender} = render(test) + + rerender(test) + + expect(getLiveRegion().getMessage('polite')).toBe('') + }) + it('cancels delayed content after unmounting', async () => { const {unmount} = render(test)