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

Allow for custom-sized modal windows #3429

Closed
RevanProdigalKnight opened this issue Mar 24, 2015 · 13 comments
Closed

Allow for custom-sized modal windows #3429

RevanProdigalKnight opened this issue Mar 24, 2015 · 13 comments

Comments

@RevanProdigalKnight
Copy link
Contributor

Currently, a modal window's size classes are restricted to Bootstrap's built-in window classes of .modal-sm or .modal-lg, but if a user adds a custom modal size class (e.g. .modal-xl or .modal-xs), they cannot use it in $modal.open().

Looking at the templates (template/modal/window.html), this could be simply modified from the existing

// ui-bootstrap-tpls.js line 4064
ng-class="{'modal-sm': size == 'sm', 'modal-lg': size == 'lg'}"

to instead be

ng-class="size ? 'modal-' + size : ''"

or something of that nature.

@wesleycho
Copy link
Contributor

Couldn't one use the windowClass option to override the behavior in the modal?

@RobJacobs
Copy link
Contributor

Here is a demo using the windowClass property to achieve this.

@RevanProdigalKnight
Copy link
Contributor Author

windowClass adds a class to the outermost <div class="modal">, whereas the modal size class is applied on the next level, <div class="modal-dialog">

@wesleycho
Copy link
Contributor

One should be able to use that top level class to do any CSS overrides of the inner class

@RevanProdigalKnight
Copy link
Contributor Author

It's rather inconvenient to have to override other CSS properties, though.

@wesleycho
Copy link
Contributor

I think in this case, it is more desirable than adding an additional watch expression & add complexity to the modal API for something that can be solved with the existing API and pure CSS.

@RevanProdigalKnight
Copy link
Contributor Author

Unless I don't understand how watch expressions work, it doesn't seem like it's adding an additional watch expression the way I suggested.

Currently, it has to check the size attribute once for each of the modal-<size> classes when generating the modal. Using the method I suggested, it still only has to check the size attribute twice (since I used a ternary)

@wesleycho
Copy link
Contributor

Ah, I see now. I think I can actually get behind this then - sorry, I misunderstood what you were suggesting as the implementation.

Would you like to file a PR with this implementation and the appropriate documentation change?

@RevanProdigalKnight
Copy link
Contributor Author

I'll have to look up how to do that (first time), but sure.

@RobJacobs
Copy link
Contributor

We might be better of changing it to a function call, I believe that would remove the watch altogether.

@chrisirhc
Copy link
Contributor

@RobJacobs , while I like less watchers, I think, for this particular case, it's better to keep it as a watcher since it's less likely that there'll be so many modals that the number of watchers per modal matters. Also, it's more readable to keep classes at the template.

We can refactor this later for performance if needed.

@RevanProdigalKnight
Copy link
Contributor Author

The suggestion does make sense. After all, in theory a modal's size shouldn't change once it's been created.

chrisirhc added a commit that referenced this issue Mar 25, 2015
@karianna karianna removed the PRs plz! label Mar 25, 2015
@karianna karianna modified the milestones: 0.13.0, Backlog Mar 25, 2015
@ermalmino
Copy link

Thank you @RevanProdigalKnight for this suggestion, it was really useful.

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

6 participants