Skip to content
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

Remove .modal-open class after backdrop is hidden #14570

Merged
merged 1 commit into from
Sep 22, 2014

Conversation

hnrch02
Copy link
Collaborator

@hnrch02 hnrch02 commented Sep 8, 2014

Fixes #14274 and #14632.

@cvrebert cvrebert added the js label Sep 8, 2014
@cvrebert cvrebert added this to the v3.2.1 milestone Sep 8, 2014
@cvrebert
Copy link
Collaborator

cvrebert commented Sep 8, 2014

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Sep 8, 2014

Sure, give me a second, I'll post a screencap.

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Sep 8, 2014

Notice how the scrollbar fades to the original one in the first tab (new behavior) and immediately changes back to the body scrollbar in the second one (current behavior).

Screencap

@X-Coder
Copy link

X-Coder commented Sep 16, 2014

The fix is not complete. You should also think of cases when multiple instances are opened at the same time: only when the last dialog is closed, it should remove the .modal-open class.

For example I use this for forms inside modals, popping up another modal when error occurs.

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Sep 16, 2014

We do not support overlapping modals and state this explicitly in the docs:

Overlapping modals not supported

Be sure not to open a modal while another is still visible. Showing more than one modal at a time requires custom code.

@X-Coder
Copy link

X-Coder commented Sep 16, 2014

Yes I know you are currently not supporting it, thats why I need to write workarounds each time I need to use this. And I'm guessing I'm not the only one. I hoped for a future release you could support it, would make life easier.

@cvrebert
Copy link
Collaborator

Anyway, that issue is broader than just this pull request. We're unlikely to try to add such a feature before v4, and even then it's still quite uncertain.

@hnrch02
Copy link
Collaborator Author

hnrch02 commented Sep 16, 2014

Also, you might be interested in nakupanda/bootstrap3-dialog, which we have listed in the "Resources" section of the Expo.

@X-Coder
Copy link

X-Coder commented Sep 16, 2014

Thank you hnrch02, nakupanda's bootstrap3-dialog is very good, I'm using it already for everything were I need dialogs, it's a lot easier to use than plain bootstrap.

Of course, it has the same scrollbar-shifting issues on "stacked" dialogs because it sits on top of bootstrap's modal (which doesn't support it - as you wrote). Debugging of it showed its related to .modal-open class, so I needed to write workarounds for both.

@hnrch02 hnrch02 force-pushed the remove-modal-open-after-backdrop-closed branch from f6117e1 to c4f431d Compare September 17, 2014 07:21
hnrch02 added a commit that referenced this pull request Sep 22, 2014
…closed

Remove `.modal-open` class after backdrop is hidden
@hnrch02 hnrch02 merged commit acf3c0b into master Sep 22, 2014
@hnrch02 hnrch02 deleted the remove-modal-open-after-backdrop-closed branch September 22, 2014 19:44
@cvrebert cvrebert mentioned this pull request Sep 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Small Modal Shifting 15px when closing (Chrome Win7)
3 participants