Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow rendering of form submit buttons as HTML buttons, instead of input #463

Merged
merged 4 commits into from
May 29, 2018
Merged

Conversation

jsaraiva
Copy link
Contributor

As the title says, this PR allows devs to render submit buttons as HTML buttons, instead of using input tags.
The major advantage of this is that these submit buttons can then be styled, because they can have HTML content, as well as support for CSS pseudo-selectors.

All existing code should keep working normally. To use buttons instead of input tags, provide the render_as_button option with a truthy value:
<%= f.primary t('.signin_button'), render_as_button: true %>

Cheers!

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 the PR! We appreciate your effort to make this gem better. And my apologies for the slow reply.

This looks like a good addition. However, I was wondering if you considered adding a button helper to the bootstrap_form helpers, instead of implementing an option on submit?

@jsaraiva
Copy link
Contributor Author

Hi, Larry!

I've been thinking about your suggestion, and you're right 👍 . I had initially implemented it like that (in the submit helper) because I don't agree much with Rails' semantic treatment of the input helper. IMHO, a submit helper should be renderable as any element that HTML allows for submitting forms; currently, that is input[type="submit"] and button[type="submit"] (or button[type=""]) tags.

But, this would be a considerable departure from Rails' current helpers, and thus go against the philosophy of this great gem. :-)
Therefore, I have reverted the change to the submit helper and, following your advice, added a button helper (that just adds a CSS class if none is given, just like submit does).
I have however changed the primary helper so that, if it receives a :render_as_button option or a content block, it will output a button instead an input tag.

Thanks for the tip!

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.

Nicely done! This looks great. I hate to have to be so picky, but could you also please add button to the long list of helpers that's about 1/3 of the way through the README.md file? Thanks again for the great work. Once that's done we should be good to go.

@jsaraiva
Copy link
Contributor Author

Hi, Larry!

It's no problem 👍 . I should have done that to begin with. :-(

I also took the liberty of adding the submit helper to the list, and an explanation to the primaryhelper about the rendering as HTML button. The "save" button example used is similar to what I usually do with this new addition, but I typically use SCSS in conjunction with FontAwesome's source code to achieve the same effect. :-)

Let me know if you wish me to revert something, or if you need anything else.

Cheers!

@lcreid
Copy link
Contributor

lcreid commented May 21, 2018

@mattbrictson could you please take a look at this? I think @jsaraiva found a legit hole in our coverage of Rails helpers, and filled it nicely. Let me know if you want me to merge it, or merge it yourself if that's easier.

@mattbrictson
Copy link
Contributor

This looks good. I'll clean up the conflicts and merge. Thanks!

added button helper, similar to submit
changed primary helper so that it can render as an input submit tag _or_ a button, depending on whether it is given a content block or an explicit option to render as a button
added missing helpers to list
added explanation about render_as_button option to Submit Buttons section
@mattbrictson mattbrictson merged commit 51fb089 into bootstrap-ruby:master May 29, 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.

None yet

3 participants