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

Extends, nesting and the & combinator. #1554

Closed
pedrocorreia opened this issue Sep 17, 2013 · 18 comments
Closed

Extends, nesting and the & combinator. #1554

pedrocorreia opened this issue Sep 17, 2013 · 18 comments

Comments

@pedrocorreia
Copy link

Hi, and first of all thank you for making and maintaining LESS.

I'm not sure if this is a bug or i'm simply not understanding the intended usage for extends, but i thought that this:

.foo {

    /* foo */

    &.bar {
        /* prop A */
        /* prop B */
    }

    &.baz {
        &:extend(&.bar);
        /* prop C */
    }

}

would result in this:

.foo {
    /* foo */
}
.foo.bar,
.foo.baz {
      /* prop A */
      /* prop B */
}
.foo.baz {
      /* prop C */
}

It does not. However writing the full selector as the extend argument works:

.foo {

    /* foo */

    &.bar {
        /* prop A */
        /* prop B */
    }

    &.baz {
        &:extend(.foo.bar);
        /* prop C */
    }

}

I imagined that the & symbol would do the same magic with extends.
And having it not work, seemed really counter-intuitive.

@ZoeBijl
Copy link

ZoeBijl commented Sep 17, 2013

I imagine this is because :extend doesn't solely search within the current scope, but simple searches for &.bar, which does not exist. .foo.bar does however exist.

Even if the extend feature would be made to work with your line of code it would generate something like :extend(.foo.baz.bar). This is because you are already within .foo and .bar through their uses of the ampersand; you are extending .foo.baz with what ever you throw after the ampersand, so to say.

@jonschlinkert
Copy link
Contributor

I imagine this is because :extend doesn't solely search within the current scope

I was just about to comment something similar, and that even if that weren't the case I personally would find using the ampersand that way to be counter-intuitive.

Keep in mind that directives can be used, like all, so doing something like :extend(&.foo all) is pretty hairy and, at this point, probably isn't something we're going to add to the syntax. For that reason I'm going to close this.

Please feel free to continue the conversation if you want to present additional viewpoints. we can always reopen in the future if we see additional use cases and clarification on the syntax. or @lukeapage might have some perspective to lend as well.

@lukeapage
Copy link
Member

I think & is just not getting replaced. You could argue it should error, instead it will just never match anything

@jonschlinkert
Copy link
Contributor

@lukeapage are you saying that the & already works the way he's describing? if so, what are the scenarios in which it will/won't work?

@pedrocorreia
Copy link
Author

To be honest i haven't looked at the inner workings of it, i'd say that to determine if .foo.bar is a valid selector the compiler already has to go to the parent scope and check its children.

Having been a LESS user for quite some time now the :extend(&.bar) just seemed natural.

as @lukeapage said: I think & is just not getting replaced

@jonschlinkert
Copy link
Contributor

Having been a LESS user for quite some time now the :extend(&.bar) just seemed natural.

I can see where you're coming from. To date, the way that extend is, or should have been implemented, has seems to have been somewhat contentious, and understandably so. perhaps I was a little hasty in closing the issue, there was no intent other than attempting to dwindle down the massive number of issues building up. @lukeapage what are your thoughts? feel free to reopen

@lukeapage lukeapage reopened this Sep 17, 2013
@pedrocorreia
Copy link
Author

Sure @jonschlinkert, no harm done.

I was just trying to convey what it felt like as well as the "problem" itself.

@lukeapage
Copy link
Member

It should probably be open but low priority.. and fixed by either showing an error or making it work, whatever is easier.. I can't see many uses-cases for this?

@ZoeBijl
Copy link

ZoeBijl commented Sep 17, 2013

The above code could easily be achieved by this:

.foo {
    &.bar,
    &.baz,{
       /* prop a + b */
    }

    &.baz {
      /* prop c */
    }

}

Even if the ampersand would be changed, doesn't that make you code less readable?

@pedrocorreia
Copy link
Author

Sure, but if selector grouping was easy to do or maintain then there wouldn't be much use for the extend feature.

Here's a use case:

.button {

  &.basic {

    padding:1em;
    color: black;

    &.green {
      color: white; 
      background: green;
    }

    &.red {
      &:extend(.button.basic.green);
      background: red;

    }

    &.blue  {
      &:extend(.button.basic.green);            
      background: blue;
    }

  }
}

Currently results in :

.button.basic {
  padding: 1em;
  color: black;
}
.button.basic.green,
.button.basic.red,
.button.basic.blue {
  color: white;
  background: green;
}
.button.basic.red {
  background: red;
}
.button.basic.blue {
  background: blue;
}

It just seemed that,

.button {

  &.basic {

    padding:1em;
    color: black;

    &.green {
      color: white; 
      background: green;
    }

    &.red {
      &:extend(&.green);
      background: red;

    }

    &.blue  {
      &:extend(&.green);            
      background: blue;
    }

  }
}

Would be a "leaner" way to write it.

@ZoeBijl
Copy link

ZoeBijl commented Sep 17, 2013

Would be a "leaner" way to write it.

Sure, that is leaner, but I don't think it is the right usage of extend.

Say for example we got a .clearfix(); mixing:

// Credit: Nicolas Gallagher
.clearfix {
  &:before,
  &:after {
    content: " ";
    display: table;
  }
  &:after {
    clear: both;
  }
}

Before :extend() you would do something like this:

.gallery-list {
  .clearfix();
  border: 1px solid #afa;
  border-radius: 4px;
}

.gallery-item {
  float: left;
}

The output would include a standalone .clearfix class, but also the exact same code on the .gallery-list class—plus the code that was in addition to .gallery-list of course.

With extend the output would be much leaner; it would generate the styles specific to .gallery-list on it's own and the styling for .clearfix combined with the .gallery-list class.

So if we'd put this into action:

.gallery-list {
  &:extend(.clearfix());
  border: 1px solid #afa;
  border-radius: 4px;
}

.gallery-item {
  float: left;
}

Would result in the following CSS:

.gallery-list {
  border: 1px solid #afa;
  border-radius: 4px;
}

.gallery-item {
  float: left;
}

.clearfix:before,
.gallery-list:before,
.clearfix:after,
.gallery-list:after {
  content: " ";
  display: table;
}
.clearfix:after,
.gallery-list:after {
  clear: both;
}

Which is of course a lot shorter than copying all the styles to the gallery-list itself.

As for your example, I'd recommend reading up on SMACSS: http://smacss.com or another modular system for CSS. If you have a better example I'd love to discuss this further, I just don't think the given example is the correct usage of :extend.

@jonschlinkert
Copy link
Contributor

This is just food for thought, but it will always be difficult or impossible to make decisions based on stylistic preferences. The reason I closed the issue originally is that this can already be achieved using other means, but... that doesn't mean that some users won't get value from a different method or that other use cases won't be exposed once the feature is implemented. I suspect that this is one of those features that will end up giving users interesting workarounds and hacks that no one is thinking of yet...

however, all the examples given so far don't necessarily provide any benefit over current methods. So IMHO it boils down to this: out of the ~200 issues, many of which are feature requests for things that would provide transformative unilateral value to Less.js users, which features do you want to cast your vote for? We only have one @lukeapage...

@pedrocorreia
Copy link
Author

As i said earlier the only issue i found to be unexpected was that suddenly the & symbol does not do it's magic. Since the & symbol is used as a way of referencing selectors in the inheritance chain it felt weird that the same "shorthand" could not be used to pass selectors as arguments to the extend function.

Let's not forget that either CSS or LESS are just tools, so the definition of proper usage depends on the person using it and the objective.

@CodingKittens my example is essentially the same usage as Luke Page proposes / demonstrates on his article Extends in LESS, the difference being that i incorporated additional LESS features such as nesting and the & combinator.

Your example would be a perfect usage of extends, but unfortunately you cannot pass a parametric mixin (that would simply be the bees knees) as an argument and for the code to work, as you intended, you'll have to use the allkeyword:

.clearfix {
  &:before,
  &:after {
    content: " ";
    display: table;
  }
  &:after {
    clear: both;
  }
}

.gallery-list {
  &:extend(.clearfix all);
  border: 1px solid #afa;
  border-radius: 4px;
}

.gallery-item {
  float: left;
}

results in

.clearfix:before,
.clearfix:after,
.gallery-list:before,
.gallery-list:after {
  content: " ";
  display: table;
}
.clearfix:after,
.gallery-list:after {
  clear: both;
}
.gallery-list {
  border: 1px solid #afa;
  border-radius: 4px;
}
.gallery-item {
  float: left;
}

I'm currently working on a html / css framework Ink and we use multiple classes to set styling on elements, so my usage of extends would be to minimize if not, completely eliminate duplicate declarations of css properties, which was the basis for my example and seems to be what extend does.

@pedrocorreia
Copy link
Author

Also i just found that extend seems to allow a selector to extend itself:

.foo {

    /* foo */

    &.bar {
        /* prop A */
        /* prop B */
    }

    &.baz {
        /* prop C */
        &:extend(.foo.baz);
    }

}

compiles to

.foo {
    /* foo */
}
.foo.bar {
    /* prop A */
    /* prop B */
}
.foo.baz,
.foo.baz {
    /* prop C */
}

@seven-phases-max
Copy link
Member

Btw. I'd rather marked this as a feature request not as a bug (there're two features here actually: 1. expanding & within the extend statement itself. 2. Extending "dynamically generated classes" (which is also two sub-features in its turn: i.e. #1539 and #1597 :) .

@mattez
Copy link

mattez commented Jun 23, 2015

Hi. I like LESS but... Dont you know is this is an issue in SASS too? Thank you.

@seven-phases-max
Copy link
Member

@mattez

Not a good idea to ask if Fortran supports something at the C++ issue tracker (not counting that issue-trackers are normally not for general language questions at all) :) Consider ask at the SO.

@stale
Copy link

stale bot commented Nov 14, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 14, 2017
@stale stale bot closed this as completed Nov 28, 2017
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

6 participants