Separate modal click functionality, utilizing backdrop callback #35554
Conversation
| @@ -15,6 +15,7 @@ | |||
| height: 100%; | |||
| overflow-x: hidden; | |||
| overflow-y: auto; | |||
| pointer-events: none; | |||
There was a problem hiding this comment.
@patrickhlauke @ffoodd I would like you opinion on this. 😃
There was a problem hiding this comment.
seems to all be working as intended
There was a problem hiding this comment.
Nope, it's not: the "Modal - Scrolling long content" demo does not scroll anymore.
VS 5.1.3: https://getbootstrap.com/docs/5.1/components/modal/#scrolling-long-content
ping @GeoSot
There was a problem hiding this comment.
tried https://getbootstrap.com/docs/5.1/components/modal/#scrolling-long-content in Safari/iOS 15.4.1, Chrome 98/Android 11, Firefox 97/Android 11, as well as Chrome/Firefox/Edge latest on Windows. seem to all behave as they should @tkrotoff, unless I'm missing something? if there is a bug/problem here, suggest opening a new issue ticket
There was a problem hiding this comment.
@patrickhlauke Please re-read my comment. Of course stable version 5.1.3 which does not contain this PR code works.
There was a problem hiding this comment.
ah, apologies @tkrotoff i did indeed read your comment backwards. i see what you mean now. confirming that with this change, the first long modal in https://deploy-preview-35554--twbs-bootstrap.netlify.app/docs/5.1/components/modal/#scrolling-long-content can't be scrolled in Chrome/Edge (though oddly, it does scroll in Firefox if you use the scrollwheel, but not with the regular scroll bar) on Windows
There was a problem hiding this comment.
Apologies, for my delayed answer. We are aware of this issue, and we are on discussion how to solve it, (or to revert this commit)
any suggestions, are welcome
There was a problem hiding this comment.
i suspect that for the first type of long modal (where the whole content of the overlay, which contains the actual modal itself, needs to scroll), there's no easy fix other than reverting the change here (as you effectively do want that container to be scrollable, so you can't neutralise pointer-events on it).
There was a problem hiding this comment.
I sadly tend to agree here. It is a pity for such a clear approach
GeoSot
force-pushed
the
gs/separate-modal-functionality-for-backdrop
branch
from
8151a06
to
0e4bb1b
Compare
GeoSot
force-pushed
the
gs/separate-modal-functionality-for-backdrop
branch
from
4058fbd
to
6be4162
Compare
GeoSot
force-pushed
the
gs/separate-modal-functionality-for-backdrop
branch
from
6be4162
to
8c37637
Compare
…odal * Revert #35554 and backdrop callback usage Explanation: In order to bypass `.modal`, was applied a css rule `pointer-events:none` which caused the side effect, and user couldn't scroll "long content modals"
…odal * Revert #35554 and backdrop callback usage Explanation: In order to bypass `.modal`, was applied a css rule `pointer-events:none` which caused the side effect, and user couldn't scroll "long content modals"
* refactor(Modal.js): stop using backdrop class to handle clicks over modal * Revert #35554 and backdrop callback usage Explanation: In order to bypass `.modal`, was applied a css rule `pointer-events:none` which caused the side effect, and user couldn't scroll "long content modals" * Update .bundlewatch.config.json Co-authored-by: Mark Otto <markd.otto@gmail.com>
This MR tries to remove some tricks that are happening, inside modal.js. Instead of listen over modal
clickevents, it delegates the listener to thebackdropalready existingclickcallback.