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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Westbrook
Copy link
Collaborator

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

Lighthouse scores

Category Latest (report) Main (report) Branch (report)
Performance 0.94 0.95 0.98
Accessibility 1 1 1
Best Practices 0.96 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 222.31 kB 210.469 kB 🏆 217.24 kB
Scripts 53.819 kB 48.187 kB 🏆 54.534 kB
Stylesheet 35.007 kB 30.427 kB 🏆 30.794 kB
Document 5.926 kB 5.175 kB 🏆 5.20 kB
Font 127.558 kB 126.68 kB 🏆 126.712 kB

Request Count

Category Latest Main Branch
Total 47 45 43 🏆
Scripts 37 37 35 🏆
Stylesheet 5 5 5
Document 1 1 1
Font 4 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 488 kB 40.05ms - 41.84ms - faster ✔
0% - 6%
0.16ms - 2.50ms
branch 475 kB 41.52ms - 43.03ms slower ❌
0% - 6%
0.16ms - 2.50ms
-

combobox permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 708 kB 35.79ms - 36.21ms - faster ✔
3% - 5%
0.97ms - 1.86ms
branch 696 kB 37.02ms - 37.81ms slower ❌
3% - 5%
0.97ms - 1.86ms
-

light-dom-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 709 kB 383.54ms - 389.73ms - faster ✔
2% - 5%
9.76ms - 18.54ms
branch 696 kB 397.67ms - 403.90ms slower ❌
3% - 5%
9.76ms - 18.54ms
-

field-group permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 422 kB 42.95ms - 44.29ms - faster ✔
2% - 6%
0.75ms - 2.55ms
branch 409 kB 44.67ms - 45.87ms slower ❌
2% - 6%
0.75ms - 2.55ms
-

help-text permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 387 kB 9.88ms - 10.01ms - faster ✔
3% - 4%
0.27ms - 0.46ms
branch 375 kB 10.23ms - 10.38ms slower ❌
3% - 5%
0.27ms - 0.46ms
-

number-field permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 523 kB 65.51ms - 66.82ms - faster ✔
11% - 14%
7.91ms - 10.40ms
branch 511 kB 74.26ms - 76.38ms slower ❌
12% - 16%
7.91ms - 10.40ms
-

radio permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 406 kB 34.83ms - 35.74ms - faster ✔
1% - 5%
0.44ms - 1.84ms
branch 394 kB 35.90ms - 36.96ms slower ❌
1% - 5%
0.44ms - 1.84ms
-

search permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 472 kB 37.58ms - 38.42ms - faster ✔
4% - 7%
1.46ms - 2.75ms
branch 459 kB 39.62ms - 40.59ms slower ❌
4% - 7%
1.46ms - 2.75ms
-

slider permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 480 kB 74.58ms - 76.29ms - faster ✔
3% - 6%
2.42ms - 5.15ms
branch 467 kB 78.15ms - 80.28ms slower ❌
3% - 7%
2.42ms - 5.15ms
-

textfield permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 433 kB 23.02ms - 23.62ms - faster ✔
10% - 14%
2.60ms - 3.63ms
branch 420 kB 26.02ms - 26.85ms slower ❌
11% - 16%
2.60ms - 3.63ms
-
Firefox

color-field permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 496 kB 74.77ms - 77.95ms - unsure 🔍
-4% - +1%
-3.21ms - +0.89ms
branch 483 kB 76.22ms - 78.82ms unsure 🔍
-1% - +4%
-0.89ms - +3.21ms
-

combobox permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 708 kB 61.37ms - 65.67ms - slower ❌
1% - 9%
0.79ms - 5.21ms
branch 696 kB 59.99ms - 61.05ms faster ✔
1% - 8%
0.79ms - 5.21ms
-

light-dom-test permalink

Version Bytes Avg Time vs remote vs branch
npm latest 709 kB 696.35ms - 711.01ms - slower ❌
0% - 3%
2.56ms - 22.48ms
branch 696 kB 684.41ms - 697.91ms faster ✔
0% - 3%
2.56ms - 22.48ms
-

field-group permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 422 kB 99.67ms - 103.57ms - faster ✔
1% - 6%
1.17ms - 6.67ms
branch 409 kB 103.59ms - 107.49ms slower ❌
1% - 7%
1.17ms - 6.67ms
-

help-text permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 387 kB 19.16ms - 20.56ms - unsure 🔍
-6% - +1%
-1.26ms - +0.30ms
branch 375 kB 19.99ms - 20.69ms unsure 🔍
-2% - +6%
-0.30ms - +1.26ms
-

number-field permalink

basic-test

Version Bytes Avg Time vs remote vs branch
npm latest 523 kB 144.24ms - 151.04ms - faster ✔
6% - 12%
8.74ms - 18.90ms
branch 511 kB 157.68ms - 165.24ms slower ❌
6% - 13%
8.74ms - 18.90ms
-

radio permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 406 kB 70.46ms - 74.58ms - unsure 🔍
-6% - +2%
-4.25ms - +1.65ms
branch 394 kB 71.71ms - 75.93ms unsure 🔍
-2% - +6%
-1.65ms - +4.25ms
-

search permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 472 kB 65.09ms - 69.59ms - faster ✔
1% - 10%
0.71ms - 7.17ms
branch 459 kB 68.97ms - 73.59ms slower ❌
1% - 11%
0.71ms - 7.17ms
-

slider permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 480 kB 156.90ms - 162.70ms - faster ✔
1% - 6%
2.12ms - 10.40ms
branch 467 kB 163.11ms - 169.01ms slower ❌
1% - 7%
2.12ms - 10.40ms
-

textfield permalink

test-basic

Version Bytes Avg Time vs remote vs branch
npm latest 433 kB 48.84ms - 52.72ms - faster ✔
2% - 12%
1.00ms - 6.48ms
branch 420 kB 52.59ms - 56.45ms slower ❌
2% - 13%
1.00ms - 6.48ms
-

@@ -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
Collaborator 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
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
3 participants