Add support for conditional values to TagBuilder#37872
Merged
tenderlove merged 3 commits intorails:masterfrom Dec 4, 2019
joelhawksley:content-tag-hash-class-conditional
Merged
Add support for conditional values to TagBuilder#37872tenderlove merged 3 commits intorails:masterfrom joelhawksley:content-tag-hash-class-conditional
tenderlove merged 3 commits intorails:masterfrom
joelhawksley:content-tag-hash-class-conditional
Conversation
Adds support for conditional values to TagBuilder, extracting logic we use in the GitHub application, inspired by https://github.com/JedWatson/classnames. It’s common practice to conditionally apply CSS classes in Rails views. This can lead to messy string interpolation, often using ternaries: ```ruby content_tag( "My username", class: "always #{'sometimes' if current_user.special?} another" ) ``` By adding support for hashes to TagBuilder, we can instead write the following: ```ruby content_tag( "My username", class: ["always", "another", { 'sometimes' => current_user.special? }] ) ``` cc @JedWatson
tenderlove
reviewed
Dec 4, 2019
Member
tenderlove
left a comment
There was a problem hiding this comment.
I think this makes a lot of sense for CSS classes. Should it be applied to all options though?
Contributor
Author
|
@tenderlove this probably doesn't make much sense for values to arguments besides |
Edouard-chin
added a commit
to Edouard-chin/rails
that referenced
this pull request
Dec 9, 2019
- rails#37872 introduced a regression and you can't do ```html.erb hidden_field_tag('token', value: [1, 2, 3]) ``` This will result in a `<input type="hidden" value=""`>. I chose `hidden_field_tag` and the `value` attribute as an example but this issue applies to any tag helper and any attributes. rails#37872 (comment) mention that the feature should only apply for "class" attribute. This commit fix original intent of rails#37872
For honest comparison with the new version, one could use arrays (that were already supported) + pure Ruby content_tag(
"My username",
class: ["always", "another", ("sometimes" if current_user.special?)]
)vs content_tag(
"My username",
class: ["always", "another", { 'sometimes' => current_user.special? }]
)Probably, the hash notation was inspired by JS Classnames library. Because in JS something like [1, 2, (if (true) { 3 })]is not only ugly, but also an invalid syntax. |
Contributor
|
@nashbridges I guess this API has parity with #37918 in mind. |
Contributor
Author
|
@nashbridges - @kenn is correct. This PR was meant to enable #37918. |
|
@kenn @joelhawksley I see, thanks guys! On our project we use slim, so the .wrapper class=("active" if item.for_sale?)but yeah, I can see that for ERB users the result is much more cleaner. |
inopinatus
added a commit
to inopinatus/rails
that referenced
this pull request
Feb 19, 2020
Taking the ideas introduced in rails#37872 and rails#37918, eliminating the use of a switch-on-class, and generalising the production of tags to any context. Passes an accumulator/result array to minimise allocations.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
This PR adds support for conditional values to TagBuilder,
extracting logic we use in the GitHub application,
inspired by https://github.com/JedWatson/classnames.
It’s common practice to conditionally apply CSS classes
in Rails views. This can lead to messy string interpolation,
often using ternaries:
By adding support for hashes to TagBuilder, we can instead write the following:
cc @JedWatson