Skip to content

Conversation

@atipugin
Copy link
Contributor

@atipugin atipugin commented Sep 2, 2014

No description provided.

@carloslopes
Copy link
Member

Hey @atipugin, thanks for your PR!

For me it's fine, let's just ask @datWav what he think about it, because the wrapper_class option was added by him.

@datWav what do you think? Do we keep both wrapper_* options or is it fine to use only the wrapper_options?

@potenza
Copy link
Member

potenza commented Sep 4, 2014

@atipugin thanks! I like this, the only thing I'm concerned about is that it makes it a little more verbose to add a wrapper class. I don't use this option all that much so it's not really a big deal for me.

@carloslopes
Copy link
Member

@potenza yeah, we can keep both options too (wrapper_class that is most commonly used and wrapper_options that is more complete)

@datWav
Copy link

datWav commented Sep 5, 2014

@potenza, @carloslopes I think both options are good. It is simpler to write only wrapper_class if you want to add just the class.
The wrapper_options possibility is the same as form_group label: {text: 'label'}.

What do you think if we use only wrapper: {} and parse all options to the wrapper element?

@carloslopes
Copy link
Member

@datWav I agree.

So, we will have both options, wrapper and wrapper_class.

Can you work on this @atipugin?

@atipugin
Copy link
Contributor Author

atipugin commented Sep 6, 2014

Sure but no sooner than next week ;)

@carloslopes
Copy link
Member

Np!
On Sep 6, 2014 2:06 PM, "Alexander Tipugin" notifications@github.com
wrote:

Sure but no sooner than next week ;)


Reply to this email directly or view it on GitHub
#136 (comment)
.

@atipugin
Copy link
Contributor Author

Done.

@atipugin
Copy link
Contributor Author

Rebased my PR on top of current master, so we can merge it now :)

@potenza
Copy link
Member

potenza commented Sep 11, 2014

👍

potenza added a commit that referenced this pull request Sep 11, 2014
Allow to pass any attributes to wrapper (not just class)
@potenza potenza merged commit 14771ae into bootstrap-ruby:master Sep 11, 2014
@pelted
Copy link

pelted commented Dec 2, 2014

What about turning of the wrapper entirely. I came across issue #62 with regard to have more than one text field in a row when using horizontal layout. The problem with the provided solution is that the fields still get wrapped thus making the solution to that issue useless without being able to turn off the wrapper.

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.

5 participants