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

Nesting Level Contradiction #17

Closed
stramel opened this issue Jun 15, 2015 · 2 comments
Closed

Nesting Level Contradiction #17

stramel opened this issue Jun 15, 2015 · 2 comments

Comments

@stramel
Copy link
Contributor

stramel commented Jun 15, 2015

At Variants of nested components, the article states not to reach in and use nesting to solve this issue.

.vote-box {
  &.-highlight {
    > .up { /* ... */ }
  }
}

Then at Avoid over-nesting, the article states one should avoid nesting more than 1 level deep. This would make the previous example incorrect and the following example correct which was stated as incorrect in the article.

.article-header {
  > .vote-box > .up { /* ✗ avoid this */ }
}

Could you add some clarification to this?

@mcgwiz
Copy link
Contributor

mcgwiz commented Jun 15, 2015

As the submitter of the PR for "variants of nested components" section, I am personally indifferent toward the notion of over-nesting so I didn't pay attention to it in my submission. That was my fault and the example should be rewritten to follow the over-nesting syntax guidance:

.vote-box {
  &.-highlight > .up { /* ... */ }
}

To clarify, what was incorrect in the other example was not the syntax, but the organizational coupling where the CSS implementation of one component (article-header) knows about and makes direct customizations to the CSS implementation of another component (vote-box). This couples the components together and also means components are taking dependencies on the implementation details of other components. You want to keep components abstract (make no assumptions about implementation details) and decoupled from each other in order to allow components to change independently of each other.

(Regarding over-nesting, I acknowledge that excessive indentation can reduce readability, but the simplicity and consistency of always nesting outweighs that risk. IMHO, it is unnecessary complication to nest some [arbitrary] number of levels then switch to standard contextual selector syntax. When it comes to variants, nesting is very convenient as customizations are often defined for more than one element in the DOM tree. Also, always nesting makes CSS parent-descendant value inheritance more apparent. When the Less/Sass nesting becomes excessive and difficult to read, then the HTML indentation will also be equivalently unreadable, and I prefer addressing those cases by considering if one or more new components can be factored out. Just my 2 cents.)

stramel added a commit to stramel/rscss that referenced this issue Jun 15, 2015
Update the Variants of nested components section to respect nesting rules stated in the Avoid over-nesting section. Related to issue rstacruz#17
@stramel stramel closed this as completed Aug 4, 2015
@rstacruz
Copy link
Owner

rstacruz commented Aug 5, 2015

Thank you for the fixes! Yes it was an oversight indeed.

Regarding over-nesting, I acknowledge that excessive indentation can reduce readability, but the simplicity and consistency of always nesting outweighs that risk.

I completely agree. There are times when nesting makes sense. That's why rscss uses the words "avoid" and "prefer" (instead of "don't" and "do") meaning they should be considered more like guidelines than hard rules.

In my experience, the tendency to over-nest is something I see very often. This pattern persists a lot that I felt it was necessary to address it:

.navbar {
  .left {
    .search {
      .input-box {
        background: white;

There's clearly many way to fix this, and the a { b { c { } } } => a { b c { .. } } style is just one of the ways.

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