Fix for opaque keyframes as object values #1414
Conversation
💥 No ChangesetLatest commit: e45ae4e Merging this PR will not cause any packages to be released. If these changes should not cause updates to packages in this repo, this is fine 🙂 If these changes should be published to npm, you need to add a changeset. Click here to learn what changesets are, and how to add one. |
| ) | ||
| } | ||
| }, | ||
| 'object with animationName and string keyframes as value': { |
There was a problem hiding this comment.
this is a test covering previously uncovered (but working) situation
| @@ -32,7 +32,7 @@ let processStyleValue = ( | |||
| case 'animation': | |||
| case 'animationName': { | |||
| if (typeof value === 'string') { | |||
| value = value.replace(animationRegex, (match, p1, p2) => { | |||
| return value.replace(animationRegex, (match, p1, p2) => { | |||
Codecov Report
|
Andarist
force-pushed
the
fix/interpolated-opaque-keyframes
branch
from
6de7aec
to
d846ede
Compare
| )}}` | ||
| ) | ||
| switch (key) { | ||
| case 'animation': |
There was a problem hiding this comment.
This case will never happen and because of that, let's change this to an if (key === 'animationName') else
packages/core/__tests__/keyframes.js
Outdated
| return ( | ||
| <div | ||
| css={{ | ||
| animation |
There was a problem hiding this comment.
This won't do anything, you need to at least also specify the duration of the animation. (hence the previous comment)
There was a problem hiding this comment.
I can add animationDuration here to reflect better real-world scenario (EDIT: done), but unfortunately animation can contain just an animation name and it's a valid CSS:
https://codesandbox.io/s/sad-cookies-oj0wk
Andarist
force-pushed
the
fix/interpolated-opaque-keyframes
branch
from
d846ede
to
e45ae4e
Compare
fixes #1259
Let's discuss the fix first, going to add changeset later.