-
Notifications
You must be signed in to change notification settings - Fork 1
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
You MUST use @extend when working with module to inherit from the base element #47
Comments
Interested as to why this route? Personally I've been running a .site-message {
color: white;
font-size: 1.4rem;
&.site-message--error {
background-color: red;
}
} style of approach. |
Isnt that an unnecessary increase on specificity though? B |
I don't know... I've never come across a time when I've needed a |
But the above would result in the following;
B |
Corrected the code example... |
Jon, please explain. I'm so confused. Why do you feel your approach is a more suited option? B |
I'm not really trumping my method, it's a genuine question... I don't like repetition. .site-message {
color: white;
font-size: 1.4rem;
}
.site-message--error {
color: white;
font-size: 1.4rem;
background-color: red;
} I've been using the quoted code, mainly as it encapsulates the whole block in indented braces and keeps everything in a module together. The more specific selector doesn't do any harm (or does it?). As a modifier class has a distinct name, why extend at all? (Not rhetorical, discuss) .site-message {
color: white;
font-size: 1.4rem;
}
.site-message--error {
background-color: red;
} |
The option you include @jonspark will not inherit the properties set on the base class as required. If you run your option through a Sass compiler, you get the following;
This doesn't achieve the required result with regards to our BEM approach. B |
So what is the result we're trying to achieve cos I think I missed the memo... |
Well if If you look at your implementation, B |
I know the aim of your example @Passenger-Inspired is to stop bloat of the class attribute, but logically wouldn't it make more sense to have: <div class="site-message site-message--error"></div> That way i can instantly see it's the site message module with the error helper? Little article on using more classes in HTML from Harry Roberts which is a good read - http://csswizardry.com/2012/10/a-classless-class-on-using-more-classes-in-your-html/ |
Actually sorry @jonspark, now see what you're saying. Will speak when you come in the office, it will be easier. B |
Closed, as I have spoken with @jonspark seperately |
Implemented |
Needs to be changed slightly to specify you should only extend placeholder elements, and the fact it may not need to be a MUST |
Two things with this. I propose we only use Thoughts on it being a MUST? I think it should be due to trying to enforce a coding style, but open to other ideas. |
I think this sounds like a good idea going forward as we've seen the mess that using I've not got any issues with going with a MUST for this as i tend to stay away from using the |
Still requires sign off from @matchboxhero, so will wait to add. Ben |
Using @extend will allow you to remove the bloat which may exist within your class attribute contained in your module's markup.
Because, as a team we work using BEM notation, a modifier or child element are prefixed with the base class, so for readability purposes within your Markup, there is no need to contain both the base element and the modifier within the class attribute.
Example:
With regards to file size, this is not so much of an issue, as GZIP will take care of any duplication contained within your partials.
Note: This also applies to utility and helper methods too.
The text was updated successfully, but these errors were encountered: