Convey Spinner to assistive technologies

[mperrotti]

Closes https://github.com/primer/react/issues/3942

Changelog

New

• Spinner accepts srText prop (default value is "Loading"

Changed

• Deprecated aria-label on Spinner
• Restricts Spinner to only accept specified props, not just any HTML attribute

Removed

Rollout strategy

• [ ] Patch release
• [x] Minor release
• [ ] Major release; if selected, include a written rollout or migration plan
• [ ] None; if selected, include a brief description as to why

When we ship the next major release, we can remove aria-label from Spinner and provide a codemod to change it to srText.

Testing & Reviewing

Merge checklist

• [x] Added/updated tests
• [x] Added/updated documentation
• [x] Added/updated previews (Storybook)
• [unsure] Changes are SSR compatible
• [x] Tested in Chrome
• [x] Tested in Firefox
• [ ] Tested in Safari
• [ ] Tested in Edge
• [ ] (GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: 7c736b8e4efc991e40938588125c8592f8145017

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/react	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions[bot]]

size-limit report 📦

Path	Size
packages/react/dist/browser.esm.js	89.72 KB (+0.1% 🔺)
packages/react/dist/browser.umd.js	89.92 KB (+0.03% 🔺)

[mperrotti]

I'm converting this back to a draft until we decide on a direction based on feedback in this discussion: https://github.com/github/primer/discussions/2976

[mperrotti]

The aria-live issues should be resolved by https://github.com/primer/react/pull/4174

[TylerJDev]

Hey @mperrotti 👋! With live-region-element being available, do you think this PR is good to go?

[lindseywild]

@mperrotti Hi! Just wanted to check in on the status of this PR 🙏🏻

[mperrotti]

Sorry @lindseywild - this PR got lost and stale. Going to update today.

Related PVC PR: https://github.com/primer/view_components/pull/2884

[mperrotti]

@lindseywild @TylerJDev @joshblack - should I remove the announcements and follow @owenniblock's PVC implementation until we've figured out how to handle multiple announcements?

Comment from the PVC PR:

I would not feel comfortable if the Spinner announced by default, I think that's too risky given the number of Spinners we have in the wild already

Thread in the ADR PR

[TylerJDev]

should I remove the announcements and follow @owenniblock's PVC implementation until we've figured out how to handle multiple announcements?

I think that's fair. Unless we can make it opt-in, where the live region only exists if the consumer turns it on themselves via a prop. Even without the live region, this PR will fix the accessibility issue on there being no context by default on the <Spinner> component.

[mperrotti]

@TylerJDev - after talking to @joshblack, I decided to remove the announcement until we land on a solution for using Status in components without accidentally triggering too many announcements.