danilowoz / react-content-loader

⚪ SVG-Powered component to easily create skeleton loadings.
https://skeletonreact.com
MIT License
13.66k stars 417 forks source link

feat(svg): animation alternative #317

Closed danilowoz closed 6 months ago

danilowoz commented 6 months ago

This introduces a new approach to animate SVG gradients and work around the new chromium limitation.

BREAKING CHANGE: The keyTimes, gradientTransform and animateBegin options have been removed.

Preview: https://kn8w9s-6006.csb.app/?path=/story/react-content-loader--animate Close https://github.com/danilowoz/react-content-loader/issues/316

codesandbox[bot] commented 6 months ago

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders
Open Preview

codesandbox-staging[bot] commented 6 months ago

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders
Open Preview

amoshydra commented 6 months ago

Tested in Safari 17.3, the linear gradient does not animate.

In Chrome 122.0.6261.94, this works reasonably well, if not identical. Edit: This also works reasonably well in Firefox 122

tienifr commented 6 months ago

Thanks @danilowoz for the PR

This is working well for me in:

But it's not animating in:

tienifr commented 6 months ago

@danilowoz According to https://stackoverflow.com/a/59932642, using animateTransform won't work for IE and Safari.

I think we can detect IE, Safari by userAgent and use animate for those browsers, that way, we can make sure it works well in all browsers. I'd be happy to contribute the changes for this if you feel that's the direction we can go with 👍

There's a proven, battle-tested way to check for Safari browser (both mobile and desktop) here

amoshydra commented 6 months ago

EDIT: my message below is incorrect


@tienifr instead of user agent detection, we should be able to check for support of gradientTransform like this:

const linearGradient = document.createElementNS("http://www.w3.org/2000/svg", "linearGradient");
console.log(linearGradient.gradientTransform); // return --u-n-d-e-f-i-n-e-d-- on unsupported browser

const canSupportGradientTransform = !!linearGradient.gradientTransform;
Browser ReturnValue
Chrome 122 SVGAnimatedTransformList
Firefox 122 SVGAnimatedTransformList
Safari 17.3 undefined ++SVGAnimatedTransformList*

^ I am not able to test for IE for now.

amoshydra commented 6 months ago

oops! What I've mentioned above is incorrect.

Safari also return SVGAnimatedTransformList. the detection approach above doesn't work.

danilowoz commented 6 months ago

I don't want to rely on the userAgent property because then this component would be client-only. I'm looking for a one-fit-all solution, but I'm running out of ideas :(

danilowoz commented 6 months ago

Maybe we can go for the animateTransform approach:

https://caniuse.com/?search=animateTransform

tienifr commented 6 months ago

Safari will support it soon - it's currently in the preview step;

@danilowoz I think there're certain applications that would need to work well on Safari now.

How about adding an optional useLegacyAnimate prop that will allow using the animate (as currently) instead of animateTransform. Any applications that want to support Safari now will pass useLegacyAnimate as true if they detect the Safari browser. So the logic of detecting browser will reside in the users of the library and not the library itself.

Later once Safari adds support for animateTransform, we can deprecate the useLegacyAnimate prop.

This change will be straight-forward to make and will allow react-content-loader to work on all platforms 👍

abzokhattab commented 6 months ago

Found this article where in the accepted answer the author was able to make the animateTransform work with safari and chrome

danilowoz commented 6 months ago

Ok, I got it running on Safari. Turns out, the linearGradient element requires an initial gradientTransform value to trigger the animation. Plus, I misread the caniuse.com/?search=animateTransform table, and actually, Safari fully supports animateTransform already, so we're good to go.

Considering deprecating the following props:

Another round of review? I try fixing tests meanwhile.

abzokhattab commented 6 months ago

Tried Chrome, Firefox, and Safari, and they all work as expected. Great job @danilowoz

Roeefl commented 6 months ago

@danilowoz

feat(svg): animation alternative #317
+14,239 −64,905 

You crazy bro? 🥲 this is a full blown repo refactor XD

danilowoz commented 6 months ago

Screenshot 2024-03-07 at 13 27 20

The output in NPM will be the same

Roeefl commented 6 months ago

My bad. Just saw a bunch of files, where I was expecting mostly 1 file changed.

Nicely done, great job & thanks for the quick response.

tienifr commented 6 months ago

@danilowoz Works well for me in all browsers too, thanks!!

danilowoz commented 6 months ago

All tests and build passed! I'm going to publish a new major version soon

github-actions[bot] commented 6 months ago

:tada: This PR is included in version 7.0.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: