-
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
Improve Discover screen-reader accessibility: #11598
Improve Discover screen-reader accessibility: #11598
Conversation
- Apply aria-label to field name icons. - Change title to alt for Discover 'Back to top' link. - Remove unnecessary title attributes from Discover No Results state. - Add aria-label to Discover sidebar's string field value. - Adjust aria-label of Discover Field Chooser 'Show field settings' button.
ng-class="{ 'kuiButton--basic': !filter.active, 'kuiButton--primary': filter.active, 'hidden-xs': !showFields, 'hidden-sm': !showFields }" | ||
class="kuiButton kuiButton--small pull-right discover-field-filter-toggle" | ||
ng-click="showFilter = !showFilter" | ||
aria-label="Field Settings" | ||
aria-label="Show field settings" |
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.
I think we should use verbs to communicate to the screen reader what clicking this button will do.
ng-click="onAddFilter(field, bucket.value, '-')" data-test-subj="minus-{{::field.name}}-{{::bucket.display}}"></i> | ||
<i aria-hidden="true" class="fa fa-search-plus pull-right discover-field-details-filter" | ||
ng-click="onAddFilter(field, bucket.value, '+')" data-test-subj="plus-{{::field.name}}-{{::bucket.display}}"></i> | ||
<span aria-hidden="true" class="fa fa-search-minus pull-right discover-field-details-filter" |
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.
Not an accessibility thing, but we should never use <i>
tags.
css-truncate | ||
css-truncate-expandable="true" | ||
class="discover-field-details-value" | ||
aria-label="Value: {{:: bucket.display === '' ? 'Empty string' : bucket.display }}" |
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.
No changes here, just formatting.
</center> | ||
These are the first {{opts.sampleSize}} documents matching | ||
your search, refine your search to see others. | ||
<a ng-click="toTop()">Back to top.</a> |
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.
Not accessibility related, just removed the <center>
tag and changed the CSS to center this instead.
ng-if="failures.length === failuresShown && failures.length > 5" | ||
> | ||
Show Less | ||
</a> |
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.
Removed unnecessary title
attributes here.
jenkins, test this |
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!
|
||
const typeIcon = function (fieldType) { |
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.
😍
I'm definitely not an accessibility expert so I can't judge how well this adheres to best practices, but I did use OSX VoiceOver to confirm that all of the updated elements in this PR are accessible via screen reader. LGTM. |
- Apply aria-label to field name icons. - Change title to alt for Discover 'Back to top' link. - Remove unnecessary title attributes from Discover No Results state. - Add aria-label to Discover sidebar's string field value. - Adjust aria-label of Discover Field Chooser 'Show field settings' button.
- Apply aria-label to field name icons. - Change title to alt for Discover 'Back to top' link. - Remove unnecessary title attributes from Discover No Results state. - Add aria-label to Discover sidebar's string field value. - Adjust aria-label of Discover Field Chooser 'Show field settings' button.
- Apply aria-label to field name icons. - Change title to alt for Discover 'Back to top' link. - Remove unnecessary title attributes from Discover No Results state. - Add aria-label to Discover sidebar's string field value. - Adjust aria-label of Discover Field Chooser 'Show field settings' button.
- Apply aria-label to field name icons. - Change title to alt for Discover 'Back to top' link. - Remove unnecessary title attributes from Discover No Results state. - Add aria-label to Discover sidebar's string field value. - Adjust aria-label of Discover Field Chooser 'Show field settings' button.
Partially addresses #11520. Extracts some work from #11548.