activeadmin / arbre

An Object Oriented DOM Tree in Ruby
MIT License
762 stars 74 forks source link

hyphenated tag attributes are poorly supported, making stimulus integration painful #450

Closed Ikariusrb closed 1 year ago

Ikariusrb commented 1 year ago

I'm suprised as they seem to work correctly in ActiveAdmin forms, but in Arbre, hyphenated tag attributes are difficult to emit.

In an ActiveAdmin form, I can do the following: f.input(:trade, input_html: { data: { action: 'click->trade#foo' } }) The expected hyphenated data-action="click->trade#foo' attribute will come out in the HTML, which works seamlessly with stimulus.

If I'm rendering an Arbre partial template, the same does not yield the desired results:

In order to get hyphenated attributes, I ended up doing this: div(class: 'myclass', **{ 'data-controller'.to_sym => 'worker-cred' }) do Passing a straight hash as the second parameter works for some tags, but didn't work for a div with a block. I didn't dig into the why, but using to_sym and splatting the hash eventually got me the desired result.

That code ends up appropriately rendering a data-controller attribute for the div (and working with stimulus), but it's definitely not intuitive to anyone who's not pretty deep in ruby magic.

If instead I put the following in the Arbre template: div(class: 'myclass', data: { controller: 'worker-cred' }) do

I end up with html like this: <div class="myclass" data="{:controller=>&quot;worker-cred&quot;}">

I believe the code is in lib/arbre/html/attributes.rb in the to_s method . What are the chances of updating this code to allow the form that works in .erb and ActiveAdmin forms, in order to better integrate with Stimulus?

It looks like the ActionView code for rendering tag attributes can be found here: /home/rbecker/.rbenv/versions/3.1.1/lib/ruby/gems/3.1.0/gems/actionview-7.0.2.4/lib/action_view/helpers/tag_helper.rb

javierjulio commented 1 year ago

Duplicate of the previous issue. I haven't dealt much with Arbre but as a maintainer I'd be open to accepting a PR for resolving this. This should be supported.

Ikariusrb commented 1 year ago

Duplicate of the previous issue. I haven't dealt much with Arbre but as a maintainer I'd be open to accepting a PR for resolving this. This should be supported.

Actually separate issue, as the prior issue is about tag names with hyphens, and this is about tag attributes with hyphens. The code for attributes and tag names are separate, so I opened a separate issue. I'll see if there's something reasonable I can do to put together a PR that would support hyphenated attributes.

javierjulio commented 1 year ago

Perhaps changes will be similar there as well. If you are able to get an update with tests, I'm happy to make the time to review and get that in. I agree, this is something that should be supported. Thank you.

Ikariusrb commented 1 year ago

@javierjulio - here's a first cut: https://github.com/activeadmin/arbre/pull/451

javierjulio commented 1 year ago

@Ikariusrb looks great, thanks! I've asked another maintainer (David) to review.