Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more docs around different browser + screen reader behaviours for LabeledField #2403

Open
wants to merge 1 commit into
base: feature/labeled-field
Choose a base branch
from

Conversation

beaesguerra
Copy link
Member

@beaesguerra beaesguerra commented Dec 19, 2024

Summary:

Documenting the different browser + screen reader behaviours for reference since there are some browser/screen reader inconsistencies!

Issue: WB-1842

Test plan:

  • Review new LabeledField Accessibility docs (?path=/docs/packages-labeledfield-accessibility--docs)
  • Review the LabeledField stories with different screen readers/browsers, especially around errors (would appreciate any feedback around this!)

Recordings:

Interacting with fields with errors

Safari + VoiceOver

safari-vo-errors.mov

Chrome + NVDA

chrome-nvda-errors.mov

Firefox + NVDA

firefox-nvda-errors.mov

Error validation while filling out fields

Safari + VoiceOver

safari-vo-validation.mov

Chrome + NVDA

chrome-nvda-validation-3.mov

Firefox + NVDA

firefox-nvda-validation-3.mov

Error validation after submission

Safari + VoiceOver

safari-vo-submit.mov

Chrome + NVDA

chrome-nvda-submit.mov

Firefox + NVDA

firefox-nvda-submit.mov

@beaesguerra beaesguerra self-assigned this Dec 19, 2024
Copy link
Contributor

github-actions bot commented Dec 19, 2024

A new build was pushed to Chromatic! 🚀

https://5e1bf4b385e3fb0020b7073c-rlugudlftv.chromatic.com/

Chromatic results:

Metric Total
Captured snapshots 381
Tests with visual changes 0
Total stories 525
Inherited (not captured) snapshots [TurboSnap] 0
Tests on the build 381

Copy link
Contributor

github-actions bot commented Dec 19, 2024

Size Change: 0 B

Total Size: 98.8 kB

ℹ️ View Unchanged
Filename Size
packages/wonder-blocks-accordion/dist/es/index.js 3.77 kB
packages/wonder-blocks-banner/dist/es/index.js 1.53 kB
packages/wonder-blocks-birthday-picker/dist/es/index.js 1.77 kB
packages/wonder-blocks-breadcrumbs/dist/es/index.js 887 B
packages/wonder-blocks-button/dist/es/index.js 4.04 kB
packages/wonder-blocks-cell/dist/es/index.js 2.01 kB
packages/wonder-blocks-clickable/dist/es/index.js 3.06 kB
packages/wonder-blocks-core/dist/es/index.js 3.52 kB
packages/wonder-blocks-data/dist/es/index.js 6.24 kB
packages/wonder-blocks-dropdown/dist/es/index.js 19.1 kB
packages/wonder-blocks-form/dist/es/index.js 6.2 kB
packages/wonder-blocks-grid/dist/es/index.js 1.36 kB
packages/wonder-blocks-icon-button/dist/es/index.js 2.95 kB
packages/wonder-blocks-icon/dist/es/index.js 871 B
packages/wonder-blocks-labeled-field/dist/es/index.js 1.93 kB
packages/wonder-blocks-layout/dist/es/index.js 1.82 kB
packages/wonder-blocks-link/dist/es/index.js 2.28 kB
packages/wonder-blocks-modal/dist/es/index.js 5.42 kB
packages/wonder-blocks-pill/dist/es/index.js 1.65 kB
packages/wonder-blocks-popover/dist/es/index.js 4.85 kB
packages/wonder-blocks-progress-spinner/dist/es/index.js 1.52 kB
packages/wonder-blocks-search-field/dist/es/index.js 1.36 kB
packages/wonder-blocks-switch/dist/es/index.js 1.92 kB
packages/wonder-blocks-testing-core/dist/es/index.js 3.74 kB
packages/wonder-blocks-testing/dist/es/index.js 1.07 kB
packages/wonder-blocks-theming/dist/es/index.js 693 B
packages/wonder-blocks-timing/dist/es/index.js 1.8 kB
packages/wonder-blocks-tokens/dist/es/index.js 2.36 kB
packages/wonder-blocks-toolbar/dist/es/index.js 905 B
packages/wonder-blocks-tooltip/dist/es/index.js 6.99 kB
packages/wonder-blocks-typography/dist/es/index.js 1.23 kB

compressed-size-action

@beaesguerra beaesguerra changed the title Add more docs around different browser + AT behaviours for LabeledField Add more docs around different browser + screen reader behaviours for LabeledField Dec 19, 2024
@@ -25,3 +25,84 @@ prop is not provided, a unique id will be auto-generated!
- The `LabeledField` component does not need to be used with the `CheckboxGroup`
or `RadioGroup` components since those components already have accessible labels
built-in when the `label` prop is used.

Copy link
Member Author

Choose a reason for hiding this comment

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

Elaborating more around the screen reader behaviour from a previous PR comment: #2344 (comment)

@beaesguerra beaesguerra marked this pull request as ready for review December 19, 2024 00:40
@khan-actions-bot khan-actions-bot requested a review from a team December 19, 2024 00:40
@khan-actions-bot
Copy link
Contributor

Gerald

Required Reviewers
  • @Khan/wonder-blocks for changes to __docs__/wonder-blocks-labeled-field/accessibility.mdx

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

Copy link
Contributor

github-actions bot commented Dec 19, 2024

npm Snapshot: Published

🎉 Good news!! We've packaged up the latest commit from this PR (12ead53) and published all packages with changesets to npm.

You can install the packages in webapp by running:

./services/static/dev/tools/deploy_wonder_blocks.js --tag="PR2403"

Packages can also be installed manually by running:

yarn add @khanacademy/wonder-blocks-<package-name>@PR2403

Copy link

codecov bot commented Dec 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (af12901) to head (5c2cf40).

Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                @@
##   wb-1783-use-lf-in-stories   #2403   +/-   ##
=================================================
=================================================

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af12901...5c2cf40. Read the comment docs.

@beaesguerra beaesguerra force-pushed the wb-1783-use-lf-in-stories branch from 8959097 to 070a249 Compare December 19, 2024 21:45
Base automatically changed from wb-1783-use-lf-in-stories to feature/labeled-field December 19, 2024 21:49
@beaesguerra
Copy link
Member Author

The parent pull-request (#2400) has been merged into feature/labeled-field, but this branch (wb-1842) now has conflicts with the new base branch. These conflicts must be resolved before checks can complete on this pull-request.

Copy link

changeset-bot bot commented Dec 19, 2024

⚠️ No Changeset found

Latest commit: 6ea8aa7

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

Copy link
Member

@jandrade jandrade left a comment

Choose a reason for hiding this comment

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

This is soo well written and super detailed! This looks good to me, but I'd love to get a review from @marcysutton as well.

Copy link
Member

@marcysutton marcysutton left a comment

Choose a reason for hiding this comment

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

Very thorough explanations @beaesguerra, nice work!

### Error messages

The error message is implemented following the
[W3 Form Notifications On Focus Change](https://www.w3.org/WAI/tutorials/forms/notifications/#on-focus-change)
Copy link
Member

Choose a reason for hiding this comment

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

nit: adjusting the title of the W3C standards body!

Suggested change
[W3 Form Notifications On Focus Change](https://www.w3.org/WAI/tutorials/forms/notifications/#on-focus-change)
[W3C-WAI Form Notifications On Focus Change](https://www.w3.org/WAI/tutorials/forms/notifications/#on-focus-change)

so that any updates to the error message is announced.

Different browsers and screen readers announce this differently unfortunately
(in both the W3 example and LabeledField stories). The following documents the
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
(in both the W3 example and LabeledField stories). The following documents the
(in both the W3C example and LabeledField stories). The following documents the

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.

4 participants