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

[InfraUI] [LogsUI] Accessibility fixes #41883

Merged
merged 7 commits into from
Aug 2, 2019

Conversation

Kerry350
Copy link
Contributor

@Kerry350 Kerry350 commented Jul 24, 2019

Summary

This PR addresses 3 2 accessibility issues. The individual commits contain the relevant issue number, and should be easy to review in isolation.

The only issue that introduces any significant change is #40419. For this I opted to use EUI's EuiDescribedFormGroup component. This component adds the relevant role, id and aria-labelledby attributes. However, it does introduce a visual alignment change. To avoid a lot of whitespace to the left I've made use of the description prop.

Checklist

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

@Kerry350 Kerry350 added review release_note:fix Feature:Metrics UI Metrics UI feature 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 :infrastructure labels Jul 24, 2019
@Kerry350 Kerry350 requested a review from a team July 24, 2019 11:37
@Kerry350 Kerry350 self-assigned this Jul 24, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-logs-ui

@Kerry350
Copy link
Contributor Author

Ah, some of the functional tests were relying on data-test-subj attributes attached to the titles that have been changed. I'll fix this.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@Kerry350
Copy link
Contributor Author

I've reverted the #40419 fix in this PR, as those changes will very soon be superceded by #42243 and they were quite large. This PR is now pretty small only addressing the other 2.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@jasonrhodes jasonrhodes requested review from jasonrhodes and removed request for a team July 31, 2019 12:15
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 thanks!

@Kerry350 Kerry350 requested a review from a team as a code owner August 1, 2019 22:39
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticdog
Copy link
Contributor

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Kerry350 added a commit to Kerry350/kibana that referenced this pull request Aug 2, 2019
* Fixes elastic#40411 - Add aria-label to search input

* Fixes elastic#40425 - Align gear icon aria-labels

* Fixes elastic#40419 - Make use of EuiDescribedFormGroup in the source configuration panel

* Add data-test-subj attribute back for functional test

* Revert "Fixes elastic#40419 - Make use of EuiDescribedFormGroup in the source configuration panel"

This reverts commit 5075777.
Kerry350 added a commit that referenced this pull request Aug 5, 2019
* Fixes #40411 - Add aria-label to search input

* Fixes #40425 - Align gear icon aria-labels

* Fixes #40419 - Make use of EuiDescribedFormGroup in the source configuration panel

* Add data-test-subj attribute back for functional test

* Revert "Fixes #40419 - Make use of EuiDescribedFormGroup in the source configuration panel"

This reverts commit 5075777.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Logs UI Logs UI feature Feature:Metrics UI Metrics UI feature release_note:fix review Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants