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

Enable button_to to use button without block #35143

Closed
wants to merge 1 commit into from

Conversation

duleorlovic
Copy link
Contributor

@duleorlovic duleorlovic commented Feb 3, 2019

Summary

button_to 'Label', url generates <input value='Label'> but if we
want to generate <button>Label</button> than we can pass the argument

   button_to 'Label', url, element: :button

This way we can generate <button> for block and non block version of
button_to.

<button> element is more powerful than <input> element since it can contain
other markup (value of <input> can be only a text).

I found useful while testing since we can use assert_select 'button', 'Label' (instead of assert_select 'input[value=?]', 'Label').

Do you think it is useful ? If so, I can update docs...
Or maybe we can replace <input> with <button> completely (Does anyone know of the advantage of <input> over a <button>) ?

@rails-bot rails-bot bot added the actionview label Feb 3, 2019
@foucist
Copy link

foucist commented Feb 3, 2019

It seems a bit weird to repeat 'button' twice (i.e. button_to and :button) maybe it's better to make a semantic_button_to helper method instead? That way there's opportunity to create more semantic url helpers down the road.

@rafaelfranca
Copy link
Member

I think we used input mostly because of IE6. I'd just change the default behavior to always generate a button, with an application wide configuration to generate inputs. Can you work on that?

@duleorlovic
Copy link
Contributor Author

@rafaelfranca yea, button as default element is expected, I'm working on it and will report all implications.
Can you explain more with an application wide configuration to generate inputs ? Did you mean something like this configuration?

@rafaelfranca
Copy link
Member

Yes

@duleorlovic
Copy link
Contributor Author

@rafaelfranca I added button_to_generates_input configuration so it can be used to generate <input> instead of <button>.

I think that this change is safe and will not have big impact since button_to (without block) is not used so often.

@rails-bot
Copy link

rails-bot bot commented Dec 18, 2019

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Dec 18, 2019
@duleorlovic
Copy link
Contributor Author

duleorlovic commented Dec 18, 2019

@rafaelfranca do you know how should I "unstale" this pull requests or how to make it more popular?

@rails-bot rails-bot bot removed the stale label Dec 18, 2019
@rails-bot
Copy link

rails-bot bot commented Mar 17, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Mar 17, 2020
@rails-bot rails-bot bot closed this Mar 24, 2020
@rafaelfranca rafaelfranca reopened this Mar 30, 2020
@rails-bot rails-bot bot removed the stale label Mar 30, 2020
Copy link
Member

@rafaelfranca rafaelfranca left a comment

Choose a reason for hiding this comment

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

Sorry for the delay reviewing this. Code looks good with one exception that I just commented. Can you also add a CHANGELOG entry?

@@ -119,6 +119,7 @@ def load_defaults(target_version)

if respond_to?(:action_view)
action_view.default_enforce_utf8 = false
action_view.button_to_generates_input = false
Copy link
Member

Choose a reason for hiding this comment

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

Can you move to the 6.1 defaults instead of 6.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved and rebased to the latest master commit.

`<button>` element is more powerful than `<input>` element since it can
contain other markup (value of `<input>` can only be a text).

To keep generating `<input>` when `button_to` is used without block,
there is a configuration:

    Rails.configuration.action_view.button_to_generates_input = true
@duleorlovic
Copy link
Contributor Author

Thank you for reviewing, I added to CHANGELOG and moved configuration to 6.1

@rails-bot
Copy link

rails-bot bot commented Jun 29, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
Thank you for your contributions.

@rails-bot rails-bot bot added the stale label Jun 29, 2020
@rails-bot rails-bot bot closed this Jul 6, 2020
@duleorlovic
Copy link
Contributor Author

I do not understand why this is not merged yet...

@seanpdoyle
Copy link
Contributor

This has been merged as part of #40747.

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.

4 participants