Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Remove dialogOpenClass from dialog.js to get in line with v2.3 #399

Closed
wants to merge 2 commits into from

Conversation

joshbroton
Copy link

I don't have older browsers available to test on my local machine, so I left body in the code in case that covers < IE10, although I'm 95% sure that can be removed.

…refox, IE10

I don't have older browsers available to test on my local machine, so I
left body in the code in case that covers < IE10
@pkozlowski-opensource
Copy link
Member

@joshbroton could you provide a plunker that demonstrates an issue that you want to address in this PR? I want to make sure that we are solving the right thing.

@pkozlowski-opensource
Copy link
Member

@angular-ui/bootstrap @SidhNor Do you guys remember why we are even adding this modal-open class to the <body>? Is it so people can do some processing when a modal is open???

I can't see the modal-open class being part of the original bootstrap CSS so I would be simply in favor or removing it. Unless there is a very good reason for its existence...

@bekos
Copy link
Contributor

bekos commented May 11, 2013

@pkozlowski-opensource .modal-open was removed in 2.3. More here: twbs/bootstrap#5719

@pkozlowski-opensource
Copy link
Member

@bekos Aha, I see! So I guess we should remove it as well and expose a promise to an opening dialog, as discussed in #354

@bekos
Copy link
Contributor

bekos commented May 11, 2013

@pkozlowski-opensource I think it is safe to remove this. And yes, I suppose it will be handy to have access to the promise (if anyone needs it).

@joshbroton
Copy link
Author

I agree. It would be nice. There needs a way to be able to stop the scroll on the elements behind a modal.

Also, a friend and I were discussing the idea of an exposed promise after the modal is finished opening last night! Use case we were trying to hack around: Calling a jquery plugin on the contents of a modal.

Do you want me to alter my pull request to remove modal-open?

@pkozlowski-opensource
Copy link
Member

@joshbroton Yep, would be awesome if you could alter this PR to remove toggling of this modal-open class altogether.

We will tackle exposing promises in the #354 I'm in the process of re-writing the $dialog service to address all the issues opened for it and I will add support for opening-promises as well.

@joshbroton
Copy link
Author

That's great. I'll have that finished for you in 5 min.

@joshbroton
Copy link
Author

OK, I've removed all references to dialogOpenClass, including the toggle.

@pkozlowski-opensource
Copy link
Member

@joshbroton thnx for the commits and pointing this one out. I was about to merge your last commit but some tests were failing and had to be modified as well. Finally I did the whole change in f009b23.

codedogfish pushed a commit to codedogfish/angular-ui-bootstrap that referenced this pull request Sep 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants