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

fix: support value !important; #289

Merged
merged 5 commits into from Apr 23, 2022

Conversation

SukkaW
Copy link
Contributor

@SukkaW SukkaW commented Apr 21, 2022

Fix an edge case that position:sticky!important can be prefixed while position:sticky !important (a white space before !important) can not.

@coveralls
Copy link

coveralls commented Apr 21, 2022

Pull Request Test Coverage Report for Build 0fe902e46c8f0c4f8fb719f2593a0295c729c11d-PR-289

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.002%) to 98.833%

Totals Coverage Status
Change from base Build c3c05cd233b50a72c146c2dbf1d5513cf61b0c94: 0.002%
Covered Lines: 271
Relevant Lines: 273

💛 - Coveralls

src/Prefixer.js Outdated Show resolved Hide resolved
@SukkaW SukkaW requested a review from Andarist April 21, 2022 09:01
Comment on lines -108 to -116
// (s)ticky?
if (charat(value, length + 1) !== 115)
break
// stick(y)?
if (charat(value, length + 6) === 121)
return replace(value, ':', ':' + WEBKIT) + value
break
// display: (flex|inline-flex|grid|inline-grid)
case 6444:
switch (charat(value, strlen(value) - 3 - (~indexof(value, '!important') && 10))) {
// stic(k)y
case 107:
return replace(value, ':', ':' + WEBKIT) + value
Copy link
Contributor Author

@SukkaW SukkaW Apr 21, 2022

Choose a reason for hiding this comment

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

Instead of letting position: sticky fallthrough to display and handle it there, the change now handles it in position directly, and either break or return the prefixed to stop it from being fallthrough.

Use stick(y) instead of (s)ticky to handle position: static properly.

Comment on lines +115 to +117
// (inline-)?fle(x)
case 120:
return replace(value, /(.+:)([^;\s!]+)(;|(\s+)?!.+)?/, '$1' + WEBKIT + (charat(value, 14) === 45 ? 'inline-' : '') + 'box$3' + '$1' + WEBKIT + '$2$3' + '$1' + MS + '$2box$3') + value
Copy link
Contributor Author

Choose a reason for hiding this comment

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

display:(f)lex
display:(f)low

display:f(l)ex
display:f(l)ow
display:b(l)ock

display:gr(i)d
display:in(i)tial

So the fourth letter is being used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Related test cases have been added.

@@ -1,9 +1,14 @@
import {compile, serialize, stringify, middleware, prefixer, prefix} from "../index.js"

const globalCssValues = ['inherit', 'initial', 'unset', 'revert', 'revert-layer']
Copy link
Contributor Author

@SukkaW SukkaW Apr 21, 2022

Choose a reason for hiding this comment

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

Copy link
Owner

Choose a reason for hiding this comment

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

Can inline it in the single test case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thysultan globalCssValues is being used twice (That's why I extracted it in the first place). One for display and one for position. And in the future PR I will add more test cases related with it.

@thysultan thysultan merged commit 8a42669 into thysultan:master Apr 23, 2022
@thysultan
Copy link
Owner

As always, thanks: published 4.1.1 with both of these.

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.

None yet

4 participants