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

Bug: extend doesn't work when appended on nested selector with & #2206

Closed
SimonHarte opened this issue Sep 25, 2014 · 10 comments
Closed

Bug: extend doesn't work when appended on nested selector with & #2206

SimonHarte opened this issue Sep 25, 2014 · 10 comments

Comments

@SimonHarte
Copy link

Tested this with http://less2css.org/, experienced it in a project with LessJS 1.7.1:

LessCSS:

.icon-heart {
  content: '\2661';
}

.myModule {
  .test {
    &.active:extend(.icon-heart) {}
  }

  .test2 {
    &.active {
      &:extend(.icon-heart);
    }
  }
}

Expected:

.icon-heart,
.myModule .test.active,
.myModule .test2.active {
  content: '\2661';
}

Result:

.icon-heart,
.myModule .test2.active {
  content: '\2661';
}
@SimonHarte
Copy link
Author

Note that these test cases work:

.icon-heart {
  content: '\2661';
}

.test {
  &.active:extend(.icon-heart) {}
}
.myModule {
  .test {
    .active:extend(.icon-heart) {}
  }
}

@kdask-devnull
Copy link

I think another relevant but simplier example is the following:

.wrapper {
    .bar {
      text-decoration:underline;
    }
    .foo {
      &:extend(.bar all );
      color:red;
    }
}

That results to:

.wrapper .bar,
.wrapper .wrapper .foo {
  text-decoration: underline;
}
.wrapper .foo {
  color: red;
}

Instead of:

.wrapper .bar,
.wrapper .foo {
  text-decoration: underline;
}
.wrapper .foo {
  color: red;
}

@SimonHarte
Copy link
Author

That is another case @kdask-devnull and (currently) proper behaviour. Like mentioned in the Extend "all" section:

You can think of this mode of operation as essentially doing a non-destructive search and replace.

In your example LessJS just takes the selector .wrapper .foo (extracted from nested &) and "non-destructively" replaces all .bar strings in the CSS output with that, resulting in .wrapper .wrapper .foo. What you actually should do is to properly extend .wrapper .bar like this:

.wrapper {
    .bar {
      text-decoration: underline;
    }
    .foo {
      &:extend(.wrapper .bar all);
      color: red;
    }
}

(What if there is a global .bar selector anyway, you would automatically extend that one as well)

@kdask-devnull
Copy link

I see, thanks for your reply, but doesn't this conflicts with the nested rules logic?
Let's say for example i want to use a 3rd party library (bootstrap for example) only
inside a nested element and not globally eg:

.wrapper {

    @import "bootstrap/less/bootstrap.less";

}

This will have an effect that the 3rd party mixins would not be valid anymore.
For example: (taken from https://github.com/twbs/bootstrap/blob/master/less/mixins/grid.less)

.make-row(@gutter: @grid-gutter-width) {
    margin-left: (@gutter / -2);
    margin-right: (@gutter / -2);
    &:extend(.clearfix all);
}

@seven-phases-max
Copy link
Member

@kdask-devnull
As @SimonHarte already mentioned this is just how all is supposed to work. Your use-case example is a subject for another extend sub-feature(s) like those mentioned in #2101 (in particular deep + scoped combo that essentially will make extend to work with scope and nesting just like mixins do).

@macgyver
Copy link

can't you fix this by putting some rules in that empty selector block?

@seven-phases-max
Copy link
Member

@macgyver The easiest workaround (if you stepped this issue) is given in the initial post itself, i.e. just put :extend on its own as in .test2 class. And no, contents of the {} after &.active:extend(.icon-heart) do not affect anything.

@SomMeri SomMeri self-assigned this Jan 28, 2015
@SomMeri
Copy link
Member

SomMeri commented Jan 29, 2015

I can fix this, it is just small change. However, I have one question before I do so.

How should following compile:

  .icon-heart {
    content: '\2661';
  }

  .myModule:extend(.icon-heart) {
    .test {
      &.active {}
    }
  }

a.) like this:

.icon-heart,
.myModule {
  content: '\2661';
}

b.) or rather like this:

.icon-heart,
.myModule .test,
.active.myModule .test {
  content: '\2661';
}

@seven-phases-max
Copy link
Member

@SomMeri

It's a I guess. To get b it has to be something scary like:

.icon-heart {
    content: '\2661';
}

.myModule {
    .test {
        &, .active& {
            &:extend(.icon-heart);
        }
    }
}

@lukeapage
Copy link
Member

definitely (a).

Because its equivalent to this..

  .icon-heart {
    content: '\2661';
  }

  .myModule:extend(.icon-heart) {
  }

  .myModule .test {
      &.active {
      }
  }

which is consistent with less - it does the pulling out of the scopes before processing the extend.

In addition its as Max pointed out.. if we got (b) there would be no way to not get (b) where as with (a) a user has the option of adding (b).

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