Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Impossibility of the Mixes implementation. Breaking general BEM concepts requirements. #10

Open
victorpavlov opened this issue Jul 31, 2018 · 3 comments

Comments

@victorpavlov
Copy link

Regarding to the BEM key concepts:

Blocks can be nested inside any other blocks.

In case, when we are placing one block inside another it's useful to have special class names (Mixes). This class names are responsible for the external geometry and positioning are set via the parent block.

So the block itself should not contain any external geometry and positioning information, because it can be used anywhere.

With current function implementation we can't use mixes. Now, when we are using $blockname variable together with $base_class we will have exactly something like a mixes, but in that case we will lose the block name itself.

Element with modifiers and blockname (optional)

And regarding to the BEM key concepts:

Element is a constituent part of a block that can't be used outside of it.

But the usage of the function described above breaks that main concepts requirements.
Assuming all said I can propose that more proper format to use the function will be something like this:

bem(block_name, (modifiers), mixes_name)

And in that case when we will use something like this:

<h1 {{ bem('title', ['small', 'red'], 'card') }}>

We should get:

<h1 class="title title--small title--red card__title">
@williambe
Copy link

Thanks for this Twig extension, my components are now far DRYer.
I agree whit the position of @victorpavlov and my approach also leans on BEM mixes instead of overriding base class.

If someone else is interested, these few changes work in my case:
https://github.com/williambe/bem-twig-extension/blob/4693f9c36c444720d33d4d27309e748e7a62488a/bem.function.php#L9-L27

@evanmwillhite
Copy link

evanmwillhite commented Dec 11, 2018

This is an interesting proposal that we could possibly use a separate argument for like above. And if BEM supports it theoretically, we should allow for it.

Just personal preference, in your example above, I wouldn't typically create an element like <h1 class="title title--small title--red card__title"> because it lessens encapsulation. Rather, I would create a Sass mixin for title which would be used in .title and any other element I wanted it in like:

.card__title {
  @include title;
}

This still keeps my styles encapsulated to each component but allows for reusability of styles. Not sure if that helps or is just personal opinion and not applicable.

I still think this is worthy of conversation as a topic, so I'm going to leave this open.

@evanmwillhite
Copy link

Rounding back to this. I think it would be fair to support BEM mixes, but I'm not a fan of changing the current implementation to automatically assume those. If I use

<h1 {{ bem('title', ['small', 'red'], 'card') }}>

I don't want it to create

<h1 class="title title--small title--red card__title">

I don't want title in my classes but would instead opt to make it a Sass mixin and then my card__title styling would be:

.card__title {
  @include title;

  // ..whatever else
}

So yea, I don't want the implementation to assume that someone would want both title and card_title in the output. That means to support this feature, we'd need to go a different route (a fifth argument or something else?). I'm open to whatever makes the most sense but don't have time to put towards this currently. If someone would like to submit a PR, feel free.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants