-
-
Notifications
You must be signed in to change notification settings - Fork 319
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
mergeRules when combined with mergeLonghand can create the wrong rules #701
Comments
Oh, can we reduce example? I think problem in |
To be honestly, i don't have enough time and funds 😞 to works a lot of time on this so feel free to send a PR |
Cool thanks I will see if I can get it working and PR something! 👍 For others, here's the minimum reproduction: .foo {
border-color: #cc1f1a;
}
.bar {
border-bottom-width: 1px;
border-style: solid;
border-color: #cc1f1a;
} Compiles to: .bar,
.foo {
border-color: #cc1f1a
}
.bar {
border-bottom: 1px;
border-style: solid
} Expected: .foo {
border-color: #cc1f1a;
}
.bar {
border-bottom: 1px;
border-style: solid;
border-color: #cc1f1a;
} This is related but possibly a different issue, but this CSS: .foo {
border-color: #cc1f1a;
}
.bar {
border-bottom: 1px;
border-color: #cc1f1a;
} ...compiles to: .bar,
.foo {
border-color: #cc1f1a
}
.bar {
border-bottom: 1px
} ...which is also wrong, since it's important that the Expected result is that it just doesn't change: .foo {
border-color: #cc1f1a;
}
.bar {
border-bottom: 1px;
border-color: #cc1f1a;
} |
@adamwathan Can you provide what you expected too for other developers? |
Added expected behavior to the comment 👍 |
This comment has been minimized.
This comment has been minimized.
@tmorehouse not related to this issue |
Input .foo {
border-color: #cc1f1a;
}
.bar {
border-bottom-width: 1px;
border-style: solid;
border-color: #cc1f1a;
} With .foo {
border-color: #cc1f1a;
}
.bar {
border-bottom: 1px;
border-color: #cc1f1a;
border-style: solid;
} With .foo {
border-color: #cc1f1a;
}
.bar {
border-bottom-width: 1px;
border-style: solid;
border-color: #cc1f1a;
} With .foo,.bar {
border-color: #cc1f1a;
}
.bar {
border-bottom: 1px;
border-style: solid;
} With .foo {
border-color: #cc1f1a;
}
.bar {
border-bottom: 1px;
border-color: #cc1f1a;
border-style: solid;
} Here, both plugins are working fine as independently but not together. cc @evilebottnawi ! |
After running a quick test, I think #1450 might solve this |
Apologies for not being able to reduce this to the absolute minimum reproduction, but given this CSS:
When minified with mergeRules and mergeLonghand both enabled, the output is:
The important changes here are that the
border-color
declarations were grouped and theborder-color
declaration from the.error-banner .field-errors.filled
selector was removed, and that what was originallyborder-bottom-width: 1px
is simplified toborder-bottom: 1px
.But because the
border-color
declaration was hoisted to the group at the top, the border color no longer takes effect for the.error-banner .field-errors.filled
selector becauseborder-bottom: 1px
sets a default color ofblack
which overrides it.This sort of hand-crafted diff tries to make the important changes clear:
In this case the expected behavior is that because
border-color
was hoisted out,border-bottom-width
cannot be simplified toborder-bottom
, or alternatively, theborder-color
declaration should not be removed from that selector at all.Tricky stuff to get right for sure, I don't envy anyone trying to build a tool like this but am extremely grateful for your hard work 👍
The text was updated successfully, but these errors were encountered: