search

material-components / material-components-web

Modular and customizable Material Design UI components for the web
https://material.io/develop/web
MIT License
17.14k stars 2.14k forks source link

[Elevation] Elevation CSS does not follow BEM best-practices #4760

Open smoyer64 opened 5 years ago

smoyer64 commented 5 years ago

Bug report

According to the Block-Element-Modifier documentation, modifiers should not be used without a block.

Additional context

In the context of an elevation component, having a block (which in theory is an empty style class) seems like a bit of excess baggage. One benefit is that it becomes easier to find all elevations regardless of their current z-order.

Possible solution

Adding an empty mdc-elevation style class for the elevation block would allow component styling to occur per the BEM specification. The component's containing element would change from <div class="mdc-elevation--z4"> to <div class="mdc-elevation mdc-elevation--z4">

abhiomkar commented 5 years ago

Thanks for reporting this bug!

Did you mean <div class="mdc-elevation mdc-elevation--z4">?

I think the reason we created modifier class names (mdc-elevation--z4) without base class name is because mdc-elevation needs to be always paired with z value. mdc-elevation itself wouldn't be a standalone block.

Technically mdc-elevation--z4 is not serving as modifier, may be we need to just make it block instead of modifier for this reason: mdc-elevation-z4 instead of mdc-elevation--z4.

smoyer64 commented 5 years ago

Yes ... that's exactly what I meant (fixed). Providing each of the elevations as a block would indeed be an alternative that would comply with the BEM specification. I think your alternate solution is perfectly acceptable.

Note that searching for all elevation components would be a bit more complicated. With the proposed solution, document.getElementsByClassName("mdc-elevation") would return every elevation on the page. Admittedly I don't have a use-case that requires this search. (And it's important to note that you can find multiple classes simply by including them in the search - e.g. document.getElementsByClassName("mdc-elevation-z1 mdc-elevation-z4")).