-
Notifications
You must be signed in to change notification settings - Fork 6.7k
Remove dialogOpenClass from dialog.js to get in line with v2.3 #399
Conversation
…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
@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. |
@angular-ui/bootstrap @SidhNor Do you guys remember why we are even adding this I can't see the |
@pkozlowski-opensource |
@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). |
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? |
@joshbroton Yep, would be awesome if you could alter this PR to remove toggling of this We will tackle exposing promises in the #354 I'm in the process of re-writing the |
That's great. I'll have that finished for you in 5 min. |
OK, I've removed all references to dialogOpenClass, including the toggle. |
@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. |
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.