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

fix #43146 button_to now not works with hash params for url #43157

Closed
wants to merge 2 commits into from

Conversation

qinmingyuan
Copy link

Summary

def button_to(name = nil, options = nil, html_options = nil, &block)
html_options, options = options, name if block_given?
options ||= {}
html_options ||= {}
html_options = html_options.stringify_keys
url = options.is_a?(String) ? options : url_for(options)
remote = html_options.delete("remote")
params = html_options.delete("params")
method = html_options.delete("method").to_s
method_tag = BUTTON_TAG_METHOD_VERBS.include?(method) ? method_tag(method) : "".html_safe
form_method = method == "get" ? "get" : "post"
form_options = html_options.delete("form") || {}
form_options[:class] ||= html_options.delete("form_class") || "button_to"
form_options[:method] = form_method
form_options[:action] = url
form_options[:'data-remote'] = true if remote
request_token_tag = if form_method == "post"
request_method = method.empty? ? "post" : method
token_tag(nil, form_options: { action: url, method: request_method })
else
""
end
html_options = convert_options_to_data_attributes(options, html_options)
html_options["type"] = "submit"
button = if block_given? || button_to_generates_button_tag
content_tag("button", name || url, html_options, &block)
else
html_options["value"] = name || url
tag("input", html_options)
end
inner_tags = method_tag.safe_concat(button).safe_concat(request_token_tag)
if params
to_form_params(params).each do |param|
inner_tags.safe_concat tag(:input, type: "hidden", name: param[:name], value: param[:value])
end
end
content_tag("form", inner_tags, form_options)
end

In this line 348, name always be the hash { action: 'sync' }, url is url_for(action: 'sync'), is string

content_tag("button", { action: 'sync' }, { class: 'button is-success' }, &block),

html_options becomes { action: 'sync' }, the real html options { class: 'button is-success' } lost, if use a string as param, the param will be ignore because of block exist

@rails-bot rails-bot bot added the actionview label Sep 3, 2021
@qinmingyuan qinmingyuan marked this pull request as ready for review September 3, 2021 04:05
@qinmingyuan
Copy link
Author

@ rafaelfranca Could you please help review this PR, you have reviewed the PR #40747 , which broke this.

Copy link
Contributor

@marcelolx marcelolx left a comment

Choose a reason for hiding this comment

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

@qinmingyuan Could you add a test case and a changelog entry?

@qinmingyuan
Copy link
Author

@qinmingyuan Could you add a test case and a changelog entry?

ok, I will do it

rafaelfranca added a commit that referenced this pull request Dec 20, 2021
    Add test for #43157

    fix test

    fix #43146

Co-Authered-By: qinmingyuan <mingyuan0715@foxmail.com>
rafaelfranca added a commit that referenced this pull request Dec 20, 2021
    Add test for #43157

    fix test

    fix #43146

Co-authored-by: qinmingyuan <mingyuan0715@foxmail.com>
@rafaelfranca
Copy link
Member

Merged in 075ffd5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants