Skip to content

Add classNameSize (Bootstrap optional sizes)#228

Merged
makeusabrew merged 2 commits intobootboxjs:masterfrom
ThibaudD:master
Mar 4, 2014
Merged

Add classNameSize (Bootstrap optional sizes)#228
makeusabrew merged 2 commits intobootboxjs:masterfrom
ThibaudD:master

Conversation

@ThibaudD
Copy link
Contributor

A little option attribute for being able to use the Bootstrap optional sizes ("modal-lg", "modal-sm") that we have to add in the modal-dialog div. (http://getbootstrap.com/javascript/#modals-sizes)

Example for a large modal:

bootbox.dialog({
    message: "Large modal",
    title: "Large",
    classNameSize: "modal-lg"
});

@makeusabrew
Copy link
Collaborator

Hmm.

What a pity the class has to be added on the modal-dialog and not the outer wrapper (which we can already target). Either way, there are a few things which I think would benefit from addressing:

  1. The option name is awkward. classNameSize is wrong, since you could pass any arbitrary class which may or may not affect the modal's size. innerClassName would be better, albeit ugly.

  2. Related, I'm not sure if this should be exposed as a class name property, or just a "size" property. E.g. should it remain as you've done (but with a clearer name), or:

bootbox.dialog({
    message: "Large modal",
    title: "Large",
    size: "large"
});

Which internally applies whatever the relevant classes are in the relevant places. I'm leaning towards this, especially as the Bootstrap examples add classes to both the dialog and the outer wrapper (although they don't state that the outer ones are necessary).

  1. It just needs a test covering the option being supplied to the dialog rather than just using setDefaults (belt & braces)

  2. I don't scan the Bootstrap docs constantly by any means but I don't recognise the size options; are they new in 3.1.x? If so I'll need to update the supported versions and check everything else is okay (should be fine).

Thanks,

Nick

@ThibaudD
Copy link
Contributor Author

ThibaudD commented Mar 1, 2014

2)I made the changes that you advised. It works like in your example ("large" or "small").
However I didn't see what to put in the outer wrapper (I think it is custom class in the bootstrap example).

  1. I added tests covering the option for the dialog and modified the tests using setDefaults.

  2. You're right. I checked the changelog, the size option is from the version 3.1.0

makeusabrew added a commit that referenced this pull request Mar 4, 2014
Add classNameSize (Bootstrap optional sizes)
@makeusabrew makeusabrew merged commit 9ea3bb1 into bootboxjs:master Mar 4, 2014
@lusabo
Copy link

lusabo commented Jul 7, 2014

When this option will be ready?

sumityadav pushed a commit to sumityadav/bootbox that referenced this pull request Feb 1, 2018
Add classNameSize (Bootstrap optional sizes)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants