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] Make field type icon component keyboard accessible #22708

Merged

Conversation

peteharverson
Copy link
Contributor

Fixes a couple of accessibility issues with the field type icon component, as used on the cards in the Data Visualizer, which were highlighted by #22414:

  • The icon is now keyboard accessible, allowing keyboard users to view the tooltip.
  • The text used for the aria-label attribute is now the same as that used in the tooltip referenced by the aria-describedby attribute, so that the text read out by the screen reader matches the text shown visually in the tooltip. The text also now matches the names of the types, as shown in the 'filter by type' dropdown of the Data Visualizer.

image

Related to #22414

@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui

@peteharverson peteharverson changed the title [ML] Makefield type icon component keyboard accessible [ML] Make field type icon component keyboard accessible Sep 5, 2018
Copy link
Member

@jgowdyelastic jgowdyelastic left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@walterra walterra 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

@peteharverson
Copy link
Contributor Author

jenkins test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

@peteharverson peteharverson force-pushed the ml-field-type-icon-a11y branch from 792f167 to 6b3ef33 Compare September 5, 2018 16:11
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

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