TextInput - Accessibility review recommended updates

[pksjce]

Describe your changes here.

Here, I've made a few recommended updates as suggested by the accessibility review in https://github.com/github/primer/issues/538

• Removed inline validation from the stories. It's not a feature of textinput. But its not recommended for accessibility. Page level validation is preferred as my understanding
• Deprecate loading related props. Also remove aria-live and aria-busy values.
• Remove examples of placeholder usage. I don't think we should block it though. It's a feature of <input /> html element.

Merge checklist

• [x] Added/updated tests
• [x] Added/updated documentation
• [x] Tested in Chrome
• [x] Tested in Firefox
• [x] 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[bot]]

⚠️ No Changeset found

Latest commit: ab25520436983062f4268ace1035d5a8b8dbd969

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[github-actions[bot]]

size-limit report 📦

Path	Size
dist/browser.esm.js	76.24 KB (-0.03% 🔽)
dist/browser.umd.js	76.85 KB (-0.03% 🔽)

[mperrotti]

I'm working on adding a validation example that aligns with what we defined in the interface guidelines: https://github.com/primer/react/pull/2315

[github-actions[bot]]

Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.

[lesliecdubs]

@mperrotti what are your thoughts on next steps on this PR - do we need Allie's input to unblock? I'd hate to let this work go stale while @pksjce is on leave.

[mperrotti]

Based on feedback during a11y office hours, I think we to remove aria-live and aria-busy.

the react input has aria-live and aria-busy and this was reported a long time ago. It means that a screenreader will read out the input as you type into it which as you can imagine is really annoying. — Slack message (only available to Hubbers)

---

Before moving forward, I'd like to get feedback from @alliethu or somebody else from the a11y team about my review comment.

[github-actions[bot]]

Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.

[alliethu]

@mperrotti @lesliecdubs apologies for missing all these pings!

Removed inline validation from the stories. It's not a feature of textinput. But its not recommended for accessibility. Page level validation is preferred as my understanding

@alliethu - can we really not have an error message at the text input level? I thought this was ok as long as we followed the guidelines defined here: https://primer.style/design/ui-patterns/forms#validation

I just had a chance to review the very thorough forms validation guidelines and agree that it would be ok assuming guidelines are followed. I'm cc/ing @primer/accessibility-design to keep me honest.

Deprecate loading related props. Also remove aria-live and aria-busy values.

I don't think we actually want to remove these.

@alliethu's review assumes that the loading indicator is only used for inline validation. This is not the case - we would use a loading state when a text input is performing a search.

I think you're right that my review was based on the assumption the loading indicator was only being used for inline validation. Again, I would default to the team to get this a second look, as it's been so long since I performed this review.

[ichelsea]

Agree that inline validation is okay per the guidelines in our form validation guidance! I think it would be good to also have examples of places this is okay outside of a failed validation re-do, like a password field checking if a username is available on sign up.

Based on feedback during a11y office hours, I think we to remove aria-live and aria-busy.

We can remove these. If we left them we'd want to add an additional element of timing; when the validation message is read off and trying to come up with just the right timing would get complex. For example, if the validation waits 5 seconds after a user stops typing to read out the validation. Too quickly and it's annoying, too slow and the user might miss it. We just want to be sure that the user has some form of reading that inline validation.

[lesliecdubs]

Thanks folks!

Based on feedback during a11y office hours, I think we to remove aria-live and aria-busy.

@mperrotti are you comfortable executing on this change ^ directly on the branch in this PR so we can move toward merge? Let me know if you want support to get this over the line.

[github-actions[bot]]

Hi! This pull request has been marked as stale because it has been open with no activity for 60 days. You can comment on the pull request or remove the stale label to keep it open. If you do nothing, this pull request will be closed in 7 days.