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

[ML] (Accessibility) "Analyzing Data" should announce #32472

Merged
merged 3 commits into from
Mar 7, 2019

Conversation

PhilippBaranovskiy
Copy link
Contributor

@PhilippBaranovskiy PhilippBaranovskiy commented Mar 5, 2019

FIX: #31111

Summary

Now the "Analyzing data" popup pronouns itself with ScreenReader by using ScreenReaderOnly component from eUI.
Tested with ChromeVox.

@PhilippBaranovskiy PhilippBaranovskiy requested a review from a team as a code owner March 5, 2019 11:52
@elasticmachine
Copy link
Contributor

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@PhilippBaranovskiy
Copy link
Contributor Author

retest

1 similar comment
@jgowdyelastic
Copy link
Member

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@PhilippBaranovskiy
Copy link
Contributor Author

retest

Copy link
Contributor

@alexwizp alexwizp left a comment

Choose a reason for hiding this comment

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

Code LGTM.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

If you can make the change I suggested @rockfield I can go ahead and merge this to master, and then backport to 7.x, 7.0. Many thanks for looking at this one!

@elasticmachine
Copy link
Contributor

💔 Build Failed

@PhilippBaranovskiy
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

LGTM

@peteharverson
Copy link
Contributor

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@PhilippBaranovskiy PhilippBaranovskiy merged commit 16f9e25 into elastic:master Mar 7, 2019
@PhilippBaranovskiy PhilippBaranovskiy deleted the 31111 branch March 7, 2019 13:43
PhilippBaranovskiy added a commit to PhilippBaranovskiy/kibana that referenced this pull request Mar 7, 2019
* [ML] (Accessibility) "Analyzing Data" should announce

* moved aria role to the header tag
PhilippBaranovskiy added a commit to PhilippBaranovskiy/kibana that referenced this pull request Mar 7, 2019
* [ML] (Accessibility) "Analyzing Data" should announce

* moved aria role to the header tag
PhilippBaranovskiy added a commit that referenced this pull request Mar 7, 2019
* [ML] (Accessibility) "Analyzing Data" should announce

* moved aria role to the header tag
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants