-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Compatibility with Bootstrap 3 #164
base: master
Are you sure you want to change the base?
Conversation
This is a basic compatibility update. I essentially made the diff between bootstrap-modal 2.1 JS files and Bootstrap 3's modal.js, merged them. Some features of Bootstrap modal may be broken, but it seems to work for the most at this point.
+1 |
not working closing modals for me, anyway +1 for bootstrap 3 compatibility, so thanks for this patch |
+1 Bruno Machado On Mon, Sep 2, 2013 at 2:35 PM, Crempa notifications@github.com wrote:
|
Hey thanks for the PR. I ended up accomplishing this with just a simple css patch file. I didn't want to break backwards compatibility at this point so for now you'll need to include an extra css file for BS3. Hope this works for most folks for the time being. |
Hi @jschr, When you say you "ended up accomplishing with just a simple css patch file", you mean the closing modals issue on my PR or the whole compatibility thing? If this is for the whole Bootstrap 3 compatibility, I'm curious! Did you share it somewhere? Thanks & cheers! |
@rchampourlier My goal for addressing BS3 was to not break backwards compatibility with BS2 as this plugin was originally created to address the problems with BS2's default modal. Since lots of folks are still on BS2 and probably will be for quite some time I wanted to leave this plugin written primarily for BS2 but offer an upgrade patch. I believe BS3 default modals address a lot of the issues that this plugin was created to fix. That being said I'll leave this PR open for the meantime as I'm sure there are some people would find this useful as well. Thanks |
Oh ok, I haven't even tried to play with BS 3 modals without your plugin. I should try this now indeed, thanks for the details! |
I have tested bs3 and the ajax modal has some issues. After it closes, the page-overflow class stays on the body, not letting any click on the page. Please use the actual bs3 files from the cdn in your bs3.html example to ensure that everything works with bs3. |
@liago86 I am also hitting this issue ! |
@liago86, @urzur Can you guys be more specific or provide an example of the problem? Unless I'm missing something I'm not seeing any problems with the Ajax example on the demo page in chrome. @liago86 I'm using the same files as www.getbootstrap.com last I checked, if you can provide a link to the cdn that would be appreciated. |
@jschr The js used in the bs3 demo page example is the http://getbootstrap.com/2.3.2/assets/js/bootstrap.js. The official bs3 js from the cdn is the //netdna.bootstrapcdn.com/bootstrap/3.0.0/js/bootstrap.min.js. Replace the one used now with the one i mention, and you will see the problem at the AJAX (via jQuery.load) example. The problem is that when the modal is closed, the html body still has a "page-overflow" class. I am not sure how i can provide more details. |
@jschr I just re-tested the case and i really can't reproduce the same bug. I am not sure what is going on. I will come back with more details if i will be able to reproduce the bug. |
@jschr Cool, i saw you updated your demo page with the bs3 cdn resources. Now, please check http://jschr.github.io/bootstrap-modal/bs3.html with Firefox 23.0.1, and you will see the bug i have mentioned. With Chrome or IE10 it's fine. |
Boostrap 3 stackable-modals demo doesnt work. (firefox 23) |
@kosekk The bug you are mentioning is being caused by the same reason with mine. The page-overflow class remains on the html body after the modal is closed. |
I couldn't wait for update so I made some changes in bootstrap-modalmanager.css - it is just a line, but it works for me and removes backdrop layer. May be helpfull (line in file: 286) :
" |
this works much better:
|
kosekk, thanks for posting this. Works great and corrects the darker background on every call to the modal. Has anyone figured out how to bring up click events ? After the modal closes, once my page is done, my click events disappear :/ |
If the DIV with class 'modal-scrollable' is removed the modal does not open again. When I just hide it, it works.
|
+1 for click events issue. Using Firefox 23 Windows, the bs3 demo page doesn't work anymore after closing a single modal. Works OK in Chrome. |
I believe I have a fix for this issue you guys are having: 3731fd5 Let me know if that works. Thanks |
Nice, it works fine now on Firefox. Thanks a lot @jschr ! |
When using Bootbox.js together with this patch, it does not work. Bootstrap3 introduced the two wrappers ".modal-dialog" and ".modal-content" which, when used (according to the BS3 specs) are not working. The result is a double modal frame/border. |
I'm also experiencing the double modal frame/border issue reported by @richterfritz with BS3. For the time being, I'm removing the .modal-dialog div as a quick fix. |
I'm also having the same issue as @richterfritz with BS3. |
If I add the following to the bootstrap-modal-bs3patch.css, it fixes the double border for me (and also makes it look nicer in my opinion because of less padding for the dialog). .modal-dialog { |
One last thing. I also changed the width of the modal definition in bootstrap-modal-bs3patch.css to be 600px since that matches up to bs3. /*!
body.modal-open, .modal { |
Template for modal windows are different! Modal window does not close by clicking dismiss button |
That's not a patch, it overrides the entire library (what it shouldn't). I was looking over the changes and it is not possible to say what's namely changed to fix the compatibility. Push request should only contain related changes. |
This is a basic/hacky compatibility update. I essentially made the diff between bootstrap-modal 2.1 JS files and Bootstrap 3's modal.js, then merged them.
Some features of Bootstrap modal may be broken, but it seems to work for the most at this point.
I'll keep this branch updated as soon as I identify something missing. I'm sorry I could not completely review the code to ensure it was rightfully done, but I hope it may help with starting the update.