Hi.

I must say that the classes that come menu.html.twig && menu-local-task.html seem to me quite redundant are can inhibit the productivity of some main-menu design tasks.

I woudl personally prefer edit the whole markup a new, after installing Basic... I thus suggest considering removing these two Twig files or at least removing this classes from them...

Ben.

Comments

Benia created an issue. See original summary.

Benia’s picture

Benia CreditAttribution: Benia commented
Issue summary: View changes
joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet as a volunteer commented
Status: Active » Postponed (maintainer needs more info)

Personally you can, this theme is meant to be a starter theme to implement your own design.
Which classes are you referring to though?

Benia’s picture

Benia CreditAttribution: Benia commented

Sure, I know it's a starter theme, and originally I meant to:

'menu-item',
item.in_active_trail ? 'menu-item--active-trail',

And also:
.is-active

We can just edit via #id a or #id a:active and I think removing these classes could make the theme even more lightweight and minimal... But I might be very wrong in that conclusion.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet as a volunteer commented

What's redundant about those classes? They are what's in Drupal 8 core and are BEM style so if you nested lists in your menu that you won't run into selector conflicts (think mega menus and things like that).

Also allows you to componentize your menu styles so that there is a consistent look. The #ids make things a bit set in stone for where you place things and should be avoided to keep things maintainable, IMO

leahtard’s picture

leahtard CreditAttribution: leahtard at The Jibe commented
Status: Postponed (maintainer needs more info) » Closed (works as designed)

Hi Benia,

Thanks for your issue -- we like cleanup the markup! I just don't see a way around needing these classes on the majority of projects. Styling :active is quite different than styling an is-active or menu-item--active-trail class. :active is only in place while the browser redirects. It is not aware of which page you are on like is-active. menu-item--active-trail is helpful when you need a parent item to have a particular style.

To be consistent, I would actually like to update is-active (SMACSS syntax) to use menu-item--active (BEM syntax) -- but I couldn't quite track down the best way to update this. It looks like core is adding this class via JS (core/misc/active-link.js).

We are using BEM syntax which, yes, can be longer at times. Like Joel mentioned, Basic is intended to be a starter theme, not a base theme. The idea being you take Basic, rename it, and make it your own. After doing that, you can defiantly rename these classes in a way that fits your particular project.

Welcome any other feedback you might have.

Cheers, Leah

Benia’s picture

Benia CreditAttribution: Benia commented

As I'm quite new to Drupal theming besides HTML-CSS, I didn't know much of these data. I no longer see these classes as redundant but I should master my work with them.

Much thanks!

leahtard’s picture

leahtard CreditAttribution: leahtard at The Jibe commented

Thanks Benia,

Don't hesitate to reach out with any other questions :)

Cheers, Leah