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

Improve parent resolution #2151

Closed
wants to merge 1 commit into from

Conversation

maciej-ka
Copy link

Places pseudo class selectors at the end, removes duplicate selectors
and removes universal selector if possible during resolution of parent
selector (&)

Pseudo at the end

div:before { &[id] { color: red; }}

Before:

div:before[id] {
  color: red; }

After:

div[id]:before {
  color: red; }

Remove duplicates

.foo { &.foo { color: red; }}

Before:

.foo.foo {
  color: red; }

After:

.foo {
  color: red; }

Remove universal

* { &.foo { color: red; }}

Before:

*.foo {
  color: red; }

After:

.foo {
  color: red; }

Related to: #490
Tests: sass/sass-spec#931

Places pseudo class selectors at the end, removes duplicate selectors
and removes universal selector if possible during resolution of parent
selector (&)

Pseudo at the end
---
```sass
div:before { &[id] { color: red; }}
```
Before:
```css
div:before[id] {
  color: red; }
```
After:
```css
div[id]:before {
  color: red; }
```

Remove duplicates
---
```sass
.foo { &.foo { color: red; }}
```
Before:
```css
.foo.foo {
  color: red; }
```
After:
```css
.foo {
  color: red; }
```

Remove universal
---
```sass
* { &.foo { color: red; }}
```
Before:
```css
*.foo {
  color: red; }
```
After:
```css
.foo {
  color: red; }
```

Related to: sass#490
Tests: sass/sass-spec#931
@long-lazuli
Copy link

Beware of removing duplicate selector.
It should not be done automatically. Sass never controlled what you are trying to achieve with CSS.

There is a lot of cases, for specificity purpose, where you exactly expect this :

.foo.foo {
  color: red; }

from this :

.foo { &.foo { color: red; }}

It's not a Sass/SCSS problem.
People want to do that, they should be able to.

@maciej-ka
Copy link
Author

maciej-ka commented Sep 27, 2016

Reason for this implementation was @nex3 suggestion in #360. @long-lazuli Could you give any example where .foo.foo would be desirable?

@long-lazuli
Copy link

long-lazuli commented Sep 27, 2016

Let's say we use a plugin which use the class .foo to describe the width property to 10em.
We want to surcharge this width, to make this 'plugin/widget/whatever' responsive, to 100%.
We can achieve that to make a specificity higher using twice the class. As in the selector chain .foo.foo.

This is not a good practice, but I deal with that kind of tricks enough. That pull request would make some old-fashioned Sass code obsolete.

@maciej-ka
Copy link
Author

@long-lazuli Right, it is possible to use .foo.foo to rise selector specifity. I'm perfectly fine to remove this part of pull request. @nex3, @chriseppstein, what's your opinion on this?

@nex3
Copy link
Contributor

nex3 commented Dec 11, 2016

Thanks for the patch, and sorry for the long response time! Unless we can find a new maintainer, Ruby Sass is on track to be deprecated, so we aren't accepting non-critical new features at this time.

I'm marking this as "on ice", which means that if/when we find a new maintainer, they can find it and revive it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
3 participants