fix(datepicker): restore focus to trigger element #4804

crisbeto · 2017-05-25T16:39:37Z

The datepicker now restores focus to whatever element was focused before it was open.
Adds a test for the functionality that closes the datepicker when pressing escape.

mmalerba · 2017-05-25T17:12:13Z

I think for datepicker we don't want to close on tab (see various accessible datepicker implementations: http://www.webaxe.org/accessible-date-pickers/)

crisbeto · 2017-05-25T17:20:04Z

In that case we'd need to trap focus inside the popup, otherwise it goes out of the page.

mmalerba · 2017-05-25T17:50:44Z

I believe it does: https://github.com/angular/material2/blob/master/src/lib/datepicker/datepicker-content.html#L1

The behavior might be a little weird though, because the dialog (which we use for the touch UI) does its own focus trapping. What I'd like to eventually do is create a MdPopup, similar to MdDialog. Things like select and auto-complete could take advantage of this as well. One of the APIs on MdPopup would allow you to set it to floating mode where its acts more like a dialog (not attached to the input) that way datepicker could just use the popup instead of the overlay/dialog hybrid approach

crisbeto · 2017-05-25T18:23:38Z

It doesn't look like the focus trapping works when it's not in touch mode. Should I remove the tabbing behavior from this PR and try address the focus trapping separately?

mmalerba · 2017-05-25T18:27:49Z

yeah sgtm

crisbeto · 2017-05-25T18:57:24Z

Reverted the tabbing behavior @mmalerba.

mmalerba · 2017-05-25T19:57:38Z

src/lib/datepicker/datepicker.ts

@@ -30,7 +32,7 @@ import {Subscription} from 'rxjs/Subscription';
 import {MdDialogConfig} from '../dialog/dialog-config';
 import {DateAdapter} from '../core/datetime/index';
 import {createMissingDateImplError} from './datepicker-errors';
-import {ESCAPE} from '../core/keyboard/keycodes';
+import {ESCAPE, TAB} from '../core/keyboard/keycodes';


TAB no longer needed

mmalerba · 2017-05-25T19:58:40Z

src/lib/datepicker/datepicker.ts

@@ -226,6 +229,11 @@ export class MdDatepicker<D> implements OnDestroy {
    if (this._calendarPortal && this._calendarPortal.isAttached) {
      this._calendarPortal.detach();
    }
+    if (this._focusedElementBeforeOpen && 'focus' in this._focusedElementBeforeOpen) {


why is the 'focus' in part needed?

It's there for IE's sake. In some cases it assigns the activeElement to something that doesn't have a focus method. It's what was throwing that 4mb error in the CI.

interesting, good to know

Fixes focus trapping not working inside the datepicker in popup mode due to the A11yModule not being imported. Relates to angular#4804.

Fixes focus trapping not working inside the datepicker in popup mode due to the A11yModule not being imported. Relates to #4804.

BaimosTechnologies · 2017-06-16T11:38:16Z

Also got this focus problem, pretty important for us. Can someone please rebase and merge it?

* The datepicker now restores focus to whatever element was focused before it was open. * Adds a test for the functionality that closes the datepicker when pressing escape.

BaimosTechnologies · 2017-06-19T12:08:15Z

Works very good, thank you for merging this.

angular-automatic-lock-bot · 2019-09-06T14:01:44Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

crisbeto added the pr: needs review label May 25, 2017

googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label May 25, 2017

crisbeto changed the title ~~fix(datepicker): restore focus to trigger element and close when pres…~~ fix(datepicker): restore focus to trigger element and close when pressing tab May 25, 2017

crisbeto added the Accessibility This issue is related to accessibility (a11y) label May 25, 2017

crisbeto assigned mmalerba May 25, 2017

crisbeto force-pushed the datepicker-tab branch 2 times, most recently from de49d3e to 42cb003 Compare May 25, 2017 18:44

crisbeto changed the title ~~fix(datepicker): restore focus to trigger element and close when pressing tab~~ fix(datepicker): restore focus to trigger element May 25, 2017

mmalerba reviewed May 25, 2017

View reviewed changes

mmalerba added pr: lgtm action: merge The PR is ready for merge by the caretaker and removed pr: needs review labels May 25, 2017

crisbeto added a commit to crisbeto/material2 that referenced this pull request May 27, 2017

fix(datepicker): focus trap not working inside popup

1b91886

Fixes focus trapping not working inside the datepicker in popup mode due to the A11yModule not being imported. Relates to angular#4804.

crisbeto mentioned this pull request May 27, 2017

fix(datepicker): focus trap not working inside popup #4839

Merged

crisbeto force-pushed the datepicker-tab branch from 0b26836 to 08ea096 Compare May 27, 2017 08:15

andrewseguin pushed a commit that referenced this pull request Jun 5, 2017

fix(datepicker): focus trap not working inside popup (#4839)

157bda1

Fixes focus trapping not working inside the datepicker in popup mode due to the A11yModule not being imported. Relates to #4804.

andrewseguin added pr: needs rebase and removed action: merge The PR is ready for merge by the caretaker labels Jun 9, 2017

crisbeto added 2 commits June 16, 2017 18:37

fix(datepicker): restore focus to trigger element

857d870

* The datepicker now restores focus to whatever element was focused before it was open. * Adds a test for the functionality that closes the datepicker when pressing escape.

chore: unused import

2cd63d7

crisbeto force-pushed the datepicker-tab branch from 08ea096 to 2cd63d7 Compare June 16, 2017 16:37

crisbeto added action: merge The PR is ready for merge by the caretaker and removed pr: needs rebase labels Jun 16, 2017

kara merged commit 8860090 into angular:master Jun 16, 2017

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(datepicker): restore focus to trigger element #4804

fix(datepicker): restore focus to trigger element #4804

crisbeto commented May 25, 2017 •

edited

Loading

mmalerba commented May 25, 2017

crisbeto commented May 25, 2017

mmalerba commented May 25, 2017

crisbeto commented May 25, 2017

mmalerba commented May 25, 2017

crisbeto commented May 25, 2017

mmalerba May 25, 2017

mmalerba May 25, 2017

crisbeto May 25, 2017

mmalerba May 25, 2017

BaimosTechnologies commented Jun 16, 2017

BaimosTechnologies commented Jun 19, 2017

angular-automatic-lock-bot bot commented Sep 6, 2019

fix(datepicker): restore focus to trigger element #4804

fix(datepicker): restore focus to trigger element #4804

Conversation

crisbeto commented May 25, 2017 • edited Loading

mmalerba commented May 25, 2017

crisbeto commented May 25, 2017

mmalerba commented May 25, 2017

crisbeto commented May 25, 2017

mmalerba commented May 25, 2017

crisbeto commented May 25, 2017

mmalerba May 25, 2017

Choose a reason for hiding this comment

mmalerba May 25, 2017

Choose a reason for hiding this comment

crisbeto May 25, 2017

Choose a reason for hiding this comment

mmalerba May 25, 2017

Choose a reason for hiding this comment

BaimosTechnologies commented Jun 16, 2017

BaimosTechnologies commented Jun 19, 2017

angular-automatic-lock-bot bot commented Sep 6, 2019

crisbeto commented May 25, 2017 •

edited

Loading