Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Deprecate `:confirm` in favor of `:data => { :confirm => 'Text' }` option #6613

Merged
merged 1 commit into from

5 participants

@carlosgaldino

Just like 21141e7 this PR deprecates the :confirm option.

/cc @rafaelfranca

Related to #6614

@rafaelfranca
Owner

@carlosgaldino I think we need to change the scaffold generator to generate :data => { :confirm => 'Text' }

@rafaelfranca rafaelfranca was assigned
@carlosgaldino

Updated

@rafaelfranca
Owner

I think that is good to have some tests with :data => { :confirm => 'Text' }.

Also would be nice if you change the commit message with the reason of this deprecation. Something like this:

As :confirm is an UI specific option is better to use the data attributes, teaching the users about unobtrusive javascript and how the Rails works with it.

@carlosgaldino carlosgaldino Deprecate `:confirm` in favor of `:data => { :confirm => 'Text' }` op…
…tion

This deprecation applies to:
`button_to`
`button_tag`
`image_submit_tag`
`link_to`
`submit_tag`

As :confirm is an UI specific option is better to use the data attributes,
teaching users about unobtrusive JavaScript and how Rails works with it.
fc092a9
@rafaelfranca rafaelfranca merged commit 6347554 into rails:3-2-stable
@b4syth

Shouldn't the comments (rdoc) be updated to reflect the change in options from :confirm to :data => {:confirm => 'test'}

@b4syth yes, they should. If there's any missing doc change, you can please go ahead and do them in lifo/docrails. Thanks!

@dball

Perhaps it's a little too late to comment, but I can't help but think this change moves us to coding to the implementation, rather than the interface.

link_to('Text', 'http://example.com', :confirm => 'Seriously?')

Is a high-level abstraction.

link_to('Text', 'http://example.com', :data => { :confirm => 'Seriously?' })

Is not just more seemingly unnecessarily more verbose, it hard-codes the assumption that the confirmation dialog is going to be rendered as a data-confirm attribute. What if HTML5 introduces a different semantic attribute or other mechanism for confirmation dialogs?

@rafaelfranca

@dball the data-confirm is not handled by rails it self.

With this change we are teaching that this confirmation dialog is handled by the javascript driver and not by the Rails.

This change is exactly the opposite of hard-code. It is removing the hard coded assumption that the :confirm need to generate data-confirm. If HTML5 introduces a different semantic attribute we don't need to change anything in the Rails side.

@dball

I don't think I agree with the assertion that this change is exactly the opposite of hard-code. There isn't currently a hard-coded assumption that :confirm generates a data-confirm attribute, is there? The UJS drivers depend on that behavior, to be sure, but that's different than the contract between the link_to helper and its users. Forcing the latter to insert a :data hash strongly implies that the link_to implementation is going to render a data-confirm attribute.

If we're dead set against a top-level :confirm abstract option (which I'm not), using a :ujs hash instead of a :data hash might be a better way to educate users that those options are intended for a UJS driver.

I realize this is bikeshedding to an extent, so I won't waste your time with an extended commentary; I just wanted to register my objection to an API change that I don't think takes us in the right direction. Thanks for listening.

@rafaelfranca

@dball :confirm always generates a data-confirm:

if confirm = options.delete("confirm")
  options["data-confirm"] = confirm
end

No problem. Thank you for the considerations.

@dacamo76 dacamo76 referenced this pull request in decioferreira/bootstrap-generators
Merged

Remove deprecated use of :confirm in scaffold templates #12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 5, 2012
  1. @carlosgaldino

    Deprecate `:confirm` in favor of `:data => { :confirm => 'Text' }` op…

    carlosgaldino authored
    …tion
    
    This deprecation applies to:
    `button_to`
    `button_tag`
    `image_submit_tag`
    `link_to`
    `submit_tag`
    
    As :confirm is an UI specific option is better to use the data attributes,
    teaching users about unobtrusive JavaScript and how Rails works with it.
This page is out of date. Refresh to see the latest.
View
4 actionpack/CHANGELOG.md
@@ -1,5 +1,9 @@
## Rails 3.2.6 (unreleased) ##
+* Deprecate `:confirm` in favor of `':data => { :confirm => "Text" }'` option for `button_to`, `button_tag`, `image_submit_tag`, `link_to` and `submit_tag` helpers.
+
+ *Carlos Galdino*
+
* Allow to use mounted_helpers (helpers for accessing mounted engines) in ActionView::TestCase. *Piotr Sarnacki*
* Include mounted_helpers (helpers for accessing mounted engines) in ActionDispatch::IntegrationTest by default. *Piotr Sarnacki*
View
6 actionpack/lib/action_view/helpers/form_tag_helper.rb
@@ -423,6 +423,8 @@ def submit_tag(value = "Save changes", options = {})
end
if confirm = options.delete("confirm")
+ ActiveSupport::Deprecation.warn ":confirm option is deprecated and will be removed from Rails 4.0. Use ':data => { :confirm => \'Text\' }' instead"
+
options["data-confirm"] = confirm
end
@@ -475,6 +477,8 @@ def button_tag(content_or_options = nil, options = nil, &block)
end
if confirm = options.delete("confirm")
+ ActiveSupport::Deprecation.warn ":confirm option is deprecated and will be removed from Rails 4.0. Use ':data => { :confirm => \'Text\' }' instead"
+
options["data-confirm"] = confirm
end
@@ -510,6 +514,8 @@ def image_submit_tag(source, options = {})
options = options.stringify_keys
if confirm = options.delete("confirm")
+ ActiveSupport::Deprecation.warn ":confirm option is deprecated and will be removed from Rails 4.0. Use ':data => { :confirm => \'Text\' }' instead"
+
options["data-confirm"] = confirm
end
View
9 actionpack/lib/action_view/helpers/url_helper.rb
@@ -628,8 +628,13 @@ def convert_options_to_data_attributes(options, html_options)
html_options["data-disable-with"] = disable_with
end
- html_options["data-confirm"] = confirm if confirm
- add_method_to_attributes!(html_options, method) if method
+ if confirm
+ ActiveSupport::Deprecation.warn ":confirm option is deprecated and will be removed from Rails 4.0. Use ':data => { :confirm => \'Text\' }' instead"
+
+ html_options["data-confirm"] = confirm
+ end
+
+ add_method_to_attributes!(html_options, method) if method
html_options
else
View
38 actionpack/test/template/form_tag_helper_test.rb
@@ -385,9 +385,18 @@ def test_submit_tag_with_no_onclick_options
end
def test_submit_tag_with_confirmation
+ assert_deprecated ":confirm option is deprecated and will be removed from Rails 4.0. Use ':data => { :confirm => \'Text\' }' instead" do
+ assert_dom_equal(
+ %(<input name='commit' type='submit' value='Save' data-confirm="Are you sure?" />),
+ submit_tag("Save", :confirm => "Are you sure?")
+ )
+ end
+ end
+
+ def test_submit_tag_with_confirmation_without_deprecation_warning
assert_dom_equal(
%(<input name='commit' type='submit' value='Save' data-confirm="Are you sure?" />),
- submit_tag("Save", :confirm => "Are you sure?")
+ submit_tag("Save", :data => { :confirm => "Are you sure?" })
)
end
@@ -421,6 +430,22 @@ def test_button_tag_with_button_type
)
end
+ def test_button_tag_with_confirmation
+ assert_deprecated ":confirm option is deprecated and will be removed from Rails 4.0. Use ':data => { :confirm => \'Text\' }' instead" do
+ assert_dom_equal(
+ %(<button name="button" type="submit" data-confirm="Are you sure?">Save</button>),
+ button_tag("Save", :type => "submit", :confirm => "Are you sure?")
+ )
+ end
+ end
+
+ def test_button_tag_with_confirmation_without_deprecation_warning
+ assert_dom_equal(
+ %(<button name="button" type="submit" data-confirm="Are you sure?">Save</button>),
+ button_tag("Save", :type => "submit", :data => { :confirm => "Are you sure?" })
+ )
+ end
+
def test_button_tag_with_disable_with
assert_deprecated ":disable_with option is deprecated and will be removed from Rails 4.0. Use 'data-disable-with' instead" do
assert_dom_equal(
@@ -461,9 +486,18 @@ def test_button_tag_with_block_and_options
end
def test_image_submit_tag_with_confirmation
+ assert_deprecated ":confirm option is deprecated and will be removed from Rails 4.0. Use ':data => { :confirm => \'Text\' }' instead" do
+ assert_dom_equal(
+ %(<input type="image" src="/images/save.gif" data-confirm="Are you sure?" />),
+ image_submit_tag("save.gif", :confirm => "Are you sure?")
+ )
+ end
+ end
+
+ def test_image_submit_tag_with_confirmation_without_deprecation_warning
assert_dom_equal(
%(<input type="image" src="/images/save.gif" data-confirm="Are you sure?" />),
- image_submit_tag("save.gif", :confirm => "Are you sure?")
+ image_submit_tag("save.gif", :data => { :confirm => "Are you sure?" })
)
end
View
91 actionpack/test/template/url_helper_test.rb
@@ -75,9 +75,18 @@ def test_button_to_with_query_and_no_name
end
def test_button_to_with_javascript_confirm
+ assert_deprecated ":confirm option is deprecated and will be removed from Rails 4.0. Use ':data => { :confirm => \'Text\' }' instead" do
+ assert_dom_equal(
+ "<form method=\"post\" action=\"http://www.example.com\" class=\"button_to\"><div><input data-confirm=\"Are you sure?\" type=\"submit\" value=\"Hello\" /></div></form>",
+ button_to("Hello", "http://www.example.com", :confirm => "Are you sure?")
+ )
+ end
+ end
+
+ def test_button_to_confirm_without_deprecation_warning
assert_dom_equal(
"<form method=\"post\" action=\"http://www.example.com\" class=\"button_to\"><div><input data-confirm=\"Are you sure?\" type=\"submit\" value=\"Hello\" /></div></form>",
- button_to("Hello", "http://www.example.com", :confirm => "Are you sure?")
+ button_to("Hello", "http://www.example.com", :data => { :confirm => "Are you sure?" })
)
end
@@ -95,9 +104,18 @@ def test_button_to_with_remote_and_form_options
end
def test_button_to_with_remote_and_javascript_confirm
+ assert_deprecated ":confirm option is deprecated and will be removed from Rails 4.0. Use ':data => { :confirm => \'Text\' }' instead" do
+ assert_dom_equal(
+ "<form method=\"post\" action=\"http://www.example.com\" class=\"button_to\" data-remote=\"true\"><div><input data-confirm=\"Are you sure?\" type=\"submit\" value=\"Hello\" /></div></form>",
+ button_to("Hello", "http://www.example.com", :remote => true, :confirm => "Are you sure?")
+ )
+ end
+ end
+
+ def test_button_to_with_remote_and_javascript_confirm_without_deprecation_warning
assert_dom_equal(
"<form method=\"post\" action=\"http://www.example.com\" class=\"button_to\" data-remote=\"true\"><div><input data-confirm=\"Are you sure?\" type=\"submit\" value=\"Hello\" /></div></form>",
- button_to("Hello", "http://www.example.com", :remote => true, :confirm => "Are you sure?")
+ button_to("Hello", "http://www.example.com", :remote => true, :data => { :confirm => "Are you sure?" })
)
end
@@ -110,15 +128,22 @@ def test_button_to_with_remote_and_javascript_disable_with
end
end
- def test_button_to_with_remote_and_javascript_confirm_and_javascript_disable_with
- assert_deprecated ":disable_with option is deprecated and will be removed from Rails 4.0. Use 'data-disable-with' instead" do
+ def test_button_to_with_remote_and_javascript_confirm
+ assert_deprecated ":confirm option is deprecated and will be removed from Rails 4.0. Use ':data => { :confirm => \'Text\' }' instead" do
assert_dom_equal(
- "<form method=\"post\" action=\"http://www.example.com\" class=\"button_to\" data-remote=\"true\"><div><input data-disable-with=\"Greeting...\" data-confirm=\"Are you sure?\" type=\"submit\" value=\"Hello\" /></div></form>",
- button_to("Hello", "http://www.example.com", :remote => true, :confirm => "Are you sure?", :disable_with => "Greeting...")
+ "<form method=\"post\" action=\"http://www.example.com\" class=\"button_to\" data-remote=\"true\"><div><input data-confirm=\"Are you sure?\" type=\"submit\" value=\"Hello\" /></div></form>",
+ button_to("Hello", "http://www.example.com", :remote => true, :confirm => "Are you sure?")
)
end
end
+ def test_button_to_with_remote_and_javascript_confirm_without_deprecation_warning
+ assert_dom_equal(
+ "<form method=\"post\" action=\"http://www.example.com\" class=\"button_to\" data-remote=\"true\"><div><input data-confirm=\"Are you sure?\" type=\"submit\" value=\"Hello\" /></div></form>",
+ button_to("Hello", "http://www.example.com", :remote => true, :data => { :confirm => "Are you sure?" })
+ )
+ end
+
def test_button_to_with_remote_false
assert_dom_equal(
"<form method=\"post\" action=\"http://www.example.com\" class=\"button_to\"><div><input type=\"submit\" value=\"Hello\" /></div></form>",
@@ -206,17 +231,30 @@ def test_link_tag_with_custom_onclick
end
def test_link_tag_with_javascript_confirm
+ assert_deprecated ":confirm option is deprecated and will be removed from Rails 4.0. Use ':data => { :confirm => \'Text\' }' instead" do
+ assert_dom_equal(
+ "<a href=\"http://www.example.com\" data-confirm=\"Are you sure?\">Hello</a>",
+ link_to("Hello", "http://www.example.com", :confirm => "Are you sure?")
+ )
+ end
+ assert_deprecated ":confirm option is deprecated and will be removed from Rails 4.0. Use ':data => { :confirm => \'Text\' }' instead" do
+ assert_dom_equal(
+ "<a href=\"http://www.example.com\" data-confirm=\"You can't possibly be sure, can you?\">Hello</a>",
+ link_to("Hello", "http://www.example.com", :confirm => "You can't possibly be sure, can you?")
+ )
+ end
+ assert_deprecated ":confirm option is deprecated and will be removed from Rails 4.0. Use ':data => { :confirm => \'Text\' }' instead" do
+ assert_dom_equal(
+ "<a href=\"http://www.example.com\" data-confirm=\"You can't possibly be sure,\n can you?\">Hello</a>",
+ link_to("Hello", "http://www.example.com", :confirm => "You can't possibly be sure,\n can you?")
+ )
+ end
+ end
+
+ def test_link_tag_confirm_without_deprecation_warning
assert_dom_equal(
"<a href=\"http://www.example.com\" data-confirm=\"Are you sure?\">Hello</a>",
- link_to("Hello", "http://www.example.com", :confirm => "Are you sure?")
- )
- assert_dom_equal(
- "<a href=\"http://www.example.com\" data-confirm=\"You can't possibly be sure, can you?\">Hello</a>",
- link_to("Hello", "http://www.example.com", :confirm => "You can't possibly be sure, can you?")
- )
- assert_dom_equal(
- "<a href=\"http://www.example.com\" data-confirm=\"You can't possibly be sure,\n can you?\">Hello</a>",
- link_to("Hello", "http://www.example.com", :confirm => "You can't possibly be sure,\n can you?")
+ link_to("Hello", "http://www.example.com", :data => { :confirm => "Are you sure?" })
)
end
@@ -263,16 +301,35 @@ def test_link_tag_using_post_javascript_and_rel
end
def test_link_tag_using_post_javascript_and_confirm
+ assert_deprecated ":confirm option is deprecated and will be removed from Rails 4.0. Use ':data => { :confirm => \'Text\' }' instead" do
+ assert_dom_equal(
+ "<a href=\"http://www.example.com\" data-method=\"post\" rel=\"nofollow\" data-confirm=\"Are you serious?\">Hello</a>",
+ link_to("Hello", "http://www.example.com", :method => :post, :confirm => "Are you serious?")
+ )
+ end
+ end
+
+ def test_link_tag_using_post_javascript_and_confirm_without_deprecation_warning
assert_dom_equal(
"<a href=\"http://www.example.com\" data-method=\"post\" rel=\"nofollow\" data-confirm=\"Are you serious?\">Hello</a>",
- link_to("Hello", "http://www.example.com", :method => :post, :confirm => "Are you serious?")
+ link_to("Hello", "http://www.example.com", :method => :post, :data => { :confirm => "Are you serious?" })
)
end
def test_link_tag_using_delete_javascript_and_href_and_confirm
+ assert_deprecated ":confirm option is deprecated and will be removed from Rails 4.0. Use ':data => { :confirm => \'Text\' }' instead" do
+ assert_dom_equal(
+ "<a href='\#' rel=\"nofollow\" data-confirm=\"Are you serious?\" data-method=\"delete\">Destroy</a>",
+ link_to("Destroy", "http://www.example.com", :method => :delete, :href => '#', :confirm => "Are you serious?"),
+ "When specifying url, form should be generated with it, but not this.href"
+ )
+ end
+ end
+
+ def test_link_tag_using_delete_javascript_and_href_and_confirm_without_deprecation_warning
assert_dom_equal(
"<a href='\#' rel=\"nofollow\" data-confirm=\"Are you serious?\" data-method=\"delete\">Destroy</a>",
- link_to("Destroy", "http://www.example.com", :method => :delete, :href => '#', :confirm => "Are you serious?"),
+ link_to("Destroy", "http://www.example.com", :method => :delete, :href => '#', :data => { :confirm => "Are you serious?" }),
"When specifying url, form should be generated with it, but not this.href"
)
end
View
2  railties/lib/rails/generators/erb/scaffold/templates/index.html.erb
@@ -17,7 +17,7 @@
<% end -%>
<td><%%= link_to 'Show', <%= singular_table_name %> %></td>
<td><%%= link_to 'Edit', edit_<%= singular_table_name %>_path(<%= singular_table_name %>) %></td>
- <td><%%= link_to 'Destroy', <%= singular_table_name %>, <%= key_value :confirm, "'Are you sure?'" %>, <%= key_value :method, ":delete" %> %></td>
+ <td><%%= link_to 'Destroy', <%= singular_table_name %>, <%= key_value :method, ":delete" %>, <%= key_value :data, "{ #{key_value :confirm, "'Are you sure?'"} }" %> %></td>
</tr>
<%% end %>
</table>
Something went wrong with that request. Please try again.