Make 'time picker' button in Discover no results prompt keyboard and screen-reader accessible. #13046
Conversation
| ng-click="kbnTopNav.toggle('filter')" | ||
| kbn-accessible-click | ||
| aria-expanded="{{kbnTopNav.isCurrent('filter')}}" | ||
| aria-label="time picker" |
There was a problem hiding this comment.
It's exactly the same label as the content currently is, so we can just remove this aria-label.
| <h3>Expand your time range</h3> | ||
| <p>I see you are looking at an index with a date field. It is possible your query does not match anything in the current time range, or that there is no data at all in the currently selected time range. Click the button below to open the time picker. For future reference you can open the time picker by clicking on the <a class="kuiButton kuiButton--primary kuiButton--small" ng-click="kbnTopNav.toggle('filter')" aria-expanded="kbnTopNav.is('filter')" aria-label="time picker" data-test-subj="discoverNoResultsTimefilter"><span aria-hidden="true" class="kuiIcon fa-clock-o"></span> time picker</a> button in the top right corner of your screen. | ||
| I see you are looking at an index with a date field. It is possible your query does not match anything in the current time range, or that there is no data at all in the currently selected time range. Click the button below to open the time picker. For future reference you can open the time picker by clicking on the | ||
| <a |
There was a problem hiding this comment.
You didn't introduce this here, but I feel like nitpicking about it now (though no need to fix it) ;P
This should actually be a button and not an a imho, since it "does" something on the current page and doesn't navigate to another page.
| aria-expanded="{{kbnTopNav.isCurrent('filter')}}" | ||
| aria-label="time picker" | ||
| data-test-subj="discoverNoResultsTimefilter" | ||
| > |
There was a problem hiding this comment.
Since this link doesn't have an href attribute it wouldn't get the link role by default so we should add this role manually.
| <a | ||
| class="kuiButton kuiButton--primary kuiButton--small" | ||
| ng-click="kbnTopNav.toggle('filter')" | ||
| kbn-accessible-click |
There was a problem hiding this comment.
I guess you already discussed this, but can't find it right now: wouldn't it make sense to add ngAria as a dependency, to solve that basic issues (like automatically make ng-click stuff tabable and clickable by keyboard)?
There was a problem hiding this comment.
Would you mind creating a new issue for this, with the "discuss" and "Accessibility" labels? This is an interesting idea and it seems like it would solve a good set of low-level issues! One tradeoff is that it's magic and relives the engineer of the responsibility of understanding and using ARIA well, which could be problematic when migrating Angular code to React.
|
@timroes Changing the element type to |
|
@cjcenizal all good. I don't see that severe warning anymore. Cheers! |
cjcenizal
force-pushed
the
12970/local-nav-aria
branch
from
8a6df75
to
d12e70a
Compare
cjcenizal
force-pushed
the
12970/local-nav-aria
branch
from
d12e70a
to
77cb9de
Compare
…screen-reader accessible. (elastic#13046) * Make 'time picker' button in Discover no results prompt keyboard and screen-reader accessible.
Fixes #12970