-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Logs UI][a11y] Announce name of column on remove column button #41695
Conversation
Pinging @elastic/infra-logs-ui |
💔 Build Failed |
💔 Build Failed |
retest |
💚 Build Succeeded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but I might suggest changing the Voice Over language to Remove <message> column, button
as it's faster to recognize the context. Hearing Remove this column: ...
Remove this column: ...
Remove this column: ...
over and over again requires the listener to wait for the important bit of information.
💚 Build Succeeded |
…o 40421-column-announce
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
💔 Build Failed |
💚 Build Succeeded |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM -- I don't think we need to block ourselves on waiting to test these in Windows. Most things are roughly the same enough that focusing on fixing them in "screen readers" is a good first-step goal. As they continue to audit the code, windows-only bugs may pop up and we will need a better way to be able to easily spin up a Windows test at that point, perhaps.
Thanks!
💔 Build Failed |
retest |
1 similar comment
retest |
💚 Build Succeeded |
…tic#41695) * [Logs UI][a11y] Announce name of column on remove column button * Use title instead of aria-label * ariaColumnName => columnDescription * Move template string out of i18n * Revert label id change * Inject i18n to field title * Change title to Remove {columnDescription} column * Replace injectI18N with i18n.translate * Fix i18n
…) (#42529) * [Logs UI][a11y] Announce name of column on remove column button * Use title instead of aria-label * ariaColumnName => columnDescription * Move template string out of i18n * Revert label id change * Inject i18n to field title * Change title to Remove {columnDescription} column * Replace injectI18N with i18n.translate * Fix i18n
…-or-edit-existing-rollup-job * 'master' of github.com:elastic/kibana: (67 commits) [TSVB] Shim new platform (elastic#39169) [Metric Vis] Shim new platform (elastic#42240) [Tag Cloud] Shim new platform (elastic#42348) Disable flaky request lib tests. Add es_ui_shared plugin to CODEOWNERS. Add disk space percentage to node listing (elastic#42145) [SIEM] Add chart interactions - update date picker after brush selection on charts (elastic#42440) Document HTTP service (elastic#42331) [Reporting] Sanitize 409 error log message (elastic#42495) [docs][skip ci] Maps read only access (elastic#35561) [x-pack/ftr] refactor types to be more accurate/consistent wit… (elastic#42407) [DOCS] Updates images and content in Dashboard docs (elastic#42500) Allow sorting on multiple columns in Discover (elastic#41918) [Infra UI][Logs UI] Fix autocomplete to use proper derived index pattern (elastic#42287) [ftr/cheerio] improve cheerio types to include test subject me… (elastic#42534) Upgraded EUI 13.0.0 -> 13.1.1 (elastic#42298) Increase max-old-space-size for builds (elastic#42218) [Infra UI] Add cloud metrics and cloud/host info to metadata endpoint (elastic#41836) [Logs UI][a11y] Announce name of column on remove column button (elastic#41695) Inspector 👉 New Platform (elastic#42164) Make alerting properly space aware (elastic#42081) ...
…s_autocomplete * 'master' of github.com:elastic/kibana: (189 commits) [TSVB] Shim new platform (elastic#39169) [Metric Vis] Shim new platform (elastic#42240) [Tag Cloud] Shim new platform (elastic#42348) Disable flaky request lib tests. Add es_ui_shared plugin to CODEOWNERS. Add disk space percentage to node listing (elastic#42145) [SIEM] Add chart interactions - update date picker after brush selection on charts (elastic#42440) Document HTTP service (elastic#42331) [Reporting] Sanitize 409 error log message (elastic#42495) [docs][skip ci] Maps read only access (elastic#35561) [x-pack/ftr] refactor types to be more accurate/consistent wit… (elastic#42407) [DOCS] Updates images and content in Dashboard docs (elastic#42500) Allow sorting on multiple columns in Discover (elastic#41918) [Infra UI][Logs UI] Fix autocomplete to use proper derived index pattern (elastic#42287) [ftr/cheerio] improve cheerio types to include test subject me… (elastic#42534) Upgraded EUI 13.0.0 -> 13.1.1 (elastic#42298) Increase max-old-space-size for builds (elastic#42218) [Infra UI] Add cloud metrics and cloud/host info to metadata endpoint (elastic#41836) [Logs UI][a11y] Announce name of column on remove column button (elastic#41695) Inspector 👉 New Platform (elastic#42164) Make alerting properly space aware (elastic#42081) ...
Summary
Fixes #40421
Adds the column name to "Remove this column" button descriptions when using a screen reader. Tested on Mac VoiceOver in Chrome; can someone with access to a Windows machine please test this?
Was also unable to test in Firefox because it's apparently not compatible with VoiceOver, but it does produce the correct
aria-label
attributes.Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.[ ] This was checked for cross-browser compatibility, including a check against IE11[ ] Documentation was added for features that require explanation or tutorials[ ] Unit or functional tests were updated or added to match the most common scenariosFor maintainers