-
-
Notifications
You must be signed in to change notification settings - Fork 78.9k
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
Modal event.preventDefault() for show.bs.modal: disables modals with fade class from being displayed again in V4 & V5. #34055
Comments
Holy moly.. The time and efforts you put into that bug report, you could have simply opened a PR with the fix. 😅 If only every bug report looked something like this.. Amazing job you did there, really appreciate it! ❤️ |
In case of a modal with fading enabled, a prevented show event can cause show to not showing the modal anymore. See #34055
In case of a modal with fading enabled, a prevented show event can cause show to not showing the modal anymore. See #34055
alpadev — thanks for creating the Pull Request and getting it into the system for processing. I’ve never done a PR, so it was easier for me to explain (in detail so there would be no questions) what the problem was, where it came from, and a general solution. The fix for V5 is in the 5.0.2 release and it does work — I cloned one of my test cases and updated the Bootstrap links to the 5.0.2 release. The next step is the fix for V4. Your Pull Request is in the Inbox for v4.6.1 and is waiting for XhmikosR or js-review to review the request, although it has been in the Inbox for a month now. Hopefully it will be reviewed soon. |
In case of a modal with fading enabled, a prevented show event can cause show to not showing the modal anymore. See #34055 Co-authored-by: XhmikosR <[email protected]>
When an event listener attached to a Bootstrap modal (with the fade class) for
show.bs.modal
returnsevent.preventDefault(),
the modal is not displayed (the correct behavior) the first time, but it will no longer be possible to trigger the modal. The button associated with the modal cannot cause the modal to open again.This is because the modal.js module sets the value for whether the modal is transitioning or not before it triggers the
show.bs.modal
event. Once displaying a modal is blocked withpreventDefault()
, there's no way to reset thethis._isTransitioning
value.Expected behavior
The expected behavior is that each time the
show.bs.modal
event is triggered, the event listener can prevent the modal from being shown, but prevention will not impact future requests.This is how the
show.bs.modal
event works on V3.4.1. This is also how other show events work (at least for V5 for dropdown, collapse, popover, and offcanvas, which were tested).Starting with V4.0 and continuing through to the current V5.0.1, after a
show.bs.modal
listener returnsevent.preventDefault(),
Bootstrap no longer issues an event to the listener.Why is this happening
The Bootstrap 4 and the Bootstrap 5 documentation recommends
return event.preventDefault()
as the way to stop a modal from being shown:Bootstrap 4
Bootstrap 5
The code in the V4 & V5 modal.js files that processes
preventDefault()
is in the show function:Bootstrap 4.6.0
Bootstrap 5.0.1
For both V4 and V5, the first thing the show function does is to check whether the modal is already being shown (
this._isShown
) or if the modal is in the process of being shown (this._isTransistioning
).Both V4 and V5 then immediately set the
this._isTransitioning
value if the modal has thefade
class. The value is set before triggering theshow.bs.modal
event.If the event handler returns defaultPrevented, the show function is exited, leaving the
this._isTransistioning
set totrue
.The next time the show function is called, the
this._isTransistioning
value is checked, and since it's true, the show function exits, preventing the modal from being displayed.The solution
Move setting
this._isTransistioning
to after evaluating the defaultPrevented value, below thethis._isShown = true
line, will preventthis._isTransistioning
from being set totrue
with no way to reset the value.Test cases
Links for V4 and V5 JS Bin test cases are listed below:
To step through the Bootstrap code and see the issue as it happens, a breakpoint can be placed at line 103 in the V4.6.0 modal.js file or line 107 in the V5.0.1 modal.js file (the
show(relatedTarget)
function).There are also test cases for V3.4.1 and V4.0.0 to show the listener working on Bootstrap 3 and not working on the first version of Bootstrap 4. A V5.0.1 version with dropdown, collapse, popopen, and offcanvas is provided to show those components working propertly.
Test conditions
Note
The JS Bin examples would validate except for the use of
autocomplete="off"
for the checkbox inputs.autocomplete
is not valid for checkboxes, but it is the recommended solution from MDN Web Docs to control the checkbox status when a page is refreshed.The text was updated successfully, but these errors were encountered: