Allow reordering aggregation priority by keyboard #13635
Conversation
timroes
added
Project:Accessibility
Feature:Vislib
timroes
requested review from
aphelionz,
cjcenizal,
thomasneirynck and
ppisljar
|
works well, but would it be possible to also activate this on mouse click ? (set focus to the button?) |
|
@ppisljar It's not as easy as I thought. The drag library, that we use dragula is currently preventing focusing the buttons on click (https://github.com/bevacqua/dragula/blob/master/dragula.js#L113). We would need to work around this, by writing some directive, that just focuses an element on mousedown. That way we could steal the focus back. Due to the order in which events get resolved, this would also mean, that we need to add an |
| down: 'down' | ||
| }; | ||
|
|
||
| const directionMapping = { |
There was a problem hiding this comment.
Consider importing these keycodes from the UI Framework instead?
There was a problem hiding this comment.
Makes totally sense. I just found, that we currently have mixed ways of exporting keycodes in the ui_framework services. That's why I created another PR (#13663). If you feel like that refactoring is a good thing, you could review that PR please, and I would rebase this PR as soon as I merged the other one to use the exported keycodes.
| }; | ||
|
|
||
| uiModules.get('kibana') | ||
| .directive('keyboardMove', ($timeout) => ({ |
There was a problem hiding this comment.
How about renaming this to onKeyboardMove to reflect its role as an event handler?
There was a problem hiding this comment.
Since this directive really adds some logic to an element, and doesn't just listen on an event, that would anyway happen, I would rather not rename it onKeyboardMove. A directive named on- sounds for me side effect free, which this isn't. Besides Angular itself not naming any of the listeners on-, but just ng-keydown, etc.
Would you be up for discussion on this topic?
| .directive('keyboardMove', ($timeout) => ({ | ||
| restrict: 'A', | ||
| scope: { | ||
| keyboardMove: '&' |
There was a problem hiding this comment.
I'm worried about this isolate scope. If we apply this to an element which has its own isolate scope, they'll conflict. I think it would be better to use $parse to extract a reference to the callback, similar to the way ngClick does it: https://github.com/angular/angular.js/blob/master/src/ng/directive/ngEventDirs.js#L60
|
jenkins, test this |
timroes
force-pushed
the
keyboard-reordering
branch
from
ab5bed8 to
99dac56
Compare
* Make aggs sortable by keyboard, fix #11858 * Add keyboard_move tests * Use $parse instead of creating a child scope * Use keyCodes from ui_framework
|
Backports:
|
To make the reordering of priorities of aggregations keyboard accessible, this PR introduces the
keyboardMovedirective. While the drag button has keyboard focus, you can press the up and down key to move the aggregation up and down in the priority list.To explain the
$timeout(() => element.focus()): When reordering items in anng-repeatit will need to remove the element from DOM and reattach it at another position. When the aggregation moved down in the priority list, it wasn't detached from DOM and thus the focus was kept on the button, when moving it upwards it was reattached and thus the focus was lost after one move. To be able to move the aggregation multiple positions up without needing to tab to the button again, I focus the button again in the next execution cycle, when it is reattached to the DOM.Fixes #11858