[FIX] web: focus on close button when before closing a modal #32038
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mgeubelle
reviewed
Mar 22, 2019
| var isModal = false; | ||
| return waitForCondition(function() { | ||
| // sometimes, the app is just a modal that needs to be closed | ||
| var $modal = $('.modal[role="dialog"][open="open"]'); | ||
| if ($modal.length > 0) { | ||
| $modal.modal('hide'); | ||
| var $close_button = $('header > button.close'); |
There was a problem hiding this comment.
I would do something like $modal.find('header > button.close') to be sure that your close button is in the modal.
There was a problem hiding this comment.
variables are camelcased in JS
| var isModal = false; | ||
| return waitForCondition(function() { | ||
| // sometimes, the app is just a modal that needs to be closed | ||
| var $modal = $('.modal[role="dialog"][open="open"]'); | ||
| if ($modal.length > 0) { | ||
| $modal.modal('hide'); | ||
| var $close_button = $('header > button.close'); | ||
| $close_button.focus(); |
There was a problem hiding this comment.
Is it still needed, despite your _click function ?
There was a problem hiding this comment.
If it is, maybe the focus event should be triggered in the _click function.
There was a problem hiding this comment.
Yep, I tested without and it crashes as expected
| trigger_mouse_event($element, "mouseup"); | ||
| trigger_mouse_event($element, "click"); | ||
|
|
||
| function trigger_mouse_event($el, type, count) { |
There was a problem hiding this comment.
count is not needed in your case
During the clickEverywhere test, when a modal is opened, the hide method is called to bypass it. This is not realistic and does not work in some cases. For example in Point of sale, menu "Reporting/Sales Details", a modal is opened with a DateTimePicker (tempus dominus) already opened. When hiding or closing the modal, the DateTimePicker needs to perform some cleanup. If the modal is simply hidden, the DateTimePicker is not cleanly destroyed. With this commit, a modal is closed by clicking on its close button and before that, the focus is set on the close button to let the datepicker widget do its stuff on losing focus. Also, a more realistic click is done by trigerring more events (code adapted from tour helper).
d-fence
force-pushed
the
master-fix-click-all-datetime-moc
branch
from
0190b5f
to
ba30bb9
Compare
robodoo
added
CI 🤖
|
@robodoo r+ |
robodoo
pushed a commit
that referenced
this pull request
Mar 22, 2019
During the clickEverywhere test, when a modal is opened, the hide method is called to bypass it. This is not realistic and does not work in some cases. For example in Point of sale, menu "Reporting/Sales Details", a modal is opened with a DateTimePicker (tempus dominus) already opened. When hiding or closing the modal, the DateTimePicker needs to perform some cleanup. If the modal is simply hidden, the DateTimePicker is not cleanly destroyed. With this commit, a modal is closed by clicking on its close button and before that, the focus is set on the close button to let the datepicker widget do its stuff on losing focus. Also, a more realistic click is done by trigerring more events (code adapted from tour helper). closes #32038 Signed-off-by: Christophe Monniez (moc) <moc@odoo.com>
|
Merged, thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
During the clickEverywhere test, when a modal is opened, the hide method
is called to bypass it. This is not realistic and does not work in some
cases.
For example in Point of sale, menu "Reporting/Sales Details", a modal is
opened with a DateTimePicker (tempus dominus) already opened.
When hiding or closing the modal, the DateTimePicker needs to perform
some cleanup. If the modal is simply hidden, the DateTimePicker is not
cleanly destroyed.
With this commit, a modal is closed by clicking on its close button and
before that, the focus is set on the close button to let the
datepicker widget do its stuff on losing focus.
Also, a more realistic click is done by triggering more events (code
adapted from tour helper).