Skip to content
This repository has been archived by the owner on Apr 6, 2021. It is now read-only.

Fix @apply of animation utilities #150

Merged
merged 3 commits into from
Mar 24, 2021
Merged

Fix @apply of animation utilities #150

merged 3 commits into from
Mar 24, 2021

Conversation

thecrypticace
Copy link
Collaborator

Fixes #94

These are now in the order of the apply declaration but still obey the cascade order so pt-/pb- still wins over py- and py still wins over p-
I think this logic is too specific but it seems to work for now
Copy link
Member

@RobinMalfait RobinMalfait left a comment

Choose a reason for hiding this comment

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

Hey! Thank you for your PR!
Much appreciated! 🙏

It's looking very promising, just found a little bug!
If you like, I can update the PR for you, but just wanted to let you know about what I found.


root.walkRules((rule) => {
rule.selector = replaceSelector(apply.parent.selector, rule.selector, applyCandidate)
if (canRewriteSelector) {
Copy link
Member

Choose a reason for hiding this comment

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

This is looking good, however I think we can improve this a little bit.
Inside keyframes, you are not allowed to have !important, this is invalid:

 @keyframes spin {
   to {
     transform: rotate(360deg) !important;
   }
 }

But since the if (canRewriteSelector) is only wrapping the rule.selector = ... change, this walkDecls to change the important will still happen.

We can fix that, and we can also skip the whole root.walkRules I think:

if (canRewriteSelector) {
  root.walkRules((rule) => {
    rule.selector = replaceSelector(
      apply.parent.selector,
      rule.selector,
      applyCandidate
    )

    rule.walkDecls((d) => {
      d.important = important
    })
  })
}

You can see this behaviour when you do this:

.foo {
  @apply animate-spin !important;
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You know I thought about this — I wasn't sure if that was invalid or not so I left it just in case. I've updated the implementation.

Copy link
Member

Choose a reason for hiding this comment

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

Perfect, thanks!

@RobinMalfait RobinMalfait merged commit 602e6b0 into main Mar 24, 2021
@RobinMalfait RobinMalfait deleted the fix/animation-apply branch March 24, 2021 12:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keyframes malformed when using animation utilities with @apply
2 participants