TextInput: Add LeadingVisual/TrailingVisual to be part of the accessible description in TextInput

[TylerJDev]

Closes https://github.com/github/primer/issues/3438 https://github.com/github/primer/issues/3465

Changelog

New

• Adds aria-describedby for LeadingVisual and TrailingVisual
• Adds new prop loaderText to convey loading state to screen readers

Rollout strategy

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

Testing & Reviewing

Merge checklist

• Added/updated tests
• Added/updated documentation
• Added/updated previews (Storybook)
• Changes are SSR compatible
• Tested in Chrome
• 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)

(https://github.com/github/github/pull/341243, integration tests pass with minor changes, and some unrelated failures)

[changeset-bot]

🦋 Changeset detected

Latest commit: e9d65a2

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]

size-limit report 📦

Path	Size
packages/react/dist/browser.esm.js	97.15 KB (+0.15% 🔺)
packages/react/dist/browser.umd.js	97.43 KB (+0.04% 🔺)

[iansan5653]

🤔 I wonder if it's really necessary / a good idea for consumers to be able to customize this? It inherently means they'll be providing a different experience for sighted vs non-sighted users, because sighted users won't be able to access this text. Visually a loading indicator only indicates that something is loading, so "Loading" should be sufficient always.

Alternatively, we could make this visible by putting it in a tooltip. But that doesn't seem ideal since the loader is not focusable.

[TylerJDev]

Good question, and good point! This mainly stems from https://github.com/github/primer/issues/3465. I believe the idea is, if there's context outside of the input that conveys what is loading then it might be a good idea to add more descriptive text to indicate what is loading so users don't have to navigate outside of it to get that context.

I think this is a more specific use case that won't be used unless necessary, so in most cases having the default "Loading" is okay, which we could make more descriptive 🤔

Cc: @patrickhlauke - curious if you have any additional thoughts here?

[iansan5653]

I believe the idea is, if there's context outside of the input that conveys what is loading then it might be a good idea to add more descriptive text to indicate what is loading so users don't have to navigate outside of it to get that context.

If we do keep this prop, I think this (particularly the par about the context outside the input) seems like it would be useful to add to the props documentation. I think what may happen is consumers will see that this prop exists and think that they should fill it out similarly to how they fill out alt text in an image - they'll have good intentions but actually create a less accessible experience.

[TylerJDev]

Yup, that's totally fair @iansan5653! I can add a note on this in the props docs to make sure consumers aren't haphazardly using it!

[github-actions]

👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks!

[primer-integration]

👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/347575

[primer-integration]

🟢 golden-jobs completed with status success.