-
Notifications
You must be signed in to change notification settings - Fork 8.5k
Improve Dashboard screen-reader accessibility. #11600
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 Dashboard screen-reader accessibility. #11600
Conversation
- Add aria-label for Dashboard Add Panel tabs. - Add aria-label for Dashboard Panel title. - Refactor empty Dashboard prompt message to improve accessibility. - Adjust aria-labels for Dashboard query bar. - Adjust aria-labels for Dashboard Landing Page table. - Adjust aria-labels and interactive elements of Dashboard Panel actions buttons.
ca9967f to
5030f59
Compare
| ng-model="model.query" | ||
| placeholder="Filter..." | ||
| aria-label="Filter input" | ||
| aria-label="Enter query" |
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.
Can we confirm that this language makes more sense?
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.
Makes more sense to me. Do @Bargs or @lukasolson want to weigh in since this terminology is more in your area?
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.
Query probably makes more sense to end users since they're using the lucene query syntax. If we're changing the aria-label we should probably change the placeholder text as well right?
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.
Thanks @Bargs I'll update the placeholder text.
|
|
||
| <p class="kuiText kuiVerticalRhythm"> | ||
| Click the <a class="kuiButton kuiButton--primary kuiButton--small" ng-click="kbnTopNav.open('add'); toggleAddVisualization = !toggleAddVisualization" aria-label="Add visualization">Add</a> button in the menu bar above to add a visualization to the dashboard. <br/>If you haven't set up any visualizations yet, <a class="kuiLink" href="#/visualize">visit the Visualize app</a> to create your first visualization. | ||
| </p> |
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 real changes here, just used UI Framework classes.
|
|
||
| <p class="kuiText kuiVerticalRhythm"> | ||
| Click the <a class="kuiButton kuiButton--primary kuiButton--small" ng-click="kbnTopNav.click('edit');">Edit</a> button in the menu bar above to start working on your new dashboard. | ||
| </p> |
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 real changes here, just used UI Framework classes.
| <span | ||
| data-test-subj="dashboardPanelTitle" | ||
| class="panel-title" | ||
| aria-label="{{:: 'Visualization panel: ' + savedObj.title }}" |
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.
We should use aria-label instead of title. I added the "Visualization panel" prefix to give more context.
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.
This could be a saved search, and once we extend panels to plugins, it might be something else entirely - e.g. input controls or monitoring.
But I don't know, maybe we call all panel types "visualizations"? I think that might get confusing though.
I plan to refactor some of this out so that dashboard knows less about what it's rendering. At that point, the "panelRenderer" can supply a "type" attribute, and we can use that to describe the panel type here. In the mean time, I'll leave it up to you. I plan on having a type attribute anyway to dynamically render the add panel tabs.
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.
This is a really good point. I think I'll just change this to "Dashboard panel" for now, and we can incorporate the "type" attribute once you add it. Thanks!
stacey-gammon
left a comment
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
* Improve Dashboard screen-reader accessibility: - Add aria-label for Dashboard Add Panel tabs. - Add aria-label for Dashboard Panel title. - Refactor empty Dashboard prompt message to improve accessibility. - Adjust aria-labels for Dashboard query bar. - Adjust aria-labels for Dashboard Landing Page table. - Adjust aria-labels and interactive elements of Dashboard Panel actions buttons.
* Improve Dashboard screen-reader accessibility: - Add aria-label for Dashboard Add Panel tabs. - Add aria-label for Dashboard Panel title. - Refactor empty Dashboard prompt message to improve accessibility. - Adjust aria-labels for Dashboard query bar. - Adjust aria-labels for Dashboard Landing Page table. - Adjust aria-labels and interactive elements of Dashboard Panel actions buttons.
* Improve Dashboard screen-reader accessibility: - Add aria-label for Dashboard Add Panel tabs. - Add aria-label for Dashboard Panel title. - Refactor empty Dashboard prompt message to improve accessibility. - Adjust aria-labels for Dashboard query bar. - Adjust aria-labels for Dashboard Landing Page table. - Adjust aria-labels and interactive elements of Dashboard Panel actions buttons.
Partially addresses #11520. Extracts some work from #11548.