Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Problem/Motivation
Create hasClass() methods on Attribute object for testing if class exists.
It would be useful to be able to do functionality based on applied classes e.g replace classes in Twig template.
Proposed resolution
Add methods to the Attribute object for testing if class exists.
@mortendk proposed method name hasClass which would return boolean based on existance of class.
Example syntax within Twig
{% if attributes.hasClass('foo') %}
{{ attributes.addClass('bar') }}
{% endif %}
{% if attributes.hasClass('field-label-inline') %}
...print markup like this.
{% else %}
...print it like this.
{% endif %}
Remaining tasks
Add a change record - Novice task
User interface changes
n/a
API changes
API addition to test if class exists on Attribute object.
Comment | File | Size | Author |
---|---|---|---|
#26 | 2347511-26.patch | 1.83 KB | rpayanm |
#26 | 2347511-interdiff.txt | 805 bytes | rpayanm |
#21 | interdiff.txt | 1.66 KB | davidhernandez |
#21 | add_method_to_test_if-2347511-21.patch | 1.76 KB | davidhernandez |
#10 | add_method_to_test_if-2347511-10.patch | 1.64 KB | davidhernandez |
Comments
Comment #1
jjcarrionComment #2
kasnadd hasClass Method as proposed
Comment #3
lauriiiTest patch to prove tests are working
Comment #5
star-szrIn the example in the summary I don't understand the use case, I don't see the harm in removing the
if
in that case. Can we get more information in the issue summary please?Comment #6
davidhernandezThe use case is largely the same as the Jquery one. http://api.jquery.com/hasclass/ . It is just a bit simpler for themers to check for a class than check for some more complicated things.
Comment #7
lauriiiComment #8
davidhernandezThis permission change shouldn't be in the patch, correct?
Comment #9
lauriiifixed
Comment #10
davidhernandezI moved hasClass down to keep addClass and removeClass together. Updated all the comments. Removed the extra class name from the test. It wasn't necessary.
Comment #12
drifter CreditAttribution: drifter commentedTested this manually in a twig template (both having a class and not having a class), works well. The method and test is straightforward, parameters are documented, looks good to me.
Comment #13
davidhernandezBeing an API addition, it probably could use a change record.
Comment #14
davidhernandezJust adding the novice tag to indicate the change record is a novice task.
Comment #15
mirom CreditAttribution: mirom commentedComment #16
lauriiiPutting back to Needs Review for the Change record
Comment #17
mirom CreditAttribution: mirom commentedHere is change record
Comment #18
lauriiiThe change record looks good! Lets go with this!
Comment #19
lauriiiComment #20
alexpottThis fails and it shouldn't.
Comment #21
davidhernandezFixing #20
Comment #22
cilefen CreditAttribution: cilefen commented"The" CSS class...
Else should be on a new line.
Comment #23
cilefen CreditAttribution: cilefen commentedThe @return is missing its comment.
Comment #24
sarav.din33 CreditAttribution: sarav.din33 commentedHi davidhernandez,
Patch #20 works fine.
Test case:
In my bartik theme's node.html.twig file, i added the below snippet,
Which means check whether the class named as 'node' exist in the current content or not. Because i wrote the code in node.html.twig so we can test it in content page only. If 'node' class is exists i just print the "custom-node-container" div in it. Likewise for 'contextual-region' class.
Once you did the changes in twig files you need to clear the cache then only it will reflect in your site. After clear the cache, i can able to see the those two div containters in the page.
Comment #25
cilefen CreditAttribution: cilefen commentedMarking needs work for the code style issues.
Comment #26
rpayanmFixed code style issues.
Comment #27
cilefen CreditAttribution: cilefen commentedBased on a manual test verifying this works, and that the code style looks correct, this is RTBC.
Comment #28
cilefen CreditAttribution: cilefen commented@sarav.din33 It looks as though you did not test this method.
Comment #29
alexpottThis issue is a prioritized change (the method added here helps write cleaner templates) as per #2350615: [policy, no patch] What changes can be accepted during the Drupal 8 beta phase? and it's benefits outweigh any disruption. Committed 1d8f7c5 and pushed to 8.0.x. Thanks!
Comment #32
davidhernandezComment #33
sarav.din33 CreditAttribution: sarav.din33 commented@cilefen: Noted will make sure i do a code check along with the review.