Following Turbo action conventions
Hi, we're loving this gem. It's got a lot of really great abilities in here.
I spent a little while debugging an issue when I was using one of the actions today. I had a turbo stream template which was rendering several turbo stream actions
= turbo_stream.append dom_id(@model, "list"), ...
= turbo_stream.replace dom_id(@model, "map_pin"), ...
-# other turbo streams here
And we wanted to add a new stream for changing an attribute. Turbo power has the set_attribute action for this!
= turbo_stream.set_attribute dom_id(@model, "map"), ...
This failed however, and you might be able to guess why. Almost all of the Turbo Power actions use the custom_action_all helper, which sends builds turbo stream tags using the targets attribute. When you use the targets attribute then the Turbo JS switches to using querySelectorAll instead of findElementByID to locate your target. That took us a bit of time but we eventually figured out we needed to prefix our dom_id with a # in order to get everything working.
= turbo_stream.set_attribute "##{dom_id(@model, "map")}", ...
This is a little bit uglier, but it's workable.
Turbo has started to establish a convention with the stream helpers where calling an action sets the target attribute, and the *_all helpers sets the targets attributes (e.g. append vs append_all).
I think we could add the same thing in Turbo Power where it makes sense. That would involve making many additional helpers and having set_attribute_all in addition to set_attribute. I saw your rejected PR which would have made this quite a bit easier as we could have passed target/targets ourselves, as needed. In lieu of that, however, adding all versions of these actions might be good for consistency. Would you accept a PR which added these or do you have any other ideas of how to achieve this consistency a little better? The biggest downside I can see besides doubling the API surface is it would be a breaking change for the existing API. Let me know what your thoughts are, please!
Hey @willtcarey, thanks for opening this issue!
I agree that it currently doesn't feel 100% right, but I'm wondering if we can maybe change this upstream for the upcoming Turbo 8 release.
I still feel like the API with keyword arguments feels so much nicer and more intuitive compared to what we have right now. I'm going to propose something on turbo-rails, before adding more to Turbo Power now. How does that sounds to you?
@marcoroth That sounds great to me. I think I would enjoy the keyword API much better overall. Let me know if you need anything from me to open that issue or if I can help in any way.