Skip to content

feat: Add a semantic message and tone to most field components#667

Merged
gnapse merged 5 commits intomainfrom
ernesto/field-message-tone
Jun 29, 2022
Merged

feat: Add a semantic message and tone to most field components#667
gnapse merged 5 commits intomainfrom
ernesto/field-message-tone

Conversation

@gnapse
Copy link
Copy Markdown
Contributor

@gnapse gnapse commented Jun 1, 2022

Short description

Test plan

Review manually the live components in the various new stories titled "Message Tone Story" in storybook

  • Reviewed for TextField
  • Reviewed for SelectField
  • Reviewed for PasswordField
  • Reviewed for TextArea
Demo
CleanShot 2022-06-01 at 16 37 07@2x

PR Checklist

  • Added tests for bugs / new features
  • Updated docs (storybooks, readme)
  • Executed npm run validate and made sure no errors / warnings were shown
  • Described changes in CHANGELOG.md
  • Bumped version in package.json and package-lock.json (npm --no-git-tag-version version <major|minor|patch>) ref
  • Updated all static build artifacts (npm run build-all)

Versioning

New minor release

@gnapse gnapse requested review from craigcarlyle and frankieyan June 1, 2022 20:44
@gnapse gnapse self-assigned this Jun 1, 2022
@gnapse gnapse changed the title Ernesto/field message tone feat: Add a semantic message and tone to most field components Jun 1, 2022
type ChildrenRenderProps = {
id: string
'aria-describedby'?: string
'aria-errormessage'?: string
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe it's not an issue since the markup here is similar to mdn's example, but the error message itself isn't read when the field receives focus. It only gets read when you traverse to the message itself, but even then there's no indication that it's linked to the text field:

image
image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Apparently, support for this is not great on VoiceOver (or in general) 😕

https://a11ysupport.io/tech/aria/aria-errormessage_attribute

cc @craigcarlyle what do you think we could do? Maybe we make it part of the description all the time? I tested this, and VoiceOver still announces that the field is marked as invalid. But it does not announce the error text. So having it in the description would have it read out loud.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@gnapse Since aria-errormessage came out in WAI-ARIA 1.1 in 2017 and is still not supported across assistive software I think we have no choice but to use aria-describedby.

I think the suggestion here is the way to go:
Screen Shot 2022-06-06 at 3 15 04 PM

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in the last commit.

@gnapse gnapse force-pushed the ernesto/field-message-tone branch from 731a0a7 to 5f6c836 Compare June 29, 2022 20:47
@gnapse gnapse merged commit df21fe4 into main Jun 29, 2022
@gnapse gnapse deleted the ernesto/field-message-tone branch June 29, 2022 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants