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
Remove non essential classes from core - the classes are in classy, cores templates should be as clean as possible.
js required classes should be prefixed to js- according to code standards
Proposed resolution
See parent issue(s) and related issue(s) for details,
Twig Templates to modify
- core/modules/system/templates/select.html.twig
- core/modules/system/templates/status-messages.html.twig
core/modules/system/templates/status-report.html.twigcore/modules/system/templates/system-admin-index.html.twigcore/modules/system/templates/system-config-form.html.twigcore/modules/system/templates/system-themes-page.html.twig
Do not modify templates with a strike through their name. Those were not copied to Classy.
Remaining tasks
User interface changes
API changes
Original report by @username
Comment | File | Size | Author |
---|---|---|---|
#21 | status-bartik.png | 61.96 KB | rteijeiro |
#21 | status-stark.png | 59.98 KB | rteijeiro |
#21 | status-stark-before.png | 85.43 KB | rteijeiro |
#19 | clean-s-interdiff.txt | 1.13 KB | mortendk |
#19 | clean-template-s-2.patch | 2.05 KB | mortendk |
Comments
Comment #1
Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae commentedComment #2
davidhernandezPostponing this until we decide how we are moving forward. Keep an eye on #2348543: [meta] Consensus Banana Phase 2, transition templates to the starterkit theme for updates. Thanks.
Comment #3
davidhernandezUn-postponing. The templates have been copied to Classy, so all we need to do is remove classes from the original templates.
Comment #4
mortendk CreditAttribution: mortendk commentedComment #6
davidhernandezMessageTest needs to specify Classy.
Comment #7
mortendk CreditAttribution: mortendk commentedComment #8
mortendk CreditAttribution: mortendk commentedComment #9
danquah CreditAttribution: danquah commentedComment #10
danquah CreditAttribution: danquah commentedDid one minor change so that we now pull out the class attribute explicitly and render it, instead of the entire Attributes object.
Should we want to actually allow the entire Attributes object to be rendered we should make sure our own attributes does not collide (eg. "role").
Screenshot of the resulting markup attached, notice that the current template_preprocess_status_messages() does not add a "attributes" - so if someone wants to add classes they need to implement their own preprocess.
Comment #11
danquah CreditAttribution: danquah commentedComment #14
joelpittetThanks it's looking pretty good, just a couple of nitpicks:
Got an extra tab or space here by accdient.
That's kinda a weird place for this anyways, +1 for removing that.
No need for this extra line break.
Comment #15
mortendk CreditAttribution: mortendk commentedfixed the spaces
Comment #16
mortendk CreditAttribution: mortendk commentedups breadcrump template sneaked into #15
Comment #17
mortendk CreditAttribution: mortendk commentedya gotte be kidding me ... right patch this time the S templates
Comment #19
mortendk CreditAttribution: mortendk commented... goes make coffe interdiff added & patch cleaned up
Comment #21
rteijeiro CreditAttribution: rteijeiro commentedI think it still needs work. In Stark it status messages seem to lose the styling. In Bartik is correct. Check screenshots:
Stark BEFORE
STARK AFTER
BARTIK
Comment #22
mortendk CreditAttribution: mortendk commentedit works as it should - its by design that classes are removed (see the banana #2289511: [meta] Results of Drupalcon Austin's Consensus Banana & https://www.drupal.org/node/2348543 )
Classes should not be there in stark unless they are needed for the module to work - a color is not required to work ;)
themers that goes with stark as basetheme, knows what they are doing and will have to provide classes that they need.
Comment #23
rteijeiro CreditAttribution: rteijeiro commentedOh, thanks! That's all I need to RTBC this!
Comment #26
lauriiiComment #27
alexpottTemplate changes are not frozen in beta. Committed 00df5ba and pushed to 8.0.x. Thanks!