Make turbo_frame_tag fully compatible with dom_id #476
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, there was an inconsistency with how turbo_frame_tag and
dom_id would create ids:
```
turbo_frame_tag(Article.find(1), "comments") # => <turbo-frame id="article_1_comments" />
dom_id(Article.find(1), "comments") # => "comments_article_1"
```
This commit fixes that incompatibility by fully delegating id generation
to dom_id when given a model object.
```
turbo_frame_tag(Article.find(1), "comments") # => <turbo-frame id="comments_article_1" />
dom_id(Article.find(1), "comments") # => "comments_article_1"
turbo_frame_tag("comments") # => <turbo-frame id="comments" />
```
This does break the use case of passing multiple models, but that seems
like it would be better fixed in dom_id itself.
mamhoff
added a commit
to friendlycart/solidus_friendly_promotions
that referenced
this pull request
Oct 12, 2023
The behaviour of the turbo_frame_tag helper changed in Turbo 1.5 and now does not support passing multiple models. These changes account for the change in output and behaviour. See hotwired/turbo-rails#476
seanpdoyle
added a commit
to seanpdoyle/turbo-rails
that referenced
this pull request
Oct 16, 2023
Closes [hotwired#503][] When the positional arguments to `turbo_frame_tag` respond are compatible with `dom_id` (they respond to `#to_key`), pass them to `dom_id`, otherwise, check them individually. This approach melds the behavior introduced in [hotwired#476][] with the original support. [hotwired#503]: hotwired#503 [hotwired#476]: hotwired#476
|
I agree with this change, but upgrading broke a lot of Turbo Frames. Don't you think we should announce it with more emphasis? Is it an option to support both ways? |
jwilsjustin
pushed a commit
to jwilsjustin/rails
that referenced
this pull request
Jan 5, 2024
ref: https://discuss.rubyonrails.org/t/allow-dom-id-method-to-accept-multiple-ids-models/84408 If the first argument (records_or_classes) is an array, this change will loop through each value and form a singular dom id, then join the parts together. This change will allow turbo frame tags to accept multiple models (similar to the `cache` helper from ActionView). ``` <%= turbo_frame_tag [customer, field] do %> <!-- code --> <% end %> ``` See also: hotwired/turbo-rails#476
4 tasks
jwilsjustin
pushed a commit
to jwilsjustin/rails
that referenced
this pull request
Jan 8, 2024
ref: https://discuss.rubyonrails.org/t/allow-dom-id-method-to-accept-multiple-ids-models/84408 If the first argument (records_or_classes) is an array, this change will loop through each value and form a singular dom id, then join the parts together. This change will allow turbo frame tags to accept multiple models (similar to the `cache` helper from ActionView). ``` <%= turbo_frame_tag [customer, field] do %> <!-- code --> <% end %> ``` See also: hotwired/turbo-rails#476
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ref #449
Previously, there was an inconsistency with how turbo_frame_tag and dom_id would create ids:
This commit fixes that incompatibility by fully delegating id generation to dom_id when given a model object.
This does break the use case of passing multiple models, but that seems like it would be better fixed in dom_id itself.