Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 UTC on 18 March 2024, to get $100 off your ticket.
Problem/Motivation
#2407735: Remove classes from system templates m*.html.twig removed the 'menu' classes from menu.html.twig. The toolbar module unfortunately relies on these classes currently in both its CSS and JS.
Proposed resolution
Add the classes back until #1944572: Remove "ul.menu" dependency to prevent theme clashes is resolved. Before/after screenshots from Stark:
Before
After
Remaining tasks
- Patch
- Review
- Commit
User interface changes
Restore toolbar appearance and functionality in any theme that doesn't extend from Classy.
API changes
n/a
Comment | File | Size | Author |
---|---|---|---|
#2 | 2447829-1-after.png | 26.18 KB | star-szr |
#2 | 2447829-1-before.png | 33.4 KB | star-szr |
#1 | 2447829-1.patch | 543 bytes | star-szr |
Comments
Comment #1
star-szrComment #2
star-szrScreenshots for the issue summary.
Comment #3
star-szrComment #4
star-szrComment #5
LewisNyman CreditAttribution: LewisNyman commentedLooks good, thanks!
Comment #6
hass CreditAttribution: hass commentedDoes this make #1944572: Remove "ul.menu" dependency to prevent theme clashes critical?
Comment #7
mortendk CreditAttribution: mortendk commentedwhat - are we now adding in classes *gasp* :(
imho we should fix the real problem instead & get #1944572: Remove "ul.menu" dependency to prevent theme clashes in
@hass yes it kinda do :/
Comment #8
hass CreditAttribution: hass commentedWhat are you talking about? We are fixing the real issue.
Comment #9
mortendk CreditAttribution: mortendk commentedare were adding in a class to core now just to to remove it again when #1944572 gets in ?
Comment #10
star-szrThere is currently coupling of .menu classes with the toolbar, as long as that exists the classes should be there in every theme. The fact that the coupling exists (#1944572: Remove "ul.menu" dependency to prevent theme clashes) is not release blocking at all IMO. Let's fix the regression and move forward please.
Comment #11
mortendk CreditAttribution: mortendk commented@cottser check was just asking also why i didnt change status & is breathing in a paperbag ;)
Comment #12
star-szr:D
Comment #13
hass CreditAttribution: hass commentedWe need to get #1944572: Remove "ul.menu" dependency to prevent theme clashes done before final D8. This is a critical bugfix for contrib themes. Otherwise we may discuss why we cannot change theme templates post D8 release. :-(
Comment #14
alexpottI agree with @Cottser - #1944572: Remove "ul.menu" dependency to prevent theme clashes is not release blocking. Committed 1506a99 and pushed to 8.0.x. Thanks!