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

feat($modal): support modal window sizes #2084

Closed
wants to merge 1 commit into from

Conversation

gazoakley
Copy link
Contributor

Following on from comments by @bekos in #1885 this pull request allows specifying a size of either sm or lg when creating a modal dialog

@pkozlowski-opensource
Copy link
Member

@gazoakley thnx for this PR, I like the direction it is taking! I would like to ask you for 2 changes, though:

  • could we pass all the way down the size property to the modal window template and calculate the actual class in the template itself? This way people can use modals with any markup, not only bootstrap's one;
  • we will need some tests to prove that this new functionality works correctly.

Could you please do those changes and squash the commits afterwards? Thank you!

@bekos
Copy link
Contributor

bekos commented Apr 19, 2014

@gazoakley I don't know if I help, but for the template expression that @pkozlowski-opensource is talking about you can use something like: ng-class="{'modal-{{size}}': size}

@gazoakley gazoakley changed the title Add size property to options for modal.open feat(modal): Add size property to options for modal.open Apr 20, 2014
@gazoakley
Copy link
Contributor Author

@bekos I like the conciseness of that, but unfortunately Angular ended up appending the class as 'modal-' instead of 'modal-sm' or 'modal-lg'. At the moment I'm using ng-class="{'modal-sm': size == 'sm', 'modal-lg': size == 'lg'}" - not so pretty but it works.

@gazoakley
Copy link
Contributor Author

If people are happy with these changes let me know and I'll squash the commit.

@pkozlowski-opensource
Copy link
Member

LGTM. Please squash commits. Also it would be great if the commit message follow our convention, sth like: feat($modal): support modal window sizes

@gazoakley gazoakley changed the title feat(modal): Add size property to options for modal.open feat($modal): support modal window sizes Apr 20, 2014
@gazoakley
Copy link
Contributor Author

All done. Let me know if you'd like anything changed.

@pkozlowski-opensource
Copy link
Member

Thnx @gazoakley, your commit looks good now. But it looks like it is not based on the latest master as it doesn't merge cleanly. Will try to merge manually later on, but if you could rebase it on top of the latest master it would be helpful.

@gazoakley
Copy link
Contributor Author

Tidying this now.

Allows use of the size modifier classes shipped with Bootstrap 3.1
onwards. Specify either 'sm' or 'lg' to get a small/large modal dialog.
Defaults to normal size dialog if not specified.
@gazoakley
Copy link
Contributor Author

Should all be GTG.

@pkozlowski-opensource
Copy link
Member

@gazoakley thnx, it landed! Much appreciated!

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.

None yet

3 participants