Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Add class and id filters for use in twig templates.
Proposed resolution
Add twig filters called (names tbd) 'clean_class' and 'clean_id' that run the text through drupal_html_class() and drupal_clean_id_identifier(), respectively.
After filters added:
<div id="{{ attributes.id|clean_id }}" class="oembed oembed-video oembed-provider-{{ provider|clean_class }}">
Remaining tasks
Agree on the filter names?...
User interface changes
N/A
API changes
Two new filters available.
Comment | File | Size | Author |
---|---|---|---|
#22 | add_twig_filters_for-2283301-22.patch | 3.17 KB | davidhernandez |
#14 | interdiff.txt | 2.68 KB | star-szr |
#14 | 2283301-14.patch | 2.44 KB | star-szr |
#11 | 2283301-twig-class-id-filters.patch | 2.06 KB | Dave Reid |
#7 | interdiff.txt | 1.87 KB | joelpittet |
Comments
Comment #1
Dave ReidComment #2
Dave ReidComment #3
Dave ReidComment #4
star-szrYep for sure, I even used this as an example in the Twig playground in Austin.
Add a period here ;)
Then I can RTBC!
Comment #5
Dave ReidOne extra period added!
Comment #6
star-szrI like it! Thanks @Dave Reid :)
Adding another related issue.
Comment #7
joelpittetJust adding another test to make sure we don't accidentally break BEM syntax as we have in D7.
Comment #8
catchLet's remove the one for drupal_html_id() - that needs to go: #1305882: drupal_html_id() considered harmful; remove ajax_html_ids to use GET (not POST) AJAX requests.
Comment #9
joelpittet@Dave Reid and @Cottser, from a sober second thought, I'd like to propose a name change as
|class
is very ambiguous.Any chance 'sanitize_class' would fly as a name change?
Also, re #8 @Catch, et al would
drupal_clean_id_identifier
work for a 'sanitize_id' filter?FWIW, I'm also fine with 'clean_class' and 'clean_id', though I can think of a few cases that those
|class
and|id
filters could be used to do different things...Comment #10
mortendk CreditAttribution: mortendk commentedWe opened up a sandbox for a poc for the banana https://www.drupal.org/sandbox/cottser/2297653
Comment #11
Dave ReidOk, given the debate about drupal_html_id(), it makes sense to just have the class filter for now since it definitely will be useful for people.
Comment #12
joelpittet@Dave Reid any thoughts on #9?
Comment #13
Fabianx CreditAttribution: Fabianx commentedI agree with #12, lets use clean_id and clean_class and use drupal_clean_id_identifier and drupal_html_class to make those work.
Comment #14
star-szrHere's #13. I considered taking out the second ID from the test but I think it's useful that we are differentiating between using drupal_clean_id_identifier() and drupal_html_id(), so I tweaked the text used in the test a bit to explain it for people looking at this code later.
Comment #15
star-szrInterdiff is from #7.
Comment #16
Fabianx CreditAttribution: Fabianx commented**RTBC**
Comment #17
alexpottIt might be nice to test that
drupal_clean_id_identifier
is call correctly... with the id being "quotes" it is not actaully cleaning anything. I like the fact we're testing it is not usingdrupal_html_id()
.Also the issue summary could do with an update to accord with the latest patch.
Comment #18
jwilson3Huge +1 here. One comment though about the name of the filter...
This will drastically improve implementation and syntax as CSS classes are moved out of preprocess functions, such as the work going on here: #2217731: Move field classes out of preprocess and into templates.
The template code in that example is already very long:
If we were to switch to using this method, it would be nice to be able to shorten the code using a filter called
c
instead ofclean_css
.t
filter that themers will have to learn.t
function in templates (so we have precedence).c
means will be a no brainer.Comment #19
joelpittet@jwilson3 there is a historical presidence for the t filter and t() from drupal, and the |e and |escape from twig. I'm a bit leery at |c being for |clean_class. I understand it would have a wide use, but still worries me. We can maybe open up an issue for 8.1.x shorteners and punt that to a contrib twig extension so we can have some time to deliberate on that?
Comment #20
star-szrSingle letter things are often hard to look up.
Comment #21
Crell CreditAttribution: Crell commentedReadabilty trumps typing. We can always add aliases later if needs be, so don't let that block here. clean_class is fine.
Comment #22
davidhernandezAdded id text to be filtered, and updated some text. Updated summary.
Are we agreeing on clean_class and clean_id?
Comment #23
davidhernandezComment #24
RainbowArrayclean_class and clean_id make good sense. Patch addresses concerns raised in #17. Back to RTBC.
Comment #26
alexpottCommitted 3f3fc8d and pushed to 8.0.x. Thanks!