roryashfordbentley / Wordpress-Bem-Menu

Better menu implementation for Wordpress using BEM syntax
138 stars 23 forks source link

BEM Naming Convention & nested elements #14

Open nocean opened 8 years ago

nocean commented 8 years ago

I wanted to bring up an issue more for the sake of discussing the best practice, and getting opinions.

The default for sub-menus, Wordpress-Bem-Menu nests elements, which I think is frowned upon. So we have classes like:

.menu (block)
-- .menu__item (block__element)
-- -- .menu__sub-menu (block__element)
-- -- -- .menu__sub-menu__item (block__element__element)

IMO, I think it would be more inline with BEM methodology to either split sub-menu into its own block, with its own elements (i.e. .sub-menu__item), or make being a sub-menu a modifier, and leave the elements in it the same as for the parents:

.menu (block)
-- .menu__item (block__element)
-- -- .menu--sub-menu (block--modifier)
-- -- -- .menu--sub-menu .menu__item (block--modifier block__element)

I think the latter is the more sensible solution, as a "sub menu" isn't really a block-level item, as it cannot exist outside of the menu. The fact that it is "sub" is just a modification of the menu, the element structure is the same.

Other than some minor tweaking of the item_css_class_suffixes array (with no good affect), I haven't looked closely at how the code would need to be modified to accommodate something like this -- or even if it is possible. I thought I'd mention it all the same though, as I really like this project.

roryashfordbentley commented 8 years ago

The general rule of thumb that I work by is that nested elements is a lesser evil than nested selectors, they discuss it briefly in the article you linked under the heading 'Can a block modifier affect elements?'

By nesting the elements e.g. .my-menu__sub-menu__item the selector is long and looks a little ugly but we are ensuring that the specificity of each selector is kept as low as possible.

...That being said, I'm a big advocate of build things in a way that carries meaning for yourself so feel free to fork the repo and build in whatever syntax works best for you :)

ancienthero commented 6 years ago

Generally nesting elements is not preferred. How about .my-menu__sub-item instead of .my-menu__sub-menu__item ?

roryashfordbentley commented 6 years ago

@ancienthero As I mentioned above the justification for nesting elements is that it is a lesser evil than nesting selectors and maintains a low level of specificity.

If you wan't to change it to meet your class naming conventions you can do something like this:

$this->item_css_classes = array(
    'item' => '__item',
    'parent_item' => '__item--parent',
    'active_item'  => '__item--active',
    'parent_of_active_item' => '__item--parent--active',
    'ancestor_of_active_item' => '__item--ancestor--active',
    'link'  => '__link',
    'sub_menu'  => '__sub-menu',
    'sub_menu_item'  => '__sub-item'
);