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
fix(datepicker): restore focus after closing popup #3371
fix(datepicker): restore focus after closing popup #3371
Conversation
At the moment the focus is lost, so it will be restored to the previously focused element (before datepicker was opened) Fixes ng-bootstrap#3317 Fixes ng-bootstrap#3358
96cb653
to
83bf2e5
Compare
Codecov Report
@@ Coverage Diff @@
## master #3371 +/- ##
==========================================
- Coverage 90.88% 90.86% -0.02%
==========================================
Files 95 95
Lines 2743 2748 +5
Branches 510 511 +1
==========================================
+ Hits 2493 2497 +4
Misses 190 190
- Partials 60 61 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks OK and this is welcome change!
There is one specific path I believe, this change could brake things (the use case with overlays with overlays, which are always tricky):
- open modal with datepicker
- open datepicker
- ESC key (currently keydown) to close
You should be able to see that modal behind datepicker dialog closes, not the datepicker alone.
This is probably due to the following:
- backing modal listens on keyup.ESC on the modal element, not on the document element
- dialog opens and delegates keydown.ESC to the document
- on the ESC key event modal handler catches first, closes the modal instance
- only after the modal handler finishes the autoclose fires the close on the datepicker, but its already too late.
Here is a fix for this, but it requires more changes:
https://gist.github.com/peterblazejewicz/2fc4a682e4ed1a56093ac04f7a67b480
- modal and dialog should listen on the same key, so if any of them catches event and do action, it could prevent event bubbling up (like in the keymap processors)
- the dialog picker should listen for the ESC event on its own, otherwise it delegates to the autoclose on the document
- the dialog stack should have a chance to prevent default and stop propagation, so the modal window handler has not.
Please see the gifs below with before/after this diff:
@peterblazejewicz thanks for the review, you're right. Moreover it's the problem not only with the datepicker, but with all our components that are displayed inside the modal, ex. dropdown, typeahead. So I'd like to find a more generic solution for this problem |
Will merge this one as is and open another one to fix closing modal |
At the moment the focus is lost, so it will be restored to the
previously focused element (before datepicker was opened)
Fixes #3317
Fixes #3358