angular-ui / bootstrap

PLEASE READ THE PROJECT STATUS BELOW. Native AngularJS (Angular) directives for Bootstrap. Smaller footprint (20kB gzipped), no 3rd party JS dependencies (jQuery, bootstrap JS) required. Please read the README.md file before submitting an issue!
http://angular-ui.github.io/bootstrap/
MIT License
14.28k stars 6.73k forks source link

uibDatePicker focuses the grid of buttons after being popped up, which doesn't include the button bar at the bottom #6133

Open rsc092020 opened 8 years ago

rsc092020 commented 8 years ago

Bug description:

When opening uibDatepicker through uibDatepickerPopup, this code gets called:

var focusElement = function() {
    self.element[0].focus();
};

because of this event listener:

// Listen for focus requests from popup directive
$scope.$on('uib:datepicker.focus', focusElement);

This will focus the table that surrounds all of the buttons for each day, or month, or year, depending on which picker is showing (because it is the root element for each picker). It seems to me that the element that contains the table plus the button bar at the bottom should be focused instead so that way the whole thing gets scrolled into view, when it gets opened. It is noticeable when using the popup-placement="bottom" or popup-placement="bottom-left" or popup-placement="bottom-right" and the input is near the bottom of the page, since the button bar is below the table.

An easy workaround is just to not use the bottom placements, but I also feel like this behavior can be easily changed too, to work the same for all placements.

Link to minimally-working plunker that reproduces the issue:

http://plnkr.co/edit/ZiszhdYfbZt8UvC2MT5y?p=info

As you can see, when the datepicker gets opened at the bottom of the page, the button bar below it is still out of view.

Version of Angular, UIBS, and Bootstrap

Angular: 1.5.5

UIBS: 1.3.2 (this is the version I'm using and the version in the plunkr, but the behavior is the same in 2.0.0)

Bootstrap: 3.3.6

More info

I have some code that I could potentially use in a PR that would instead focus the outer ul element (which would be a parent of the picker and button bar), but I'm not yet sure whether this is a bug or is as designed.

wesleycho commented 8 years ago

This would be very tricky to fix - I would consider this an accessibility issue, but this could potentially result in a breaking change.

We need to investigate this - the earliest I can promise to do a detailed analysis is next week sometime, as I am currently on vacation in Europe.

rsc092020 commented 8 years ago

Ok, that will work.

I will add some suggestions/thoughts based on a potential solution that I came up with. It still has some issues (see bottom of post) and I haven't fully tested it, but it might be helpful.

I removed these lines from UibDatepickerController to stop it from focusing the element that it was previously focusing from uib:datepicker.focus:

var focusElement = function() {
    self.element[0].focus();
};

// Listen for focus requests from popup directive
$scope.$on('uib:datepicker.focus', focusElement);

Instead I wanted to listen for the event elsewhere to trigger this focus (somewhere farther up the DOM tree from UibDatepickerController). So I added this to uibDatepickerPopupWrap:

link: function(scope, element) {
    var child;
    scope.$on('uib:datepicker.focus', function() {
        if(child = element.children()[0]) {
            child.focus();
        }
    });
}

The child of uibDatepickerPopupWrap is actually the ulat the root of this template. Then, I also added a tabindex to this element to actually allow it to be focused.

This works for me, because I used the default popup template. But the main problem I see is that since the template for the popup is configurable by datepicker-popup-template-url, the child element from the uibDatepickerPopupWrap might not even be focusable. So in that case, then maybe it could focus the picker that is showing (a fallback to the current behavior).

This is a fork of the plunkr above with the changes I was talking about incorporated in.