Skip to content

Only add the loading id when loading is shown #7347

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
owenniblock merged 5 commits into from
Dec 18, 2025
Merged

Only add the loading id when loading is shown #7347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
02ad60f
Only add the loading id when loading is shown
owenniblock Dec 16, 2025
55f0e94
Changeset
owenniblock Dec 16, 2025
aeac796
Copilot review changes
owenniblock Dec 16, 2025
d9dfdf3
Fix tests
owenniblock Dec 16, 2025
ed5f23e
Merge branch 'main' into remove-aria-described-by-when-not-loading
owenniblock Dec 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/ripe-baths-rush.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@primer/react': patch
---

Only shows the aria-describedby id for loading when the component is in the loading state
7 changes: 4 additions & 3 deletions packages/react/src/Button/ButtonBase.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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-describedby 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,
Expand Down Expand Up @@ -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(' ') || 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.
Expand Down
8 changes: 8 additions & 0 deletions packages/react/src/Button/__tests__/Button.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ describe('Button', () => {
)
const buttonNode = container.getByRole('button')

fireEvent.click(buttonNode)
Copy link
Contributor Author

@owenniblock owenniblock Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test component shows the loading element after the button has been clicked.


expect(buttonNode.getAttribute('aria-describedby')).toBe(`${buttonId}-loading-announcement`)

fireEvent.click(buttonNode)
Expand All @@ -192,6 +194,8 @@ describe('Button', () => {
)
const buttonNode = container.getByRole('button')

fireEvent.click(buttonNode)

expect(buttonNode.getAttribute('aria-describedby')).toBe(`${buttonId}-loading-announcement`)

fireEvent.click(buttonNode)
Expand All @@ -213,6 +217,10 @@ describe('Button', () => {
<span>content</span>
</StatefulLoadingButton>,
)
const buttonNode = container.getByRole('button')

fireEvent.click(buttonNode)

const buttonDescribedBy = container.getByRole('button').getAttribute('aria-describedby')
const loadingAnnouncementId = `${buttonId}-loading-announcement`

Expand Down
7 changes: 0 additions & 7 deletions packages/react/src/Button/__tests__/__snapshots__/Button.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

exports[`Button > respects block prop 1`] = `
<button
aria-describedby="test-button-loading-announcement"
class="prc-Button-ButtonBase-Eb8-K"
data-block="block"
data-loading="false"
Expand All @@ -29,7 +28,6 @@ exports[`Button > respects block prop 1`] = `

exports[`Button > respects the alignContent prop 1`] = `
<button
aria-describedby="test-button-loading-announcement"
class="prc-Button-ButtonBase-Eb8-K"
data-loading="false"
data-no-visuals="true"
Expand All @@ -55,7 +53,6 @@ exports[`Button > respects the alignContent prop 1`] = `

exports[`Button > respects the large size prop 1`] = `
<button
aria-describedby="test-button-loading-announcement"
class="prc-Button-ButtonBase-Eb8-K"
data-loading="false"
data-no-visuals="true"
Expand All @@ -81,7 +78,6 @@ exports[`Button > respects the large size prop 1`] = `

exports[`Button > respects the small size prop 1`] = `
<button
aria-describedby="test-button-loading-announcement"
class="prc-Button-ButtonBase-Eb8-K"
data-loading="false"
data-no-visuals="true"
Expand All @@ -107,7 +103,6 @@ exports[`Button > respects the small size prop 1`] = `

exports[`Button > styles danger button appropriately 1`] = `
<button
aria-describedby="test-button-loading-announcement"
class="prc-Button-ButtonBase-Eb8-K"
data-loading="false"
data-no-visuals="true"
Expand All @@ -133,7 +128,6 @@ exports[`Button > styles danger button appropriately 1`] = `

exports[`Button > styles invisible button appropriately 1`] = `
<button
aria-describedby="test-button-loading-announcement"
class="prc-Button-ButtonBase-Eb8-K"
data-loading="false"
data-no-visuals="true"
Expand All @@ -159,7 +153,6 @@ exports[`Button > styles invisible button appropriately 1`] = `

exports[`Button > styles primary button appropriately 1`] = `
<button
aria-describedby="test-button-loading-announcement"
class="prc-Button-ButtonBase-Eb8-K"
data-loading="false"
data-no-visuals="true"
Expand Down
Loading