Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DatePicker: Add option to not auto-open on focus #1451

Merged
merged 4 commits into from Apr 18, 2017

Conversation

Projects
None yet
5 participants
@c-w
Copy link
Contributor

commented Apr 10, 2017

Pull request checklist

  • Addresses an existing issue: #1266
  • Include a change request file if publishing
  • New feature, bugfix, or enhancement
    • Includes tests
  • Documentation update

Description of changes

When focus lands on the date picker, it currently immediately opens the date picker. This is confusing from an accessibility point of view. This pull request adds an option to disable the auto-open behavior so that the user can open the date picker manually, e.g. by hitting enter.

Focus areas to test

Test steps (verified manually on DatePicker.Input.Example: screencast):

  1. Create a date-picker with the disableAutoFocus property set to true.
  2. Tab through the page and onto the picker, it should not automatically open.
  3. Hit enter, the picker should open.
@mikewheaton

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2017

Does it ever make sense to open it immediately on keyboard focus? Maybe instead of this being an option we should just remove the functionality entirely.

@c-w c-w changed the title Bugfix/date picker focus DatePicker: Improve focus handling for accessibility Apr 10, 2017

@c-w c-w changed the title DatePicker: Improve focus handling for accessibility DatePicker: Improve focus accessibility Apr 10, 2017

@c-w

This comment has been minimized.

Copy link
Contributor Author

commented Apr 10, 2017

@mikewheaton If you're happy to make that API change, we can go for it. @scriby: do you see a scenario where auto-opening the date-picker is the correct behavior?

@scriby

This comment has been minimized.

Copy link
Contributor

commented Apr 10, 2017

From the standpoint of supporting screenreaders, I think it's much more understandable to require the user to take an explicit action to open the popup than to just launch them into it without context. So, I think it's a reasonable stance to take that we don't allow the date picker to open automatically on focus.

Whether or not a third party might want that behavior regardless some day is another story...

@mdahamiwal

This comment has been minimized.

Copy link
Collaborator

commented Apr 11, 2017

@c-w , @mikewheaton I think we should go through the accessible date pickers listed at http://www.webaxe.org/accessible-date-pickers/ and decide on the design. There are both types 1. ones that open the picker popup on focus and 2. others that provide a focusable picker button next to date text box.

@c-w c-w referenced this pull request Apr 11, 2017

Merged

DatePicker: Restore focus after cycling out of the callout #1477

2 of 5 tasks complete
@c-w

This comment has been minimized.

Copy link
Contributor Author

commented Apr 11, 2017

While we have the conversation about the focus-open behavior, I split out #1477 for the tab-order changes as the fix is valuable on its own.

c-w added some commits Apr 11, 2017

Add option to not auto-open the picker on focus
This is a fix for issue 1 mentioned in #1266

@c-w c-w changed the title DatePicker: Improve focus accessibility DatePicker: Add option to not auto-open on focus Apr 11, 2017

@c-w

This comment has been minimized.

Copy link
Contributor Author

commented Apr 13, 2017

@mdahamiwal @mikewheaton: Did you take a decision on whether you want the always/never auto-open behavior or whether you'd prefer to make it configurable?

@mikewheaton

This comment has been minimized.

Copy link
Contributor

commented Apr 17, 2017

Let's just make it an option for now, as both look like valid options :)

@c-w

This comment has been minimized.

Copy link
Contributor Author

commented Apr 17, 2017

@mikewheaton: Thanks for the decision. This pull request adds the new option to disable auto-open on focus of the date picker. Could you please give it a review?

@mikewheaton
Copy link
Contributor

left a comment

Looks good, thanks!

@c-w

This comment has been minimized.

Copy link
Contributor Author

commented Apr 17, 2017

@mikewheaton: Thanks for the sign-off. Could you please merge this in before master advances again? I don't have merge-access.

@mdahamiwal mdahamiwal merged commit b4c3c69 into OfficeDev:master Apr 18, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.