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

You MUST use @extend when working with module to inherit from the base element #47

Open
BenjaminRCooper opened this issue Nov 3, 2013 · 19 comments

Comments

@BenjaminRCooper
Copy link
Contributor

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:

.site-message {
     color: white;
     font-size: 1.4rem;
}

.site-message--error {
     @extend .site-message;
     background-color: red;
}

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.

@ghost ghost assigned BenjaminRCooper Nov 3, 2013
@jonspark
Copy link

jonspark commented Nov 3, 2013

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.

@BenjaminRCooper
Copy link
Contributor Author

Isnt that an unnecessary increase on specificity though?

B

@jonspark
Copy link

jonspark commented Nov 4, 2013

I don't know... I've never come across a time when I've needed a .site-message--error without a .site-message. Generally did it to save on repeated code, D-R-Y and all that...

@BenjaminRCooper
Copy link
Contributor Author

But the above would result in the following;

.site-message.site-message--error {
      color: white;
      font-size: 1.4rem;
      background-color: red;
}

B

@jonspark
Copy link

jonspark commented Nov 4, 2013

Corrected the code example...

@BenjaminRCooper
Copy link
Contributor Author

Jon, please explain. I'm so confused.

Why do you feel your approach is a more suited option?

B

@jonspark
Copy link

jonspark commented Nov 4, 2013

I'm not really trumping my method, it's a genuine question... I don't like repetition. @extend is a double-declaration and produces:

.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;
}

@lewismorris
Copy link

Hey @jonspark ,

@extend would produce the following using @Passenger-Inspired example

.site-message, .site-message--error {
  color: white;
  font-size: 1.4rem;
}

.site-message--error {
  background-color: red;
}

I think this may be where the confusion is coming from.

Lewis

@BenjaminRCooper
Copy link
Contributor Author

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;

.site-message {
  color: white;
  font-size: 1.4rem;
}
.site-message.site-message--error {
  background-color: red;
}

This doesn't achieve the required result with regards to our BEM approach.

B

@jonspark
Copy link

jonspark commented Nov 4, 2013

This doesn't achieve the required result with regards to our BEM approach.

So what is the result we're trying to achieve cos I think I missed the memo...

@BenjaminRCooper
Copy link
Contributor Author

Well if site-message--error is a modifier of the site-message base block, how is the modifier receiving the base styles from the base block?

If you look at your implementation, site-message--error is only receiving one style, and not the styles from the base element.

B

@lewismorris
Copy link

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/

@BenjaminRCooper
Copy link
Contributor Author

Actually sorry @jonspark, now see what you're saying.

Will speak when you come in the office, it will be easier.

B

@BenjaminRCooper
Copy link
Contributor Author

Closed, as I have spoken with @jonspark seperately

@BenjaminRCooper
Copy link
Contributor Author

Implemented

@BenjaminRCooper
Copy link
Contributor Author

Needs to be changed slightly to specify you should only extend placeholder elements, and the fact it may not need to be a MUST

@BenjaminRCooper
Copy link
Contributor Author

Two things with this. I propose we only use @extend on placeholders. The idea is that when you create a utility or module which you would like to be extendable, you create both a placeholder and class for that item to allow for it to be referenced in your markup, as well as being extended as a placeholder.

Thoughts on it being a MUST? I think it should be due to trying to enforce a coding style, but open to other ideas.

@lewismorris
Copy link

I think this sounds like a good idea going forward as we've seen the mess that using @extend on classes can cause, when used incorrectly.

I've not got any issues with going with a MUST for this as i tend to stay away from using the @extend selector where i can.

@BenjaminRCooper
Copy link
Contributor Author

Still requires sign off from @matchboxhero, so will wait to add.

Ben

@BenjaminRCooper BenjaminRCooper removed their assignment May 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants