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
Add params option for button_to #10471
Conversation
@@ -287,6 +288,7 @@ def button_to(name = nil, options = nil, html_options = nil, &block) | |||
|
|||
url = options.is_a?(String) ? options : url_for(options) | |||
remote = html_options.delete('remote') | |||
params = html_options.delete('params') || {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
html_options.delete('params') { Hash.new }
@rafaelfranca updated as per your suggestions. |
@andyw8 required a rebase here. |
Rebased |
👍 |
1 similar comment
👍 |
@@ -310,6 +312,9 @@ def button_to(name = nil, options = nil, html_options = nil, &block) | |||
end | |||
|
|||
inner_tags = method_tag.safe_concat(button).safe_concat(request_token_tag) | |||
params.each do |name, value| | |||
inner_tags.safe_concat tag(:input, type: "hidden", name: name, value: value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome pull request !
In line inner_tags.safe_concat tag(:input, type: "hidden", name: name, value: value)
, value:value
should be value:value.to_param
instead. Otherwise when value is an ActiveRecord (or any fancy object), it prints a wrong value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated as per your suggestion, thanks.
+1 |
* Adds `params` option to `button_to` form helper, which renders the given hash | ||
as hidden form fields. | ||
|
||
*Andy Waite* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please move the changelog entry to the top of the file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Also it will nedd a rebase, this doesn't merge cleanly. Thank you! |
Rebased |
👍 |
1 similar comment
👍 |
The parameters are rendered as hidden form fields within the generated form. This is useful for when a record has multiple buttons associated with it, each of which target the same controller method, but which need to submit different attributes.
A good idea. |
LGTM 👍 |
I'm facing exactly the situation that this pull request would solve. I have a form that displays a table. In one of the columns, I have a checkbox for each displayed item. I have several buttons, one for each possible action that can be performed on the selected items. I don't want to have to define a separate route for each action, because the selected action is more like a function parameter for one action. The way I've solved this for now is to add some Javacode that triggers on the click of any one of the buttons (based on their class), and then copies the id of the button to a hidden input field of the form, which is then transmitted to the controller in the parameter list of the HTTP call. |
👍 Sounds like a nice convention that would remove a couple lines of code |
y u no merge? (are memes in PR discussions retro enough to be hip again?) |
This PR required a rebase/ |
Add params option for button_to Conflicts: actionpack/CHANGELOG.md
The parameters are rendered as hidden form fields within the generated form. This is useful for when a record has multiple buttons associated with it, each of which target the same controller method, but which need to submit different attributes.
For example, a bug tracking system may have buttons to mark an issue as Closed or Fixed. These should both make a PATCH request to
/issues/:id
. This could be implemented by adding member routes to the Issue resource, but for some apps that may result in a large number of additional entries in routes.rb.I understand that there is already a mechanism for adding additional params to the query string for
button_on
, but it's usually preferable to have this data in the request body.