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.

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

Allow fallback animations on html element #8258

Merged
merged 3 commits into from
Aug 28, 2023
Merged

Allow fallback animations on html element #8258

merged 3 commits into from
Aug 28, 2023

Conversation

matthewp
Copy link
Contributor

Changes

  • Allows animations on the html element by fixing the selector so it targets both children and the root itself.
  • Simplifies the fallback logic. Previous was not working in Safari because animationstart did not fire before the setTimeout.
  • Removed opacity: 0 for the none animation as that causes the element to be invisible.

Testing

Safari.mp4

Docs

N/A

@changeset-bot
Copy link

changeset-bot bot commented Aug 28, 2023

🦋 Changeset detected

Latest commit: 00dd8ee

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Aug 28, 2023
// Trigger the animations
document.documentElement.dataset.astroTransitionFallback = 'old';
const finished = Promise.all(document.getAnimations().map(a => a.finished));
Copy link
Member

Choose a reason for hiding this comment

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

Nice! This is the API I was talking about. Support matrix looks good to me.

getAnimations support matrix

sheet.addAnimationRaw('old', 'animation: none; opacity: 0; mix-blend-mode: normal;');
sheet.addAnimationRaw('old', 'animation: none; mix-blend-mode: normal;');
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I might be wrong but I from my testing the opacity: 0 was needed because of stacking when using view transitions (not the fallback). This should be handled by mix-blend-mode: normal though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah i wanted to get your opinion on this. I thought it was for that reason. Isn't the reason to do this so you can see the new element which is behind it? But for none is that necessary as both elements are the same?

I removed this because it makes it so that in fallback mode the element is invisible until both animations are complete. We might need a mechanism for VT only styles.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changeed it with this: 00dd8ee

Copy link
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

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

Nice, looks great!

@matthewp matthewp merged commit 1db4e92 into main Aug 28, 2023
13 checks passed
@matthewp matthewp deleted the fallbackanim branch August 28, 2023 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants