From 02ad60f82bd8800670e0194aabfe3433f9c61398 Mon Sep 17 00:00:00 2001 From: Owen Niblock Date: Tue, 16 Dec 2025 10:31:57 +0000 Subject: [PATCH 1/4] Only add the loading id when loading is shown --- packages/react/src/Button/ButtonBase.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/react/src/Button/ButtonBase.tsx b/packages/react/src/Button/ButtonBase.tsx index ba376b52eae..7fa48d7cfb8 100644 --- a/packages/react/src/Button/ButtonBase.tsx +++ b/packages/react/src/Button/ButtonBase.tsx @@ -56,6 +56,9 @@ const ButtonBase = forwardRef(({children, as: Component = 'button', ...props}, f const uuid = useId(id) const loadingAnnouncementID = `${uuid}-loading-announcement` + // Only include the loading aria-description if there is a loading state + const ariaDescribedByIds = loading ? [loadingAnnouncementID, ariaDescribedBy] : [ariaDescribedBy] + if (__DEV__) { /** * The Linter yells because it thinks this conditionally calls an effect, @@ -100,9 +103,7 @@ const ButtonBase = forwardRef(({children, as: Component = 'button', ...props}, f data-variant={variant} data-label-wrap={labelWrap} data-has-count={count !== undefined ? true : undefined} - aria-describedby={[loadingAnnouncementID, ariaDescribedBy] - .filter(descriptionID => Boolean(descriptionID)) - .join(' ')} + aria-describedby={ariaDescribedByIds.filter(descriptionID => Boolean(descriptionID)).join(' ')} // aria-labelledby is needed because the accessible name becomes unset when the button is in a loading state. // We only set it when the button is in a loading state because it will supersede the aria-label when the screen // reader announces the button name. From 55f0e94eba969631b05daa725c1adbe65773325d Mon Sep 17 00:00:00 2001 From: Owen Niblock Date: Tue, 16 Dec 2025 10:34:02 +0000 Subject: [PATCH 2/4] Changeset --- .changeset/ripe-baths-rush.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/ripe-baths-rush.md diff --git a/.changeset/ripe-baths-rush.md b/.changeset/ripe-baths-rush.md new file mode 100644 index 00000000000..b4274f32094 --- /dev/null +++ b/.changeset/ripe-baths-rush.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Only shows the aria-describedby id for loading when the component is in the loading state From aeac796a6dabeeda83c86a98fccfffa7f31e12b2 Mon Sep 17 00:00:00 2001 From: Owen Niblock Date: Tue, 16 Dec 2025 10:56:19 +0000 Subject: [PATCH 3/4] Copilot review changes --- packages/react/src/Button/ButtonBase.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react/src/Button/ButtonBase.tsx b/packages/react/src/Button/ButtonBase.tsx index 7fa48d7cfb8..d2ae5a0b6ba 100644 --- a/packages/react/src/Button/ButtonBase.tsx +++ b/packages/react/src/Button/ButtonBase.tsx @@ -56,7 +56,7 @@ const ButtonBase = forwardRef(({children, as: Component = 'button', ...props}, f const uuid = useId(id) const loadingAnnouncementID = `${uuid}-loading-announcement` - // Only include the loading aria-description if there is a loading state + // Only include the loading aria-describedby if there is a loading state const ariaDescribedByIds = loading ? [loadingAnnouncementID, ariaDescribedBy] : [ariaDescribedBy] if (__DEV__) { @@ -103,7 +103,7 @@ const ButtonBase = forwardRef(({children, as: Component = 'button', ...props}, f data-variant={variant} data-label-wrap={labelWrap} data-has-count={count !== undefined ? true : undefined} - aria-describedby={ariaDescribedByIds.filter(descriptionID => Boolean(descriptionID)).join(' ')} + aria-describedby={ariaDescribedByIds.filter(descriptionID => Boolean(descriptionID)).join(' ') || undefined} // aria-labelledby is needed because the accessible name becomes unset when the button is in a loading state. // We only set it when the button is in a loading state because it will supersede the aria-label when the screen // reader announces the button name. From d9dfdf3847ab9b92fe432ded8d82db39a6ce9401 Mon Sep 17 00:00:00 2001 From: Owen Niblock Date: Tue, 16 Dec 2025 11:18:00 +0000 Subject: [PATCH 4/4] Fix tests --- packages/react/src/Button/__tests__/Button.test.tsx | 8 ++++++++ .../Button/__tests__/__snapshots__/Button.test.tsx.snap | 7 ------- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/react/src/Button/__tests__/Button.test.tsx b/packages/react/src/Button/__tests__/Button.test.tsx index 4250ff75396..af00f249721 100644 --- a/packages/react/src/Button/__tests__/Button.test.tsx +++ b/packages/react/src/Button/__tests__/Button.test.tsx @@ -177,6 +177,8 @@ describe('Button', () => { ) const buttonNode = container.getByRole('button') + fireEvent.click(buttonNode) + expect(buttonNode.getAttribute('aria-describedby')).toBe(`${buttonId}-loading-announcement`) fireEvent.click(buttonNode) @@ -199,6 +201,8 @@ describe('Button', () => { ) const buttonNode = container.getByRole('button') + fireEvent.click(buttonNode) + expect(buttonNode.getAttribute('aria-describedby')).toBe(`${buttonId}-loading-announcement`) fireEvent.click(buttonNode) @@ -220,6 +224,10 @@ describe('Button', () => { content , ) + const buttonNode = container.getByRole('button') + + fireEvent.click(buttonNode) + const buttonDescribedBy = container.getByRole('button').getAttribute('aria-describedby') const loadingAnnouncementId = `${buttonId}-loading-announcement` diff --git a/packages/react/src/Button/__tests__/__snapshots__/Button.test.tsx.snap b/packages/react/src/Button/__tests__/__snapshots__/Button.test.tsx.snap index 0ddfa63424b..0e24734daa4 100644 --- a/packages/react/src/Button/__tests__/__snapshots__/Button.test.tsx.snap +++ b/packages/react/src/Button/__tests__/__snapshots__/Button.test.tsx.snap @@ -2,7 +2,6 @@ exports[`Button > respects block prop 1`] = `