Skip to content

Conversation

@jsaraiva
Copy link
Contributor

The button, submit, and primary helpers can now receive an additional option, extra_class. This option allows us to specify additional CSS classes to be added to the corresponding button/input, while maintaining the original default ones. E.g., a primary button with an extra_class 'test-button' will have its final CSS classes declaration as 'btn btn-primary test-button'.

This is particularly useful for adding extra details to buttons without forcing you to repeat the
Bootstrap classes. You can just write extra_class: 'test-button' instead of class: 'btn btn-primary test-button'.

@jsaraiva
Copy link
Contributor Author

Please ignore commit 00713e5 . It is just there in my repository until PR #480 is merged.

Copy link
Contributor

@lcreid lcreid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR. It's beautiful. I especially like the thorough documentation.

We're trying to limit the number of new options for the helpers, but I think in this case the new option is valuable, and I can't think of another way to do it that wouldn't break backwards compatibility, so I look forward to seeing this PR merged.

@lcreid lcreid requested a review from mattbrictson August 22, 2018 16:53
@jsaraiva
Copy link
Contributor Author

Hi, Larry!

Thank you very much. :-)

I only suggested this because I have had a lot of cases in which I used

f.button 'some text', class: 'btn btn-primary my-special-css-class'

when it would have been more readable to just do

f.primary 'some text', class: 'my-special-css-class'

A typical example of my-special-css-class is save, for the main "Save changes" button in a form.
Same goes for the button helper, with 'btn btn-secondary'.

However, the class attribute is reserved, and for good reason.
Of course, it could be argued that, in primary's case, it should always add the btn btn-primary classes (because otherwise we would just use the button helper), but backward-compatibility is very important (I know; I confess I was caught off-guard by the change to the wrapper class in the select helper), and this was the only way I could figure out to achieve my goals -- simplify my code -- without breaking anything.

BTW, I first tried just using

options.reverse_merge! class: "btn btn-primary #{options.delete(:extra_class)}"

but it broke a lot of tests because of the extra space char in the class declaration. If you wish to remove the private helper method (and change the other tests to reflect that space), that should work too.

Cheers!

@lcreid lcreid merged commit 83b01ab into bootstrap-ruby:master Sep 13, 2018
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.

2 participants