Move classes out of the preprocess functions and into the Twig templates. Use the addClass() attribute method to add classes in the template. Use the clean_class filter to filter class names, if necessary. Maintain all existing functionality and ensure all existing class names are still in the markup, even ones that are inherited.
See the following issues for more detailed examples:
#2217731: Move field classes out of preprocess and into templates
#2254153: Move node classes out of preprocess and into templates
See this change record for information about using the addClass() method:
https://www.drupal.org/node/2315471
See this change record for more information about the phase 1 process of moving class from preprocess to templates:
https://www.drupal.org/node/2325067
Preprocess Functions Modified
template_preprocess_image
template_preprocess_image_style
Twig Templates Modified
image.html.twig
image-style.html.twig
Comment | File | Size | Author |
---|---|---|---|
#25 | move_image_classes_from-2329771-25.patch | 7.31 KB | lauriii |
#25 | interdiff-2329771-22-25.txt | 913 bytes | lauriii |
Comments
Comment #1
davidhernandezComment #2
pakmanlhWorkin on it
Comment #3
pakmanlhI made a first approach, I don't know if is a correct way, but I thought it was suitable rewrite template_preprocess_image_style function to make it work like template_preprocess_image and it use the attributes variable instead of image variable.
Also I don't see anything to change on template_preprocess_image function.
Comment #5
star-szrI don't think that's going to work, the output is no longer going through #theme image. Scale back the changes to only moving classes from preprocess to templates please :)
Edit: yeah, crossposted with testbot.
Comment #6
lauriiiMoving only the class is bit of a problem because image is being rendered before passing it to template
Comment #7
davidhernandezYeah, template_preprocess_image is in theme.inc, but image module creates template_preprocess_image_style, which adds the classes and other attributes. It prints them in image.html.twig. image-style.html.twig doesn't seem to do anything. We would need the 'style_name' variable to get to the image.html.twig template.
Comment #8
davidhernandezThe style_name variable is at the image-style template. The image attribute is there as an array, not an attribute object, so I don't think it makes sense to try to add it there?
Comment #9
davidhernandezOk, I did this by adding another variable to image. Not sure if it is the best approach. I did test it with a custom image style, and it seems fine.
Comment #11
davidhernandezThe failures just look to be because the attribute order changed. I'll try to fix it, but I'm having trouble getting my local testing working right now...
Comment #12
star-szrMakes sense!
This should be marked as optional, because it won't always be there.
Comment #13
davidhernandezIs there a particular convention for listing something as optional in a comment?
Comment #14
star-szr- style_name: (optional) The name of the image style applied.
Ref. https://www.drupal.org/node/1354#lists
Comment #15
davidhernandezFixed the comment and the broken tests. (Order of the attributes changed.)
Comment #16
star-szrNeeds a reroll, looks like a patch context conflict with #2333395: Add sizes to template_preprocess_image.
Comment #17
davidhernandezreroll
Comment #18
davidhernandezComment #20
davidhernandezOops, when I merged I chopped out the variable added in the change from the other issue.
Comment #22
lauriiiRerolled the patch
Comment #23
star-szrFor actually making use of this variable in the template, it's probably going to make more sense to not run this through drupal_html_class beforehand so that it matches up with the image style machine name. Can we just use |clean_class in the template instead when building the class?
Comment #24
davidhernandezYeah I thought about using clean_class instead but then must have gotten lazy. I agree with that change.
Comment #25
lauriiiNow we're using clean_class :)
Comment #26
star-szrLooks good and manual testing checks out (and it has quite a bit of test coverage as well as we can see). RTBC.
Comment #27
alexpottCommitted 2bccde8 and pushed to 8.0.x. Thanks!