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

Method supportedValue should support !important property. #214

Open
LvChengbin opened this issue Aug 18, 2020 · 4 comments · May be fixed by #215
Open

Method supportedValue should support !important property. #214

LvChengbin opened this issue Aug 18, 2020 · 4 comments · May be fixed by #215

Comments

@LvChengbin
Copy link 

it( 'should support !important property (solid 1px red)', () => {
    expect( supportedValue( 'border', 'solid 1px red !important' ) ).to.be( 'solid 1px red !important' );
} );

it( 'should support !important property (1px solid red)', () => {
    expect( supportedValue( 'border', '1px solid red !important' ) ).to.be( '1px solid red !important' );
} );

I added these two test cases for supportedValue method, and got the output as follow:

✖ should support !important property (solid 1px red)
✔ should support !important property (1px solid red)

✖ should support !important property (solid 1px red)
        Chrome 86.0.4229.3 (Mac OS 10.15.6)
      Error: expected false to equal 'solid 1px red !important'
          at Assertion.assert (webpack://cssvendor/node_modules/expect.js/index.js:96:13 <- tests.webpack.js:8:10305)
          at Assertion.be.Assertion.equal (webpack://cssvendor/node_modules/expect.js/index.js:216:10 <- tests.webpack.js:8:11808)
          at Assertion.<computed> [as be] (webpack://cssvendor/node_modules/expect.js/index.js:69:24 <- tests.webpack.js:8:9695)
          at Context.<anonymous> (webpack://cssvendor/src/supported-value.test.js:83:81 <- tests.webpack.js:29:255519)

This issue makes using 1px solid red !important and solid 1px red !important has different result in jss. To support !important property or at least to provide same result with these two type of format.

I can send a pull request for this if you think it makes sense.
@kof
Copy link
Member

kof commented Aug 18, 2020

Have you figured out why this happens?

@LvChengbin
Copy link
Author

LvChengbin commented Aug 19, 2020 •
edited

@kof

A values ends with !important cannot be added as a property of el.style directly, The !important should be removed before adding or just to use style.setProperty instead.
https://github.com/cssinjs/css-vendor/blob/master/src/supported-value.js#L67

Using 1px solid red can pass check because the string starts with a number which matches the condition !isNaN(parseInt(prefixedValue, 10))
https://github.com/cssinjs/css-vendor/blob/master/src/supported-value.js#L52

@kof
Copy link
Member

kof commented Aug 20, 2020

@LvChengbin @AleshaOleg I think nothing stops us from using el.style.setProperty instead of el.style.property! A PR with this would be appreciated along with this use case as a test

LvChengbin added a commit to LvChengbin/css-vendor that referenced this issue Aug 23, 2020
@LvChengbin
fixed cssinjs#214
3b28551
@LvChengbin LvChengbin linked a pull request Aug 23, 2020 that will close this issue
Open
@LvChengbin
Copy link
Author

I sent a pull request but failed to pass the test and got error message:

23 08 2020 16:23:19.652:INFO [launcher]: Launching browsers BS_MobileSafari, BS_Opera52, BS_Chrome, BS_Safari, BS_Firefox, BS_InternetExplorer9, BS_InternetExplorer10, BS_InternetExplorer11, BS_Edge13 with concurrency 2
23 08 2020 16:23:19.671:ERROR [launcher]: Cannot load browser "BS_MobileSafari"!
  Error: Password is required.
    at ApiClient.BaseClient (/home/travis/build/cssinjs/css-vendor/node_modules/browserstack/lib/client.js:21:9)
    at ApiClient.ApiBaseClient (/home/travis/build/cssinjs/css-vendor/node_modules/browserstack/lib/api.js:9:13)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Method supportedValue should support !important property. LvChengbin/css-vendor
2 participants
@kof @LvChengbin