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

Allow indentation of non-nested code #233

Closed
arcanis opened this issue Sep 17, 2014 · 7 comments
Closed

Allow indentation of non-nested code #233

arcanis opened this issue Sep 17, 2014 · 7 comments

Comments

@arcanis
Copy link

arcanis commented Sep 17, 2014

There should be a setting allowing to indent non-nested code. For example, the following is valid and is often used to tell the reader that there is a kind of hierarchy, but without using fifteen kilometers long selectors :

.foo {
}

    .something-inside-foo {
    }

        .and-something-inside-its-parent {
        }

Instead of

.foo {
    .something-inside-foo {
        .and-something-inside-its-parent {
        }
    }
}

Doing this generates

.foo { }
.something-inside-foo { }
.and-something-inside-its-parent { }

Instead of

.foo { }
.foo .something-inside-foo { }
.foo .something-inside-foo .and-something-inside-its-parent { }
...

I suggest adding an option to the Indentation linter to ignore indentation errors when they increase the indentation level of a rule by a single level, and only if the rule is not the first one of the block.

@sds sds added the question label Sep 20, 2014
@sds
Copy link
Owner

sds commented Sep 20, 2014

Hey @arcanis, thanks for suggesting a way we can improve the tool.

I can appreciate the convention you're describing here: you're optimizing the resulting CSS both in terms of bytes as well as rendering performance, while continuing to describe the hierarchy of the components you are styling.

However, I would like to suggest that the effort required in following this convention is not worthwhile.

In your example, you represent your selectors in a hierarchy for readability purposes — .something-inside-foo is a descendant of .foo — but only conceptually rather than in actuality. The purpose of nesting selectors in a hierarchy is to introduce an explicit coupling.

If there isn't a need to explicitly define a hierarchy in selectors, you should bias yourself towards a flatter hierarchy. In other words, I would argue the better way to write your code would be to keep it flat:

.foo {
}

.something-inside-foo {
}

.and-something-inside-its-parent {
}

Keeping yourself to a flatter hierarchy promotes the development of more general components, which improves code reusability and reduces brittleness. Not to blindly reiterate the gospel, but it bears worth repeating: tying your CSS to your DOM structure should be avoided. [1]

Furthermore, while I don't know the specifics of your particular application, I would put forth the claim that this kind of micro-optimization isn't worth the effort for the vast majority of applications. I doubt you'll experience a major difference in rendering performance if you avoid explicit nesting. [2]

For cases where there is a dependency, be explicit and use nested selectors. It protects your components from accidentally being styled by more general selectors when you don't intend it.

Happy to hear your thoughts on this topic. I would be curious if you have specific data that provides a strong case for the convention you describe. If there is, please share!

@ArmorDarks
Copy link

I vote for that feature.

@sds Unfortunately, your suggestion applying poorly to heavy projects with tons of selectors. You simply don't have opportunity to name them in proposed by your way, since you'll quickly finish with extremely long class names, or you won't see clearly relation between components.

We're using BEM in our projects, and it means that class name always have component's (block) name inside, and in case someone is child of that block — it will have it's own reasonable name too.

From the first glance it looks like it's good idea to write such classes in flat way:

.component {}
.component__image {}
.component__text {}

But than you're quickly realizing that it will work good only on first levels. Just take a look at this code:

.component {}
.component__image {}
.component__text {}
.component-subblock {}
.component-subblock__text {}
.component-category {}
.component-other {}

What does it tells to you? Do you really see connections between blocks?

While your page's number of blocks increasing, with flat CSS style you're slowly losing whole vision of the project's structure. With proper nesting just from a first glance you'll be able to understand current relation between blocks:

.component {}
  .component__image {}
  .component__text {}
     .component-subblock {}
     .component-subblock__text {}
  .component-category {}
    .component-other {}

Now you see what real page currently looks like? It's completely different from what you've imagined at first, while looking at flatted style.

Your suggestion won't work for BEM, since it's main idea is abstraction and reusability. In your case to move block to another place I will need to heavily change both CSS and HTML (since class name is tied way too much to structure... just to keep flat style readable and understandable... so now I need to change it in all instances) or deal with simple fact that it's impossible to understand structure just from CSS, while in case of BEM if I decide that I need to move .component-other outside of .component-category I will need to change only HTML code, and simply to move selector with indentation to another position and I and other devs always have clear vision of current structure. It's way much easier than renaming class names, especially in large projects.

Besides, just image distributed across different domains components — in such case you can't "indent" component and represent their structure by naming in your way.

In such situations indentation is the only possible way to keep reasonable connection between HTML and CSS structure without making them dependent on each other, like in case of nesting usage.

To be honest, I was surprised that SCSS-lint does treat such indentation as errors, since it's very popular, especially across BEM-devs.

For example, well-known Harry Roberts: https://github.com/csswizardry/work/blob/gh-pages/work.scss

@sds sds added the enhancement label Oct 7, 2014
@sds
Copy link
Owner

sds commented Oct 7, 2014

Sorry for the delay in response, @ArmorDarks.

Thank you for clarifying how this style is used, especially for BEM projects. I stand by my comments, but I completely acknowledge that there are simply different strokes for different folks. I can see the advantage of using indentation to communicate additional information about hierarchy without the performance cost.

At the end of the day, I don't see a major issue with adding an option to the Indentation linter to support this. A pull request would be welcome. Thanks!

@sds sds changed the title Indentation Allow indentation of non-nested code Oct 7, 2014
@sds sds removed the question label Oct 7, 2014
@sds
Copy link
Owner

sds commented Feb 2, 2015

Implemented in 1b0f2e5.

@sds sds closed this as completed Feb 2, 2015
@ArmorDarks
Copy link

Great, thanks a lot!

@kty
Copy link

kty commented May 11, 2015

This doesn't support class names with parent selectors.
As of Sass 3.3 parent selectors (&) can be used for prefixing class names, which means we can now rewrite this:

.block {}
.block__table {}
.block__row {}

into this:

.block {
    &__table {}
    &__row {}
}

Let's say &__row will always be inside &__table and I'd like to suggest that in the code:

.block {
    &__table {}
        &__row {}
}

This triggers a lint error, although being (IMO) a legitimate use of arbitrary indentation.

Would you consider removing the condition that arbitrary indentation is allowed only for top-level rulesets? Thanks!

@sds
Copy link
Owner

sds commented May 12, 2015

Hey @kty, we would happily accept a pull request adding support for the feature you describe. Thanks!

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

4 participants