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
fix: merge-rule before longhand in default preset #886
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
import postcss from 'postcss'; | ||
import cssnano from '..'; | ||
|
||
test('should merge rules before longhand', () => { | ||
const css = ` | ||
.foo { | ||
border-color: #cc1f1a; | ||
} | ||
|
||
.bar { | ||
border-bottom-width: 1px; | ||
border-style: solid; | ||
border-color: #cc1f1a; | ||
} | ||
`; | ||
|
||
return postcss([cssnano]) | ||
.process(css, { from: undefined }) | ||
.then((result) => { | ||
expect(result.css).toBe( | ||
'.foo{border-color:#cc1f1a}.bar{border-bottom:1px;border-color:#cc1f1a;border-style:solid}' | ||
); | ||
}); | ||
}); | ||
|
||
test('should merge rules before longhand #2', () => { | ||
const css = ` | ||
.has-errors .input { | ||
background-color: #fcebea; | ||
border-color: #cc1f1a; | ||
} | ||
|
||
.has-errors .checkbox-label { | ||
background-color: #fcebea; | ||
border-color: #cc1f1a; | ||
} | ||
|
||
.has-errors .checkbox-inline { | ||
background-color: #fcebea; | ||
border-color: #cc1f1a; | ||
} | ||
|
||
.error-banner .field-errors.filled { | ||
width: 100%; | ||
padding: 1.5rem 1.5rem 1rem; | ||
background-color: #fcebea; | ||
border-bottom-width: 1px; | ||
border-style: solid; | ||
border-color: #cc1f1a; | ||
} | ||
|
||
.error-banner .field-errors.filled .field-error { | ||
width: 100%; | ||
font-size: .875rem; | ||
color: #22292f; | ||
line-height: 1.5; | ||
margin-bottom: .5rem; | ||
} | ||
`; | ||
|
||
return postcss([cssnano]) | ||
.process(css, { from: undefined }) | ||
.then((result) => { | ||
expect(result.css).toBe( | ||
'.has-errors .checkbox-inline,.has-errors .checkbox-label,.has-errors .input{background-color:#fcebea;border-color:#cc1f1a}.error-banner .field-errors.filled{background-color:#fcebea;border-bottom:1px;border-color:#cc1f1a;border-style:solid;padding:1.5rem 1.5rem 1rem;width:100%}.error-banner .field-errors.filled .field-error{color:#22292f;font-size:.875rem;line-height:1.5;margin-bottom:.5rem;width:100%}' | ||
); | ||
}); | ||
}); | ||
|
||
test('should merge rules before longhand #3', () => { | ||
const css = ` | ||
.foo { | ||
border-color: #cc1f1a; | ||
} | ||
|
||
.bar { | ||
border-bottom: 1px; | ||
border-color: #cc1f1a; | ||
} | ||
`; | ||
|
||
return postcss([cssnano]) | ||
.process(css, { from: undefined }) | ||
.then((result) => { | ||
expect(result.css).toBe( | ||
'.bar,.foo{border-color:#cc1f1a}.bar{border-bottom:1px}' | ||
); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This output according to the issue #701 (comment) is wrong, not sure why it is wrong in the issue as this is the safe merging in my opinion.
according to the issue, it should be
Though I will check this once again, @evilebottnawi thoughts ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it is bug, need to fix