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 button_to helper for data-type option #3154

Closed
wants to merge 1 commit into from

Conversation

ihower
Copy link
Contributor

@ihower ihower commented Sep 28, 2011

We may use data-type attribute to set Ajax request type for "data-remote" requests. (see jquery-ujs wiki)
link_to helper has no problem, but button_to will add "data-type" to the wrong input tag and does not work:

 <%= button_to "foo", "/bar", :remote => true, "data-type" => "json" %>

 <form method="post" action="/bar" data-remote="true" class="button_to"><div><input data-type="json" type="submit" value="foo" />...</div></form>

The "data-type" should be in form tag with "data-remote" , not input tag. This patch result:

<form method="post" action="/bar" data-remote="true" data-type="json" class="button_to"><div><input  type="submit" value="foo" />...</div></form>

It will be nice also backport to 3-1-stable branch.

… put into form tag with data-remote, not input tag.
@spastorino
Copy link
Contributor

+1 can you take a look at link_to also and the rest of the helpers to see if we can apply the same pattern?.
Thanks.

@spastorino
Copy link
Contributor

Actually link_to uses tag_options so is right :)

@spastorino
Copy link
Contributor

@ihower, we were discussing with @josevalim ... what about this way ...

<%= button_to "foo", "/bar", :remote => true, :form => { "data-type" => "json" } %>

Then is up to the user to pass whatever they want to build the form attributes.

@ihower
Copy link
Contributor Author

ihower commented Sep 28, 2011

That's great and more general. I send another pull request #3156

@spastorino spastorino closed this Sep 28, 2011
@ihower ihower deleted the enhance_button_to_helper branch August 16, 2015 03:51
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

2 participants