Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 UTC on 18 March 2024, to get $100 off your ticket.
(First, verify that the preprocess changes have been made. #2322163: [meta] Consensus Banana Phase 1, move CSS classes from preprocess to twig templates.)
- Copy the Twig templates from the core module's templates directory to Classy's templates directory. Include all templates, even ones without classes.
- Remove all classes from the core module's template. Remove all classes added with addClass and ones that are hard-coded in the template.
- If there are classes that are required for basic functionality, discuss whether they should be kept.
- If there is CSS from the module, or anywhere else, referring to the class, discuss removing it or moving it to Bartik&Seven. Do not move the CSS to Classy.
Twig Templates to Copy
core/modules/filter/templates/filter-caption.html.twig
core/modules/filter/templates/filter-guidelines.html.twig
core/modules/filter/templates/filter-tips.html.twig
core/modules/filter/templates/text-format-wrapper.html.twig
Comment | File | Size | Author |
---|---|---|---|
#29 | interdiff-2349677.txt | 894 bytes | davidhernandez |
#29 | copy_filter_templates-2349677-29.patch | 11.95 KB | davidhernandez |
#27 | copy_filter_templates-2349677-27.patch | 11.08 KB | davidhernandez |
#20 | interdiff.txt | 11.62 KB | Xen |
#20 | copy_filter_templates-2349677-20.patch | 11.08 KB | Xen |
Comments
Comment #1
emma.mariaComment #2
emma.mariaI have copied the templates over to classy and removed the classes from the original module templates. There is CSS left in the module that reference the removed classes so a follow up is needed - tagged cssbanana.
Comment #3
derheap CreditAttribution: derheap commentedComment #5
derheap CreditAttribution: derheap commentedWe should remove all classes, but attributes should still be printed.
Otherwise RDF and other things can break.
{{attributs}} should still be printed.
{{tip.attributes}} should still be printed.
{{item.attributes}} should still be printed.
{{attributes}} should still be printed.
Comment #6
mortendk CreditAttribution: mortendk commentedComment #7
runand CreditAttribution: runand commentedComment #8
runand CreditAttribution: runand commentedComment #10
davidhernandezPlease double-check if any removed classes are being used in javascript. It is best to test the affected template using Stark to make sure nothing is broken.
Comment #13
Xen CreditAttribution: Xen commentedComment #14
Xen CreditAttribution: Xen commentedChecked the JS files using Stark:
filter.filter_html.admin.js
- Checked that the "Allowed HTML tags" text field got updated when dragging the sup button into the CKEditor toolbar, and that the description explained that the tag was added.
filter.admin.js
- Checked that the settings for a configurable filter was properly shown and hidden when toggling the checkbox.
filter.js
- Checked that the format description changed when changing text format on a new article.
Everything worked the same on Bartik and Stark.
The failures of the testbot is differences in classes on some elements, but I believe that the bot should now test using Classy as a theme, so I'm requeing.
Comment #15
Xen CreditAttribution: Xen commentedComment #18
davidhernandezThe test failures on the first group are unit tests that are checking for exact markup. We can probably just remove the classes from the test, as they don't appear to be relevant to the individual tests.
The main issue is those align-left, align-right, align-center classes might be considered functional, and it is exactly what those tests are checking for. Is it odd that it is a unit test and not a web test (which is why it is failing)?
If those classes are considered functional, we shouldn't remove them. Opinions?
Comment #19
Xen CreditAttribution: Xen commentedGrepping for the classes finds them in system.module.css.
I'd call them functional.
It does make some kind of sense that it's unit tests. It's testing the filtering function, and as little as possible else. That there's a bit of templating involved is a minor detail.
Comment #20
Xen CreditAttribution: Xen at Reload commentedRemoved the classes from the test, apart from the align classes.
Re-introduced adding classes to the module filter-caption.html.twig, so the align classes will get properly added.
The interdiff seems a bit odd, I think git is to clever for it.
Comment #22
mortendk CreditAttribution: mortendk commentedThe css needs to have classnames renamed so it follow the oocss doc standards.- that work can first be done when everything is moved over and should be a followup issue.
filter.caption.css contains elements that are essential for the functionality, and should be trated as such.
Comment #26
davidhernandezComment #27
davidhernandezIn filter-guidelines 'format.name' changed to 'format.label'. That was the only change.
<h4 class="label">{{ format.label }}</h4>
Comment #29
davidhernandezHad to remove the class from tests.
Comment #30
mortendk CreditAttribution: mortendk commentedok good from here
Comment #31
webchickCommitted and pushed to 8.0.x. Thanks!