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

fix(help-text): apply aria-live to ensure full help text is read to user #4210

Merged
merged 6 commits into from
Oct 24, 2024

Conversation

Westbrook
Copy link
Contributor

Description

Use aria-live="assertive" to ensure the Help Text content (when changing) has higher screen reader priority than Textfield input.

Related issue(s)

How has this been tested?

  • Test case 1
    1. Go here
    2. Turn on your screen reader
    3. Focus the Stay "Positive" text field
    4. Type or delete anything and then keep typing
    5. Ensure that the screen reader reads ALL of the updated help text when moving on and off of the value Positive

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • If my change required a change to the documentation, I have updated the documentation in this pull request.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have reviewed at the Accessibility Practices for this feature, see: Aria Practices

@Westbrook Westbrook requested review from jnurthen and a team March 22, 2024 14:52
Copy link

github-actions bot commented Mar 22, 2024

Branch preview

Visual regression test results

When a visual regression test fails (or has previously failed while working on this branch), its results can be found in the following URLs:

Copy link

github-actions bot commented Mar 22, 2024

Lighthouse scores

Category Latest (report) Main (report) Branch (report)
Performance 0.99 0.99 0.99
Accessibility 1 1 1
Best Practices 1 1 1
SEO 1 0.92 0.92
PWA 1 1 1
What is this?

Lighthouse scores comparing the documentation site built from the PR ("Branch") to that of the production documentation site ("Latest") and the build currently on main ("Main"). Higher scores are better, but note that the SEO scores on Netlify URLs are artifically constrained to 0.92.

Transfer Size

Category Latest Main Branch
Total 228.101 kB 217.372 kB 214.491 kB 🏆
Scripts 57.744 kB 52.723 kB 49.971 kB 🏆
Stylesheet 34.499 kB 30.21 kB 30.111 kB 🏆
Document 6.221 kB 5.465 kB 5.391 kB 🏆
Font 126.809 kB 126.623 kB 126.623 kB

Request Count

Category Latest Main Branch
Total 52 52 51 🏆
Scripts 41 41 40 🏆
Stylesheet 5 5 5
Document 1 1 1
Font 2 2 2

Copy link

github-actions bot commented Mar 22, 2024

Tachometer results

Chrome

color-field permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 503 kB 43.09ms - 44.04ms - faster ✔
3% - 6%
1.38ms - 2.66ms
branch 479 kB 45.16ms - 46.02ms slower ❌
3% - 6%
1.38ms - 2.66ms
-

combobox permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 765 kB 37.77ms - 38.22ms - faster ✔
2% - 4%
0.95ms - 1.60ms
branch 720 kB 39.04ms - 39.51ms slower ❌
2% - 4%
0.95ms - 1.60ms
-

light-dom-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 765 kB 403.97ms - 410.21ms - faster ✔
1% - 3%
3.33ms - 13.19ms
branch 720 kB 411.53ms - 419.17ms slower ❌
1% - 3%
3.33ms - 13.19ms
-

field-group permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 437 kB 44.00ms - 45.94ms - unsure 🔍
-5% - +0%
-2.41ms - +0.10ms
branch 413 kB 45.33ms - 46.92ms unsure 🔍
-0% - +5%
-0.10ms - +2.41ms
-

help-text permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 403 kB 10.00ms - 10.16ms - faster ✔
1% - 4%
0.15ms - 0.43ms
branch 379 kB 10.25ms - 10.48ms slower ❌
1% - 4%
0.15ms - 0.43ms
-

number-field permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 541 kB 70.36ms - 74.34ms - faster ✔
6% - 14%
4.75ms - 11.07ms
branch 517 kB 77.81ms - 82.71ms slower ❌
6% - 15%
4.75ms - 11.07ms
-

radio permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 421 kB 35.71ms - 36.97ms - faster ✔
0% - 4%
0.05ms - 1.52ms
branch 398 kB 36.75ms - 37.50ms slower ❌
0% - 4%
0.05ms - 1.52ms
-

search permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 488 kB 41.34ms - 43.10ms - faster ✔
4% - 15%
1.85ms - 7.40ms
branch 464 kB 44.21ms - 49.48ms slower ❌
4% - 18%
1.85ms - 7.40ms
-

slider permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 496 kB 75.50ms - 77.28ms - faster ✔
3% - 5%
2.13ms - 4.29ms
branch 472 kB 78.98ms - 80.21ms slower ❌
3% - 6%
2.13ms - 4.29ms
-

textfield permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 448 kB 23.58ms - 24.07ms - faster ✔
8% - 11%
2.22ms - 3.01ms
branch 425 kB 26.13ms - 26.74ms slower ❌
9% - 13%
2.22ms - 3.01ms
-
Firefox

color-field permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 511 kB 82.33ms - 85.91ms - unsure 🔍
-4% - +1%
-3.76ms - +0.52ms
branch 487 kB 84.56ms - 86.92ms unsure 🔍
-1% - +5%
-0.52ms - +3.76ms
-

combobox permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 765 kB 61.98ms - 64.30ms - unsure 🔍
-4% - +0%
-2.70ms - +0.18ms
branch 720 kB 63.55ms - 65.25ms unsure 🔍
-0% - +4%
-0.18ms - +2.70ms
-

light-dom-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 765 kB 746.75ms - 767.37ms - slower ❌
2% - 6%
17.56ms - 40.44ms
branch 720 kB 723.12ms - 733.00ms faster ✔
2% - 5%
17.56ms - 40.44ms
-

field-group permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 437 kB 100.61ms - 107.59ms - unsure 🔍
-9% - +0%
-9.67ms - +0.39ms
branch 413 kB 105.12ms - 112.36ms unsure 🔍
-0% - +9%
-0.39ms - +9.67ms
-

help-text permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 403 kB 20.98ms - 22.86ms - unsure 🔍
-9% - +3%
-1.96ms - +0.72ms
branch 379 kB 21.59ms - 23.49ms unsure 🔍
-3% - +9%
-0.72ms - +1.96ms
-

number-field permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 541 kB 154.22ms - 162.06ms - faster ✔
6% - 11%
9.52ms - 19.40ms
branch 517 kB 169.60ms - 175.60ms slower ❌
6% - 12%
9.52ms - 19.40ms
-

radio permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 421 kB 73.93ms - 78.95ms - unsure 🔍
-9% - +1%
-7.66ms - +0.90ms
branch 398 kB 76.36ms - 83.28ms unsure 🔍
-1% - +10%
-0.90ms - +7.66ms
-

search permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 488 kB 82.64ms - 87.64ms - faster ✔
1% - 9%
0.96ms - 8.44ms
branch 464 kB 87.06ms - 92.62ms slower ❌
1% - 10%
0.96ms - 8.44ms
-

slider permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 496 kB 158.37ms - 164.47ms - faster ✔
2% - 7%
2.72ms - 12.60ms
branch 472 kB 165.20ms - 172.96ms slower ❌
2% - 8%
2.72ms - 12.60ms
-

textfield permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 448 kB 47.22ms - 50.74ms - faster ✔
2% - 12%
1.13ms - 6.39ms
branch 425 kB 50.79ms - 54.69ms slower ❌
2% - 13%
1.13ms - 6.39ms
-

@Westbrook Westbrook force-pushed the assertive-help-text branch from 3e192e7 to b45a997 Compare March 22, 2024 17:09
@@ -43,7 +43,10 @@ export class HelpTextManager {
// `pass-through-help-text-${this.instanceCount}` makes the slot effectively unreachable from
// the outside allowing the `help-text` slot to be preferred while `negative === false`.
return html`
<div id=${ifDefined(this.isInternal ? this.id : undefined)}>
<div
id=${ifDefined(this.isInternal ? this.id : undefined)}
Copy link
Contributor

Choose a reason for hiding this comment

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

Does a truthy this.isInternal always specify that an id will be present?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it does. And, while it's technically possible that a custom element consumer would unset the value of ID, but that would be highly irregular. Without negative intent, the id as set in the constructor (note that this is a helper class and not a custom element class) would always be available for use here.

As this isn't new functionality, if there's a concern we want to address here, it would be better to track it in an issue for a later update.

@hunterloftis
Copy link
Contributor

hunterloftis commented Mar 25, 2024

Should I expect "Tell us how you are feeling today" and/or "Please be positive?"

I get neither - instead, VoiceOver gives me (after entering "Test" into the field):

  • Test
  • More content available, You are currently on a text field.
  • (Tab off, then on)
  • Test, text contents selected, invalid data, edit.

@Westbrook Westbrook marked this pull request as draft March 25, 2024 20:52
@Westbrook Westbrook force-pushed the assertive-help-text branch from b45a997 to 5fdd568 Compare March 25, 2024 20:53
@Westbrook Westbrook force-pushed the assertive-help-text branch from 5fdd568 to 4d6a065 Compare May 3, 2024 11:54
@Rajdeepc Rajdeepc requested a review from nikkimk May 22, 2024 11:29
nikkimk
nikkimk previously approved these changes Jun 19, 2024
Copy link
Contributor

@nikkimk nikkimk left a comment

Choose a reason for hiding this comment

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

It worked, but we should have some examples of how to use it accessibly in the docs.

@Rajdeepc
Copy link
Contributor

Rajdeepc commented Jul 9, 2024

@nikkimk Would you like to take this up and complete this?

@nikkimk nikkimk mentioned this pull request Aug 12, 2024
24 tasks
@nikkimk nikkimk requested review from Rajdeepc and nikkimk September 5, 2024 18:35
Copy link
Contributor

@nikkimk nikkimk left a comment

Choose a reason for hiding this comment

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

More docs will be added with #4652 so lgtm

Copy link
Contributor

@Rajdeepc Rajdeepc left a comment

Choose a reason for hiding this comment

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

This looks very clean now and helpful! Let me know when it will be ready for review. Thanks

@nikkimk nikkimk marked this pull request as ready for review October 18, 2024 14:10
@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 11405330661

Details

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.002%) to 98.227%

Totals Coverage Status
Change from base Build 11386937763: -0.002%
Covered Lines: 32986
Relevant Lines: 33398

💛 - Coveralls

@nikkimk
Copy link
Contributor

nikkimk commented Oct 22, 2024

@Rajdeepc ready for review

Copy link
Contributor

@Rajdeepc Rajdeepc left a comment

Choose a reason for hiding this comment

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

Good work!

@nikkimk nikkimk merged commit 049dc56 into main Oct 24, 2024
61 checks passed
@nikkimk nikkimk deleted the assertive-help-text branch October 24, 2024 13:18
nikkimk added a commit that referenced this pull request Nov 19, 2024
…ser (#4210)

* fix(help-text): apply aria-live to ensure full help text is read to user

* docs(help-text): added accessible context to examples

* docs(help-text): updated help-text docs to include accessibility information

---------

Co-authored-by: Nikki Massaro <[email protected]>
Co-authored-by: Nikki Massaro <[email protected]>
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.

[Bug]: sp-help-text error message suppressed when continue typing
5 participants