search

cssinjs / css-vendor

Runtime vendor prefixing based on feature detection.
MIT License
67 stars 24 forks source link

[WIP] Support transition prefix #6

Closed cvle closed 7 years ago

cvle commented 7 years ago

This PR fixes https://github.com/cssinjs/jss/issues/317 and https://github.com/cssinjs/jss/issues/300 by adding support for prefixing properties inside transition and transition-property.

This PR uses a regular expression to match and replace properties with their prefixed version. Tests were added that should pass in browsers needing to prefix the transform property: Android stock browsers, UC Browser on Android, Safari IOS 8, and IE9.

Note: IE9 doesn't support css transitions, but the tests will pass because the tests only check for correctly prefixing the values.

The test setup is currently not ideal see https://github.com/cssinjs/jss/issues/89. Maybe we should do something like detecting the browser (e.g. using bowser) and giving out only relevant tests.

cvle commented 7 years ago

I realized that supportedValue in its current form is broken, see https://github.com/cssinjs/jss/issues/354. We might need to rethink this.

kof commented 7 years ago

Lets check what others do, for e.g. radium used initially this package as well.

cvle commented 7 years ago

They switched to inline-style-prefixer, as far as I know.

cvle commented 7 years ago

I'll make another revision after https://github.com/cssinjs/css-vendor/pull/8 lands.

kof commented 7 years ago

Now this needs a merge from master

kof commented 7 years ago

@cvle any updates?

cvle commented 7 years ago

I'll update this tomorrow.

cvle commented 7 years ago

While working on this, I got caught up in incorrect property prefixes. I'm currently enhancing our current test suites to test all property vendor prefixes and fix the issues.

kof commented 7 years ago

perfect!

On 30 Nov 2016, at 16:11, Chi Vinh Le notifications@github.com wrote:

While working on this, I got caught up in incorrect property prefixes. I'm currently enhancing our current test suites to test all property vendor prefixes and fix the issues.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

cvle commented 7 years ago

blocking https://github.com/cssinjs/css-vendor/pull/11

kof commented 7 years ago

Is there something I need/can do?

cvle commented 7 years ago

I still need to get #11 done, to have reliable property prefixes which this feature depends on. If you have spare time you can create a PR at the folks at autoprefixer to export the caniuse feature name here https://github.com/postcss/autoprefixer/blob/master/data/prefixes.coffee.

Got a nasty food poisoning last week and just got better. Going through piled up work now. Will continue the work here soon too.

kof commented 7 years ago

Don't worry its all fine, just wanted to know if I am blocking something. We are in the middle of 3 parallel big things in general, this one, new site and almost finished big core refactoring with flowtype and jss-global.

AleshaOleg commented 7 years ago

@cvle hey, could you explain if transition it's a property - why you changing supported-value.js file?

cvle commented 7 years ago

@AleshaOleg Yes because you need to prefix the properties that are inside the value e.g. it could be transition: transform(...) and in some browsers this should become -webkit-transition: -webkit-transform(...) and in others it should transition: -webkit-transform(...), while some accept everything unprefixed.

AleshaOleg commented 7 years ago

@cvle hey, thanks for your support! @cvle, @kof could you review my PR please? https://github.com/cssinjs/css-vendor/pull/6

kof commented 7 years ago

@AleshaOleg we already handled this one in the other PR, right? can be closed?

AleshaOleg commented 7 years ago

@kof yes, this one implemented in #26