[Accessibility] Route changes announcement #150461

rshen91 · 2023-02-07T17:40:50Z

Summary

Closes #149212 and closes #149213

This PR was tested with VoiceOver on Safari, Firefox and Chrome.

Checklist

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
Update and add unit tests

…nly-app-servs

…-ref HEAD~1..HEAD --fix'

…nly-app-servs

…' into add-screen-reader-only-app-servs

elasticmachine · 2023-02-13T20:39:23Z

Pinging @elastic/kibana-accessibility (Project:Accessibility)

cee-chen · 2023-02-13T21:31:36Z

packages/core/chrome/core-chrome-browser-internal/src/ui/header/nav_link.tsx

    onClick(event: React.MouseEvent<HTMLButtonElement, MouseEvent>) {
      if (event.button === 0 && !isModifiedOrPrevented(event)) {
        event.preventDefault();
+        <EuiScreenReaderLive focusRegionOnTextChange>
+          {i18n.translate('core.ui.recentLinks.linkItem.screenReaderAnnouncement.title', {
+            defaultMessage: 'type: {title}',
+            values: {
+              title: navLink?.title,
+            },
+          })}
+        </EuiScreenReaderLive>;


I don't think this usage of <EuiScreenReaderLive> within an onClick event is correct - it's not clear that it's actually rendering anywhere meaningful.

Instead of making the screen reader announcement imperative (e.g. firing on click), I think you need to make it reactive (e.g. responding to document.title changes, which it then stores in state), and only located in 1 place across all apps. Does that make sense? Happy to chat more if not.

I'll plus one to @cee-chen about responding to document.title and having this centrally located. I've experienced NVDA announce changes inconsistently when the live region is added late. Having it in the DOM on first render() and then update with document.title should give a consistent experience.

Ok thank you both! I think I'm following - updated in 226e5e0

Feb-14-2023.10-25-32.mp4

…nly-app-servs

- let's see if this fixes FTR shenanignas

cee-chen · 2023-02-23T22:47:48Z

packages/core/chrome/core-chrome-browser-internal/src/ui/header/screen_reader_a11y.tsx

@@ -38,5 +38,5 @@ export const ScreenReaderRouteAnnouncements: FC<{
    }
  }, [breadcrumbs]);

-  return <EuiScreenReaderLive focusRegionOnTextChange>{routeTitle}</EuiScreenReaderLive>;
+  return <EuiScreenReaderLive aria-live="assertive">{routeTitle}</EuiScreenReaderLive>;


@1Copenut I set aria-live to assertive to make up for the removed focus behavior - not sure if this was the right move or is overly aggressive. Do you mind testing? 🙇

from my testing: it works maybe 75% of the time on Safari+VO but not 100% of the time (for whatever reason, no idea why). Would feel better if we had JAWS and NVDA data.

This reverts commit e6bd3a9.

cee-chen · 2023-02-23T23:51:07Z

lmfao ok i figured out the other FTR failures. .reverse() mutates the array in place and as such I was mutating the observed breadcrumbs array for every other app. please fire me from a cannon into the sun

I am incredibly curious about whether it's just Canvas that throws after I fix that issue though, so I'm going to re-re-revert focusRegionOnTextChange to see what happens / how many FTR tests fail after that. Apologies for this trash fire of a commit history

cee-chen · 2023-02-24T05:07:24Z

Failures now appear to be limited to Canvas and Discover (since discover has custom h1 focusing logic). Super dumb one-off workarounds here we go!

1Copenut · 2023-02-24T16:41:06Z

I did further testing on the morning of 24 February with Safari + VoiceOver. The latest changes are solid and behavior matches what I've experienced in other live-region testing.

Safari + VO has a tendency to not announce a live region the first time it loads a view with a lot of async data and/or JS rendering, but I've witnessed that before. It also tends to smooth out when the view is loaded subsequently. I expect we'll get more consistent results with NVDA and JAWS. I'll do more testing with those screen readers as time permits.

Cee mentioned that Canvas is dynamically rebuilding the breadcrumbs as users rename the workpad. There's code in the PR to ignore this, but I was still able to trigger the live region as I edited a workpad. Let me know if I should pull code again and retest this behavior. I don't consider this worth holding up the PR--it's actually an opportunity to improve the Canvas renaming behavior with a "Save" or "Go" button.

I'll add a ticket for the Discover app to remove our earlier H1 focus workaround and point to this PR.

Much appreciated!

/cc @rshen91 @cee-chen

kibana-ci · 2023-02-24T16:46:59Z

💚 Build Succeeded

Buildkite Build
Commit: 52d555c

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`core`	414	415	+1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`core`	350.2KB	350.8KB	+557.0B

History

💚 Build #110193 succeeded 65304cd
💔 Build #110177 failed 0629d6b
💔 Build #110174 failed e6bd3a9
💔 Build #110119 failed 7f1e9df
💔 Build #110099 failed da83124
💔 Build #109844 failed b555ae2

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @rshen91

initial commit

01c7976

rshen91 added Project:Accessibility accessibility best practice labels Feb 7, 2023

Merge remote-tracking branch 'upstream/main' into add-screen-reader-o…

9f4968a

…nly-app-servs

rshen91 changed the title ~~[Accessibility] Route changes announcement through EuiScreenReaderLive~~ [Accessibility] Route changes announcement Feb 8, 2023

rshen91 self-assigned this Feb 8, 2023

rshen91 added 2 commits February 8, 2023 12:47

remove change

2e133bc

wip

2b3face

rshen91 removed the accessibility best practice label Feb 8, 2023

rshen91 and others added 10 commits February 8, 2023 14:37

wip

2832e90

add announcement to createEuiListItem()

0f58ed3

Merge remote-tracking branch 'upstream/main' into add-screen-reader-o…

ee10330

…nly-app-servs

update i18n

5b4c5ac

i18n new id

eb455b8

focusRegionOnTextChange

a9d2da8

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

ac05b30

…-ref HEAD~1..HEAD --fix'

Merge remote-tracking branch 'upstream/main' into add-screen-reader-o…

32f682a

…nly-app-servs

Merge remote-tracking branch 'origin/add-screen-reader-only-app-servs…

98349d6

…' into add-screen-reader-only-app-servs

add different ids for i18n

1bddf50

rshen91 marked this pull request as ready for review February 13, 2023 20:39

rshen91 requested a review from a team as a code owner February 13, 2023 20:39

rshen91 requested a review from cee-chen February 13, 2023 20:39

rshen91 added the release_note:skip Skip the PR/issue when compiling release notes label Feb 13, 2023

1Copenut self-requested a review February 13, 2023 20:41

rshen91 requested a review from vadimkibana February 13, 2023 20:42

cee-chen reviewed Feb 13, 2023

View reviewed changes

rshen91 added 3 commits February 14, 2023 08:23

remove EuiScreenReaderLive from within click evt

65dc82c

update for EuiScreenReaderLive before collapsible nav group

226e5e0

Merge remote-tracking branch 'upstream/main' into add-screen-reader-o…

27c4f97

…nly-app-servs

Remove focusRegionOnTextChange

e6bd3a9

- let's see if this fixes FTR shenanignas

cee-chen reviewed Feb 23, 2023

View reviewed changes

cee-chen added 2 commits February 23, 2023 15:48

who was definitely not mutating an array in place? me :|

87797b9

Revert "Remove focusRegionOnTextChange" - for science

0629d6b

This reverts commit e6bd3a9.

cee-chen added 2 commits February 23, 2023 20:57

Skip focusRegionOnTextChange behavior only for canvas and discover

84243b4

Account for non-Elastic brand page titles

65304cd

Merge branch 'main' into add-screen-reader-only-app-servs

0489ef4

rshen91 enabled auto-merge (squash) February 24, 2023 14:17

rshen91 disabled auto-merge February 24, 2023 14:17

rshen91 enabled auto-merge (squash) February 24, 2023 14:17

rshen91 disabled auto-merge February 24, 2023 14:35

rshen91 added 3 commits February 24, 2023 07:56

add a test

1857a06

add more tests

0584f62

beef up some testing

52d555c

rshen91 enabled auto-merge (squash) February 24, 2023 16:44

rshen91 merged commit d2453be into elastic:main Feb 24, 2023

rshen91 deleted the add-screen-reader-only-app-servs branch February 24, 2023 16:47

kibanamachine added v8.8.0 backport:skip This commit does not require backporting labels Feb 24, 2023

1Copenut mentioned this pull request Feb 24, 2023

(Accessibility) Discover - remove custom H1 focus on render now that global solution is in place #152131

Closed

vadimkibana mentioned this pull request Feb 27, 2023

(Accessibility) Implement client routing solution for all products #135760

Closed

2 tasks

vadimkibana mentioned this pull request Mar 28, 2023

[Accessibility] Audit route announcements in Kibana #149214

Closed

3 tasks

rshen91 mentioned this pull request Apr 10, 2023

[EBT] Add the page's title and the URL's path to the events' context #149249

Closed

jughosta mentioned this pull request Aug 3, 2023

(Accessibility) Skip links to main content #38980

Closed

1Copenut mentioned this pull request Aug 10, 2023

[Security Solution] Rule details page jumps to the top on switching between tabs #163606

Closed

alexwizp mentioned this pull request Mar 5, 2024

[EuiHeaderBreadcrumbs] Breadcrumb changes announcement elastic/eui#7550

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Accessibility] Route changes announcement #150461

[Accessibility] Route changes announcement #150461

rshen91 commented Feb 7, 2023 •

edited

Loading

elasticmachine commented Feb 13, 2023

cee-chen Feb 13, 2023

1Copenut Feb 13, 2023

rshen91 Feb 14, 2023 •

edited

Loading

rshen91 Feb 14, 2023

cee-chen Feb 23, 2023

cee-chen Feb 23, 2023

cee-chen commented Feb 23, 2023 •

edited

Loading

cee-chen commented Feb 24, 2023 •

edited

Loading

1Copenut commented Feb 24, 2023

kibana-ci commented Feb 24, 2023

[Accessibility] Route changes announcement #150461

[Accessibility] Route changes announcement #150461

Conversation

rshen91 commented Feb 7, 2023 • edited Loading

Summary

Checklist

elasticmachine commented Feb 13, 2023

cee-chen Feb 13, 2023

Choose a reason for hiding this comment

1Copenut Feb 13, 2023

Choose a reason for hiding this comment

rshen91 Feb 14, 2023 • edited Loading

Choose a reason for hiding this comment

rshen91 Feb 14, 2023

Choose a reason for hiding this comment

cee-chen Feb 23, 2023

Choose a reason for hiding this comment

cee-chen Feb 23, 2023

Choose a reason for hiding this comment

cee-chen commented Feb 23, 2023 • edited Loading

cee-chen commented Feb 24, 2023 • edited Loading

1Copenut commented Feb 24, 2023

kibana-ci commented Feb 24, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Page load bundle

History

rshen91 commented Feb 7, 2023 •

edited

Loading

rshen91 Feb 14, 2023 •

edited

Loading

cee-chen commented Feb 23, 2023 •

edited

Loading

cee-chen commented Feb 24, 2023 •

edited

Loading