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

Files: 
CommentFileSizeAuthor
#2 2447829-1-after.png26.18 KBCottser
#2 2447829-1-before.png33.4 KBCottser
#1 2447829-1.patch543 bytesCottser
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 88,015 pass(es). View

Comments

Cottser’s picture

Status: Active » Needs review
FileSize
543 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 88,015 pass(es). View
Cottser’s picture

Issue summary: View changes
FileSize
33.4 KB
26.18 KB

Screenshots for the issue summary.

Cottser’s picture

Issue summary: View changes
Cottser’s picture

Issue summary: View changes
LewisNyman’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, thanks!

hass’s picture

mortendk’s picture

what - 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 :/

hass’s picture

What are you talking about? We are fixing the real issue.

mortendk’s picture

are were adding in a class to core now just to to remove it again when #1944572 gets in ?

Cottser’s picture

There 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.

mortendk’s picture

@cottser check was just asking also why i didnt change status & is breathing in a paperbag ;)

Cottser’s picture

Title: Add some classes back to menu.html.twig for toolbar functionality » Add "menu" classes back to menu.html.twig for toolbar functionality

:D

hass’s picture

We 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. :-(

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

I 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!

  • alexpott committed 1506a99 on 8.0.x
    Issue #2447829 by Cottser: Add "menu" classes back to menu.html.twig for...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.