tag_helper :data => {:should_not => "dasherize"} #1914

Closed
Bodacious opened this Issue Jun 30, 2011 · 3 comments

Projects

None yet

4 participants

Contributor

Not sure what other's views are but for me it's a little confusing that the new tag_helper :data => {} hash dasherizes keys.

I was writing some JS today and this threw me as I had used

{:data => {:user_id => 1}}

and was looking for data-user_id in my JS

I propose getting rid of dasherize in ActionView::Helpers::TagHelper#tag_options

this way:

:data => {:user_id => 1} # => "data-user_id"="1" (and we can use $(this).data("user_id") in jQuery)

plus we can still have

:data => {"user-id" => 1) # => "data-user-id"="1"

if people are really hot for dasherized attributes

Happy to submit this change myself if enough people agree

cabgfx commented Jul 5, 2011

I'm split on this.

The current behavior is intended to "play nicely with Javascript conventions" - and it seems like the right way to go, even if it explicitly prevents you from using (perfectly valid) underscored data attributes.

Dasherizing HTML5 data-* attributes seems to be the general convention, and will give you some instant benefits in browsers that implement the JS API for dataset Examples.

Contributor

Vote to close issue/keep it as is.

Bodacious: I'm with you in principle, many JavaScript projects already have their own conventions, melding Rails conventions with them just makes for confusion. If it were October 18th, 2010, and I were next to Michael Koziarski when he wrote that code, I would have said "nah, don't do it, where a strong convention (rails) and weak/diverse convention (javascript) meld, one should yield to the diverse one".

However, that was a while ago, and a lot of apps probably depend on that behavior now and it would probably make a bunch of Rails apps break. That would be shabby. At least the dasherize behavior is well-documented.

    # git blame actionpack/lib/action_view/helpers/tag_helper.rb
    ...
    2f9e8804 134 (Michael Koziarski            2010-10-18 11:17:13 +1300 141)                   attrs << %(data-#{k.to_s.dasherize}="#{v}")
    ...
    # git describe 2f9e8804
    v2.2.0-10871-g2f9e880
Contributor

Oh -

I thought the :data => attribute was a 3.1 addition?

https://gist.github.com/958283 (ActionPack 3.1.rc4)

But if not, and this would break past apps - I agree it's best not to change

On 15 Jul 2011, at 06:19, joegoggins wrote:

Keep it as is.

Bodacious: I'm with you in principle, many JavaScript projects already have their own conventions, melding Rails conventions with them just makes for confusion. If it were October 18th, 2010, and I were next to Michael Koziarski, I would have said "nah, don't do it, where a strong convention (rails) and weak/diverse convention (javascript) meld, one should yield to the diverse one", however, that came in in v2.2.0, and it would probably make a bunch of Rails apps break. That would be shabby. At least the dasherize behavior is well-documented.

   # git blame actionpack/lib/action_view/helpers/tag_helper.rb
   ...
   2f9e8804 134 (Michael Koziarski            2010-10-18 11:17:13 +1300 141)                   attrs << %(data-#{k.to_s.dasherize}="#{v}")
   ...
   # git describe 2f9e8804
   v2.2.0-10871-g2f9e880

Reply to this email directly or view it on GitHub:
#1914 (comment)

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