Add sr-only option to spinner

[owenniblock]

Authors: Please fill out this form carefully and completely.

Reviewers: By approving this Pull Request you are approving the code change, as well as its deployment and mitigation plans.
Please read this description carefully. If you feel there is anything unclear or missing, please ask for updates.

What are you trying to accomplish?

Add a default, screen reader accessible text to the Spinner component. This is set when no aria-label or aria-labelledby is present.

Also when an aria-label is not set on the Spinner component, the aria-hidden attribute is set to true.

How did you accomplish it?

I added a new sr_text argument to the Spinner component. This argument is set to a default value of "Loading" if no aria-label. The aria-hidden attribute is set to true when an aria-label is not present.

Integration

No

List the issues that this change affects.

Relates to https://github.com/github/primer/issues/2202

Risk Assessment

• Low risk the change is small, highly observable, and easily rolled back.
• Medium risk changes that are isolated, reduced in scope or could impact few users. The change will not impact library availability.
• High risk changes are those that could impact customers and SLOs, low or no test coverage, low observability, or slow to rollback.

What approach did you choose and why?

I opted for a change which would allow existing aria-labels to still work.

Anything you want to highlight for special attention from reviewers?

I would love to know if we feel like it's necessary to be able to customise the sr_text. Do we just have the default if no aria-label is defined?

Merge checklist

• Added/updated tests
• Added/updated documentation
• Added/updated previews (Lookbook)
• Tested in Chrome
• Tested in Firefox
• Tested in Safari
• Tested in Edge

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

[changeset-bot]

🦋 Changeset detected

Latest commit: ef3b13a

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

This PR includes changesets to release 1 package

Name	Type
@primer/view-components	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

[mperrotti]

I think this needs to be announced in a live region. You can use https://github.com/primer/live-region-element

This was discussed a while ago in this discussion: https://github.com/github/primer/discussions/2976

Let me know if I misunderstood and I'll revise primer/react#4140 to align with this PR.

[owenniblock]

@mperrotti the idea is to eventually move to an announced version, however this first step will resolve the audit issue.

I felt like the announcement API needs some consideration (for example we don't want many Spinners on one page to all announce individually) and since the audit issues them. We could certainly add announcements (on this PR if we need to do so to get it released) however 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.

[TylerJDev]

LGTM!

I would love to know if we feel like it's necessary to be able to customise the sr_text. Do we just have the default if no aria-label is defined?

I feel like in cases where the spinner is not unique and/or what is being loaded should be defined is a good use case for sr_text customization. Outside of that I think the default "Loading" is fair more most cases.