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.

Sign up for GitHub

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

Jump to bottom

[Logs UI][a11y] Announce name of column on remove column button #41695

Merged
merged 15 commits into from
Aug 2, 2019
Merged

[Logs UI][a11y] Announce name of column on remove column button #41695

merged 15 commits into from
Aug 2, 2019

Conversation

Zacqary
Copy link
Contributor

@Zacqary Zacqary commented Jul 22, 2019

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.

voiceoverfix

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@Zacqary Zacqary added Project:Accessibility WCAG A v8.0.0 Feature:Logs UI Logs UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.4.0 labels Jul 22, 2019
@Zacqary Zacqary requested a review from a team July 22, 2019 18:46
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-logs-ui

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@Zacqary
Copy link
Contributor Author

Zacqary commented Jul 23, 2019

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@hbharding hbharding self-requested a review July 24, 2019 21:42
hbharding
hbharding reviewed Jul 24, 2019
View reviewed changes
Copy link
Contributor

@hbharding hbharding left a 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.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

hbharding
hbharding approved these changes Jul 29, 2019
View reviewed changes
Copy link
Contributor

@hbharding hbharding left a comment

Choose a reason for hiding this comment

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

lgtm

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

jasonrhodes
jasonrhodes approved these changes Aug 1, 2019
View reviewed changes
Copy link
Member

@jasonrhodes jasonrhodes left a 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!

@elasticmachine
Copy link
Contributor

💔 Build Failed

@Zacqary
Copy link
Contributor Author

Zacqary commented Aug 1, 2019

retest

1 similar comment
@elasticdog
Copy link
Contributor

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@Zacqary Zacqary merged commit a9e0f25 into elastic:master Aug 2, 2019
@Zacqary Zacqary deleted the 40421-column-announce branch August 2, 2019 15:20
@Zacqary Zacqary mentioned this pull request Aug 2, 2019
Merged
Zacqary added a commit to Zacqary/kibana that referenced this pull request Aug 2, 2019
@Zacqary
[Logs UI][a11y] Announce name of column on remove column button (elas…
3fee6fd 
…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
Zacqary added a commit that referenced this pull request Aug 2, 2019
@Zacqary
[Logs UI][a11y] Announce name of column on remove column button (#41695…
6b6e979 
…) (#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
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 5, 2019
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into feature/clone…
ad759ef 
…-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)
  ...
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 5, 2019
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into fix/doc_value…
b366497 
…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)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasonrhodes jasonrhodes jasonrhodes approved these changes

@hbharding hbharding hbharding approved these changes

Assignees
No one assigned
Labels
Feature:Logs UI Logs UI feature Project:Accessibility release_note:fix Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v7.4.0 v8.0.0 WCAG A
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

(Accessibility) Kibna Logs - Remove this column button does not announce the column
5 participants
@Zacqary @elasticmachine @elasticdog @jasonrhodes @hbharding