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

Merged
merged 1 commit into from Jun 5, 2012

Projects

None yet

5 participants

Contributor

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

/cc @rafaelfranca

Related to #6614

Owner

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

@rafaelfranca rafaelfranca was assigned Jun 3, 2012
Contributor

Updated

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 Jun 5, 2012
b4syth commented on fc092a9 Jun 18, 2012

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 commented Jun 20, 2012

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?

Owner

@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 commented Jun 20, 2012

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.

Owner

@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 Jul 19, 2013
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