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

combining extends #3131

Closed
ferenci84 opened this issue Nov 22, 2017 · 2 comments
Closed

combining extends #3131

ferenci84 opened this issue Nov 22, 2017 · 2 comments

Comments

@ferenci84
Copy link

I have spotted a possible improvement to the extend (and possibly the mixin) feature that can be made. Actually it would allow using an existing css framework and any markup. I tried to solve this in other preprocessor too, but I think LESS is the closest to the best solution. I wouldn't call it a bug, however one may think that the existing feature should work this way.

I am experimenting with a dropdown feature distilled from w3css.

Here is my css used for dropdown and the target html markup:

.dropdown {

  display: block;
  position: relative;
  z-index: 1;

  &:hover .dropdown-content {
    display: block;
    z-index: 1;
  }

}

.dropdown-content {
  display: none;
  position: absolute;
}

<div class="nav-level1">
    <a href="/">Home</a>
    <div>
        <a href="#">Dropdown</a>
        <div class="nav-level2">
            <a href="#">Link1</a>
            <a href="#">Link2</a>
            <a href="#">Link3</a>
        </div>
    </div>
    <a href="#">Nav1</a>
    <a href="#">Nav2</a>
</div>

I would do this .less code:

nav-level1 {
  .nav-level2 {
    &:extend(.dropdown-content all);
  }
  > div {
    &:extend(.dropdown all);
  }
}

The resulting .css:

.dropdown,
.nav-level1 > div {
  display: block;
  position: relative;
  z-index: 1;
}
.dropdown:hover .dropdown-content,
.dropdown:hover .nav-level1 .nav-level2,
.nav-level1 > div:hover .dropdown-content {
  display: block;
  z-index: 1;
}
.dropdown-content,
.nav-level1 .nav-level2 {
  display: none;
  position: absolute;
}

Actually it almost correctly replaces all .dropdown-content reference except for this part:

.dropdown:hover .dropdown-content,
.dropdown:hover .nav-level1 .nav-level2,
.nav-level1 > div:hover .dropdown-content {
  display: block;
  z-index: 1;
}

It should read:

.dropdown:hover .dropdown-content,
.dropdown:hover .nav-level1 .nav-level2,
.nav-level1 > div:hover .dropdown-content,
.nav-level1 > div:hover .nav-level1 .nav-level2 {
  display: block;
  z-index: 1;
}

I intentionally placed the &:extend(.dropdown-content all); above &:extend(.dropdown all); as I think it makes the solution easier:
When the processor reaches an extend keyword, it would traverse not only the code in the less file, but also the generated part of the file (or the structure that is already generated) and it would process the extend also in the generated selectors of the past extends.
What do you think?

@ferenci84 ferenci84 changed the title extend all with hover combining extends Nov 22, 2017
@seven-phases-max
Copy link
Member

seven-phases-max commented Nov 22, 2017

If I understand it right (see below) this is a "combinatorial explosion" already considered at #1952 (and earlier in #1641).
It would be more readable if you'd reduce the example to as minimal snippet as possible concentrating on the feature itself (Otherwise it's a bit difficult to break through all these long and similar selectors to get what it's about. Usually things like a, .b, .c work the best).

@stale
Copy link

stale bot commented Mar 22, 2018

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 Mar 22, 2018
@stale stale bot closed this as completed Apr 5, 2018
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

2 participants