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.

Sign up for GitHub

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

Jump to bottom

fix(postcss-ordered-values): preserve columns count #1144

Merged
merged 2 commits into from Jun 7, 2021
Merged

fix(postcss-ordered-values): preserve columns count #1144

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bb8ee6e
fix(postcss-ordered-values): preserve columns count
ludofischer Jun 5, 2021
1193bec
Merge branch 'master' into fix-columns
ludofischer Jun 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions packages/postcss-ordered-values/src/__tests__/index.js
View file
Open in desktop
Expand Up @@ -590,6 +590,24 @@ test(
)
);

test(
'should order columns declarations width first',
processCSS('h1 {columns: 2 20px;}', 'h1 {columns: 20px 2;}')
);

test(
'should preserve already ordered columns declaration',
passthroughCSS('h1 {columns: 20px 2;}')
);

test(
'should not crash on invalid columns declarations',
processCSS(
'h1 {columns: 2px 2px; columns: inherit 3rem; columns: 3rem 2 12em;}',
'h1 {columns: 2px 2px; columns: 3rem inherit; columns: 3rem 2 12em;}'
)
);

test(
'should order column-rule like border',
processCSS(
Expand Down
30 changes: 16 additions & 14 deletions packages/postcss-ordered-values/src/rules/columns.js
View file
Open in desktop
@@ -1,28 +1,30 @@
import { unit } from 'postcss-value-parser';
import border from './border.js';

const strValues = ['auto', 'inherit', 'unset', 'initial'];
function hasUnit(value) {
const parsedVal = unit(value);
return parsedVal && parsedVal.unit !== '';
}

export const column = (columns) => {
let newValue = { front: '', back: '' };
let shouldNormalize = false;
const widths = [];
const other = [];
columns.walk((node) => {
const { type, value } = node;
if (type === 'word' && strValues.indexOf(value.toLowerCase())) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what the old code is trying to do. This seems a strange way to check if

value.toLowerCase() !== 'auto'

(since it the only case where indexOf returns 0 )

const parsedVal = unit(value);
if (parsedVal.unit !== '') {
// surely its the column's width
// it needs to be at the front
newValue.front = `${newValue.front} ${value}`;
shouldNormalize = true;
if (type === 'word') {
if (hasUnit(value)) {
widths.push(value);
} else {
other.push(value);
}
} else if (type === 'word') {
newValue.back = `${newValue.back} ${value}`;
}
});
if (shouldNormalize) {
return `${newValue.front.trimStart()} ${newValue.back.trimStart()}`;

// only transform if declaration is not invalid or a single value
if (other.length === 1 && widths.length === 1) {
return `${widths[0].trimStart()} ${other[0].trimStart()}`;
}

return columns;
};

Expand Down