Skip to content

Conversation

@nolanlawson
Copy link
Contributor

Improve whitespace handling around : and in ! important.

Copy link
Member

@jmsjtu jmsjtu left a comment

Choose a reason for hiding this comment

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

Left a comment but otherwise, LGTM!

Comment on lines +23 to +26
.split(':')
// remove whitespace before `important`, e.g. `! important` -> `!important`
.map((_) => _.trim().replace(/!\simportant/, '!important'))
.join(': '),
Copy link
Member

@jmsjtu jmsjtu Apr 5, 2024

Choose a reason for hiding this comment

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

Suggested change
.split(':')
// remove whitespace before `important`, e.g. `! important` -> `!important`
.map((_) => _.trim().replace(/!\simportant/, '!important'))
.join(': '),
.split(':')
// remove whitespace before `important`, e.g. `! important` -> `!important`
.map((_) => _.trim().replace(/!\simportant/, '!important'))
.filter(Boolean)
.join(': '),

In case there's duplicate ':' or the string is empty after splitting, like this background-color :: blue.

Could you add a test case for it too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be invalid CSS; I don't think we need to do any special handling for that. If it's invalid, it'll be formatted weirdly. 🙂

@nolanlawson nolanlawson merged commit 8bab5bf into jtu/normalize-style-attr Apr 5, 2024
@nolanlawson nolanlawson deleted the nolan/normalize-style-attr branch April 5, 2024 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants