[Accessibility] Improve visualization legends accessibility #14505

timroes · 2017-10-23T08:57:17Z

Release Note: Improve the keyboard accessibility of visualization legends.

Improve the accessibility of the regular visualization legends:

Make the legend toggle button accessible, Fix Accessibility: Visualization legend expand/collapse button needs to be made accessible #11843
Highlight chart data also when focusing a legend item (not only on hovering), Accessibility: Surface visualization legend options via click, not hover #11845
Make the legend filter buttons and the color picker accessible

This should make the legend accessible for now. We could definitely improve the accessibility especially in the color picker a lot, but I hope that we can replace the color picker at some point with the ui framework color picker, so I wouldn't spend now too much time into improving the existing color picker as it is basically working now.

timroes · 2017-10-23T09:42:37Z

Jenkins, test this

cjcenizal

It's so great to see our visualizations becoming more accessible! I had some suggestions and questions for your consideration.

cjcenizal · 2017-10-23T17:18:10Z

src/ui/public/visualize/visualize_legend.html

@@ -3,10 +3,13 @@
    type="button"
    ng-click="toggleLegend()"
    class="kuiCollapseButton legend-collapse-button"
+    aria-label="Toggle legend"


VoiceOver doesn't announce this label because of this aria-hidden logic introduced in #13573. I think that the warning is still useful, but perhaps we should just remove aria-hidden="true" from the legend?

Good point, I will remove it, but therefor I would like to add an aria-label introducing the legend as such. That would also partly solve the issue you mentioned below, regarding missing context for the user with just legend values.

cjcenizal · 2017-10-23T17:19:29Z

src/ui/public/visualize/visualize_legend.html

@@ -18,6 +21,10 @@

      <div class="legend-value-container">
        <div
+          kbn-accessible-click
+          data-label="{{legendData.label}}"


What does this attribute do?

Currently, the label doesn't indicate to the screen reader what will happen if the user clicks the button. Is there an elegant way to add an invisible label that says something like "Click for filtering options"? Maybe we can associate it with aria-describedby? Do you think this will be helpful?

Also, if the label is a value like "181", then the screen reader will just read this number aloud, which doesn't provide enough context for me to make sense of it. What if we added a noun to the end of this value? For example, if the aggregation was on "bytes", we could read aloud:

{{ `${legendData.label} + ${legendData.values.aggConfig.params.field.name}` }}

Which would cause the screen reader to announce "181 bytes". What do you think?

The attribute itself is required by the handlers in the visualization, that are doing the highlight and unhighlight to find the actual value. Not ideal imho, but that's the way it works atm.

I will add a description for what the label triggering does, that's a very good idea.

I think extending the label with the fieldname, won't always work as smooth as it did in your example :-) So I would rather add this to the description like: "Filtering bytes fields for this value"

I added it to the label instead, so you really can't miss it. I didn't add aria-expanded since that would require we can reference the legend options via aria-controls. But they use ng-if and not ng-show at the moment, so they are really removed from DOM and we cannot reference them when hidden. I also don't want to use ng-show in that place, since imho the options contain a significant amount of DOM nodes and having a dashboard with multiple visualizations on it, which several legend items, would render a significantly higher amount of DOM nodes with ng-show.

So I decided to not use aria-expanded in that case.

cjcenizal · 2017-10-23T17:30:51Z

src/ui/public/visualize/visualize_legend.html

+          kbn-accessible-click
+          data-label="{{legendData.label}}"
+          ng-focus="highlight($event)"
+          ng-blur="unhighlight($event)"
          ng-click="showDetails = !showDetails"


Once the user opens these options, it's easy to get lost in the colorpicker. I know we don't want to spend time fixing this in this PR, but I think it would be helpful to let the user close the options by hitting ESC. This way if s/he gets lost in the color picker we've provided an easy way out. We just need to extract lines 22 through 75 into their own directive, and add a keypress handler there which sets showDetails to false. The challenge might be setting focus back on the button once the filtering options have been closed. What do you think?

I think the color picker is not really accessible the way it is even after that PR. But I don't think just adding Escape would be enough. We still would need to think about color names, proper arrow navigation, etc... So I would rather replace this in the more or less near future, by using a standardized color picker from the UI framework instead and put the a11y effort into this. It would also come with the benefit, of being able to freely assign a color.

Yeah, I hear you... I was just thinking of it as a temporary solution, since it's so easy to get stuck inside the color picker options and not know that the only way to get out of them is to backtrack all the way back to the button and then click it again.

cjcenizal · 2017-10-23T17:49:28Z

src/ui/public/visualize/visualize_legend.html

            >
              <span class="kuiIcon fa-search-minus"></span>
            </button>
          </div>

-          <div class="legend-value-color-picker">
+          <div class="legend-value-color-picker" role="listbox">
+            <span id="{{legendId}}ColorPickerDesc" class="kuiScreenReaderOnly">Set color for value {{legendData.label}}</span>


Can we indent this?

<span id="{{legendId}}ColorPickerDesc" class="kuiScreenReaderOnly" > Set color for value {{legendData.label}} </span>

What if I would say "no"? Yes of course we can :-)

Then you better have a good reason! :D

cjcenizal · 2017-10-23T17:50:32Z

src/ui/public/visualize/visualize_legend.js

        const colors = $scope.uiState.get('vis.colors') || {};
        if (colors[label] === color) delete colors[label];
        else colors[label] = color;
        $scope.uiState.setSilent('vis.colors', null);
        $scope.uiState.set('vis.colors', colors);
        $scope.uiState.emit('colorChanged');
+        if (event) {
+          event.currentTarget.blur();


Out of curiosity, why are we doing this?

Good question. I was tinged with the slight idea, of not having the color button keeping the focus ring and it's then selected state. But honestly, we don't do this anywhere else, and again I want to replace this by the unified color picker in the future, so yeah I will throw it out for this PR.

ppisljar

LGTM

thomasneirynck

I like it.

This works a lot more intuitively than I thought it would. Nice too that you can jump to filter bar and continue editing filters there.

cjcenizal

Fantastic! Thanks for adding the ESCAPE handler -- I really think it's a great escape hatch (sorry).

I had one small nitpicky comment but it's probably unnecessary so feel free to disregard it.

You mentioned this:

So I would rather add this to the description like: "Filtering bytes fields for this value"

But I didn't see this change -- not sure if this was intentional? If so, that's fine, it's a minor thing anyway, but just wanted to check.

cjcenizal · 2017-10-25T16:22:43Z

src/ui/public/visualize/visualize_legend.js

+       * This will close the details panel of this legend entry when pressing Escape.
+       */
+      $scope.onLegendEntryKeydown = function (event, scope) {
+        if (event.keyCode === keyCodes.ESCAPE) {


I might be overthinking this but maybe we should check if showDetails is true first? Maybe someday in some bizarre future, something further up the DOM tree will care about the ESCAPE key being pressed and will be sad that it's being swallowed up here.

$scope.onLegendEntryKeydown = function (event, scope) { if (event.keyCode === keyCodes.ESCAPE) { if (scope.showDetails) { event.preventDefault(); event.stopPropagation(); scope.showDetails = false; } } };

timroes · 2017-10-26T09:01:52Z

@cjcenizal actually it was intentional. Sorry for not commenting on that :/

That problem is, that the field is nested deep within the aggregation information and also there doesn't need to be a field (e.g. if you are doing a filters aggregation). Since we currently have no service (or I didn't found any) to extract the field information from the visualization legend data, but would have to implement that logic all over again, I decided it's not worth it at the moment.

…14505) * Make legend toggle button accessible, fix elastic#11843 * Make legend filter buttons accessible * Highlight chart segments when focusing legend, fix elastic#11845 * Make legend color picker accessible * Remove aria-hidden from legend * Use proper indentation * Remove bluring color button again * Close legend details on pressing escape * Add hint about the action * Only capture escape press when details are opened

…14606) * Make legend toggle button accessible, fix #11843 * Make legend filter buttons accessible * Highlight chart segments when focusing legend, fix #11845 * Make legend color picker accessible * Remove aria-hidden from legend * Use proper indentation * Remove bluring color button again * Close legend details on pressing escape * Add hint about the action * Only capture escape press when details are opened

Tim Roes added 4 commits October 23, 2017 10:21

Make legend toggle button accessible, fix elastic#11843

2327f9c

Make legend filter buttons accessible

aada893

Highlight chart segments when focusing legend, fix elastic#11845

ca55a98

Make legend color picker accessible

170932a

timroes added Feature:Visualizations Generic visualization features (in case no more specific feature label is available) Project:Accessibility bug Fixes for quality problems that affect the customer experience review v6.1.0 v7.0.0 labels Oct 23, 2017

timroes requested review from cjcenizal, thomasneirynck and ppisljar October 23, 2017 08:57

cjcenizal suggested changes Oct 23, 2017

View reviewed changes

Tim Roes added 5 commits October 25, 2017 08:41

Remove aria-hidden from legend

ed7a1e2

Use proper indentation

dd65700

Remove bluring color button again

1645518

Close legend details on pressing escape

fa2a1af

Add hint about the action

2265c3b

ppisljar approved these changes Oct 25, 2017

View reviewed changes

thomasneirynck approved these changes Oct 25, 2017

View reviewed changes

cjcenizal approved these changes Oct 25, 2017

View reviewed changes

Tim Roes added 3 commits October 26, 2017 11:35

Only capture escape press when details are opened

24ad558

Merge branch 'master' into vis-a11y

f8a5dba

Merge remote-tracking branch 'upstream/master' into vis-a11y

809217d

timroes merged commit 00bbee3 into elastic:master Oct 26, 2017

timroes deleted the vis-a11y branch October 26, 2017 12:15

timroes mentioned this pull request Oct 26, 2017

[6.x] [Accessibility] Improve visualization legends accessibility (#14505) #14606

Merged

timroes removed the review label Nov 17, 2017

thomasneirynck added the release_note:enhancement label Dec 7, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Accessibility] Improve visualization legends accessibility #14505

[Accessibility] Improve visualization legends accessibility #14505

timroes commented Oct 23, 2017 •

edited by thomasneirynck

Loading

timroes commented Oct 23, 2017

cjcenizal left a comment

cjcenizal Oct 23, 2017

timroes Oct 23, 2017

timroes Oct 25, 2017

cjcenizal Oct 23, 2017

cjcenizal Oct 23, 2017

timroes Oct 23, 2017

timroes Oct 25, 2017

cjcenizal Oct 23, 2017

timroes Oct 23, 2017

cjcenizal Oct 23, 2017

timroes Oct 25, 2017

cjcenizal Oct 23, 2017

timroes Oct 23, 2017

cjcenizal Oct 23, 2017

cjcenizal Oct 23, 2017

timroes Oct 23, 2017

ppisljar left a comment

thomasneirynck left a comment

cjcenizal left a comment

cjcenizal Oct 25, 2017

timroes commented Oct 26, 2017

[Accessibility] Improve visualization legends accessibility #14505

[Accessibility] Improve visualization legends accessibility #14505

Conversation

timroes commented Oct 23, 2017 • edited by thomasneirynck Loading

timroes commented Oct 23, 2017

cjcenizal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ppisljar left a comment

Choose a reason for hiding this comment

thomasneirynck left a comment

Choose a reason for hiding this comment

cjcenizal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timroes commented Oct 26, 2017

timroes commented Oct 23, 2017 •

edited by thomasneirynck

Loading