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

Upgrade Modal script for Bootstrap 3.1 #1718

Closed
christrude opened this issue Jan 31, 2014 · 18 comments
Closed

Upgrade Modal script for Bootstrap 3.1 #1718

christrude opened this issue Jan 31, 2014 · 18 comments

Comments

@christrude
Copy link

Bootstrap 3.1 adds some modal configurability, 2 classes modal-lg and modal-sm that need to be added to the div where modal-dialog lives, however, angularUI-bootstrap builds the modal down to the modal-content level, so we can't add those classes without a destructive jQuery.

@mvhecke
Copy link
Contributor

mvhecke commented Jan 31, 2014

You could create 2 additional templates for now, but I agree with you that this should be supported.

Edit: You can use the windowClass for this.

@Foxandxss
Copy link
Contributor

Yeah, the plan could be updating what is needed to 3.1, but we just need to finish some stuff first and I guess that we could start the updating :)

@pkozlowski-opensource
Copy link
Member

@Gamemaniak I don't think windowClass will work here as there are 2 places (and 2 different CSS classes) that need to be added... I'm more and more thinking that we need to add some sort of override for modal templates (well, for all directive templates in fact) per instance. Got some ideas on my mind, need to put it in writing.

@mvhecke
Copy link
Contributor

mvhecke commented Feb 1, 2014

@pkozlowski-opensource You're right, should've read the documentation better. I'm curious about your ideas as this is becoming quite a common problem.

@pkozlowski-opensource
Copy link
Member

@Gamemaniak see this comment: #1706 (comment) - this is more or less the idea I've got on my mind. The reasoning here is that users might want to re-use the same custom template URL so it makes sense to name it as a "family" (ex., for modals, sth like "normal", "small", "big", "mycustom") instead of re-typing the full URL on each usage.

@Siyfion
Copy link

Siyfion commented Feb 6, 2014

I'm also trying to get this new Bootstrap functionality working with the modal directive, but as @pkozlowski-opensource states; you can add the bs-modal-lg class to the windowClass property, but it still won't apply the modal-lg class to the inner <div class="modal-dialog"></div>. So the window will float correctly, but wont be any different size.

@Foxandxss
Copy link
Contributor

@Siyfion Speaking from total ignorance of bootstrap 3.1, if you need to add a class to the template, you can always provide your own template.

We will update our stuff to 3.1 for sure, but we have a lot of more work to do :)

@Siyfion
Copy link

Siyfion commented Feb 6, 2014

@Foxandxss I do provide my own template, but the <div> that needs that attribute is outside of the scope of the template, as it's the template's parent.

@Foxandxss
Copy link
Contributor

@Siyfion I meant the parent template. In this library you can always provide your templates to every directive (I am not talking about the directive you use to create a modal).

@jla
Copy link

jla commented Feb 16, 2014

In the meantime you can use windowClass: 'my-modal' with something like:

.my-modal > .modal-dialog {
    width: 900px;
}

@christrude
Copy link
Author

I haven't been able to get windowClass to work, but I will give it another try.

@christrude christrude reopened this Mar 5, 2014
@christrude
Copy link
Author

Oops, I'm a dummy, didn't mean to close the issue.

@jeremy1015
Copy link

+1 I really need modal-lg and modal-sm in my application.

@chrisirhc
Copy link
Contributor

See #1885, considering that.

@jeremy1015
Copy link

Thanks Chris!

@Siyfion
Copy link

Siyfion commented Apr 2, 2014

Well that's certainly one way of doing it, and it works!

@pkozlowski-opensource
Copy link
Member

We are going to handle it via #2084 by adding a new "size" attribute. #2084 should land today or tomorrow and be released within 2-3 days.

@Siyfion
Copy link

Siyfion commented Apr 23, 2014

@pkozlowski-opensource How are we looking for a release version containing this change? It's going to be such a useful addition, want to get my hands on it! 😉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants