2.0.0 breaks keyboard-nav on uib-dropdown

[mattlewis92]

Bug description:

keyboard-nav on the uib-dropdown menu is broken in 2.0.0. It looks like it was this commit that broke it: 6038403

If you add the event listener on the $document instead of the dropdown menu element then it works, I'd send a PR but I'm assuming there is a reason why the event listener is bound to the dropdown menu instead of the document.

Link to minimally-working plunker that reproduces the issue:

http://plnkr.co/edit/AmmUCB7iidPBKVw6r1Ne?p=preview

(Hit the up and down arrows after opening the dropdown - this was just copied from the demo)

Version of Angular, UIBS, and Bootstrap

Angular:
1.5.x
UIBS:
2.0.0
Bootstrap:
3.3.6

[icfantv]

@wesleycho, thoughts?

[wesleycho]

So as mentioned in the commit that changed this, the reason for the change is #5965 - in particular, this change is mandatory when appending/destroying the dropdown menu on toggle for the append-to option.

We need to look at this carefully.

[mattlewis92]

Even with dropdown-append-to-body added to the plunker, keyboard navigation doesn't seem to work, could binding the event listener to the $document instead handle all instances?

[wesleycho]

document is not the right solution - this would break some rules of accessibility, since the user could be focused elsewhere and it would still force navigation on the dropdown. Also if two dropdowns are open, it would navigate on both.

[mattlewis92]

OK what about binding the keydown to the document and then checking if the trigger element or the dropdown menu contains the $event.target element of the keydown event? Something like:

this.keybindFilter = function(evt) {
    var dropdownElementTargeted = dropdownElement && dropdownElement[0].contains(evt.target);
    var toggleElementTargeted = toggleElement && toggleElement[0].contains(evt.target);
    ... omitted
    if (...omitted (dropdownElementTargeted || toggleElementTargeted)) {
      evt.preventDefault();
      evt.stopPropagation();
      openScope.focusDropdownEntry(evt.which);
    }
  };

[mattlewis92]

@wesleycho any more thoughts on my proposed solution to this? Happy to work on a PR for it unless you can think of any potential issues with it?

[wesleycho]

I am not able to evaluate any issues deeply for at least another week - I am currently on vacation in Europe (have been for the past two weeks).

[mattlewis92]

Ah no worries, enjoy your holiday! 😄

[wesleycho]

Reading your proposed solution, I think this might be how we have to handle this - would you be interested in filing a PR? Adding a test for this situation also would help guard us from breaking this accessibility related issue in the future.

[mattlewis92]

Sure thing, I'll try and get to it over the next day or so