-
Notifications
You must be signed in to change notification settings - Fork 42
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
Add transitions with view transitions API #52
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make the data-wp-key
change and this should be ready to go 🙂
wpmovies.php
Outdated
$p->seek( 'parent' ); | ||
$data_key = ''; | ||
if ( $p->next_tag( 'img' ) ) { | ||
$data_key = $p->get_attribute( 'data-key' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to change this too.
I made the changes suggested. There is one issue though. It seems that the JS file is being fetched each time I visit a movie. I made a quick video showing what I mean: Extra.scripts.mp4 |
That's because you have the "Disable cache" option enabled, but that option is always off when the devtools are closed. |
This reverts commit a5d70f0.
I'm not sure about this. Right now, fetching the script multiple times throws a console.log error. And I can see that error even if I have the devtools closed. I made a quick video explaining it: https://www.loom.com/share/eb25392388a749f98d31c4c1844623e4?sid=4ed71037-4a4e-40eb-ad41-862f0f4a1fd4 |
Then maybe there's a problem with the way the router is loading the scripts during the client-side navigation? I'm not familiar with that logic, so @michalczaplinski, could you please take a look? Thanks! 🙂 |
Sure thing, I'll take a look on Monday! |
@SantosGuillamot I took a look at the issue but I'm getting the error:
I'm not sure why it doesn't fail for you as we need to export the (sorry for the bad audio) 2023-07-10_16-51-44.mp4 |
@michalczaplinski, you need to use this "coupled" PR: |
@DAreRodz and I looked at this problem, and we found the culprit. The We've tried a quick fix by adding 2023-07-13_11-41-35.mp4 |
So, if I understood it correctly, it is not a problem with the movies implementation but with client-side navigation, right? Should we open a new issue for this and discuss it there? |
Yes, that's right! @SantosGuillamot I have now updated the video with better-quality audio, btw. I'll open a new issue in https://github.com/WordPress/block-interactivity-experiments |
Opened the issue: WordPress/block-interactivity-experiments#242 |
Coupled with this other pull request, we added a new experimental directive to add a transition between featured images using the View Transitions API. Here you have a video with the explanation:
https://www.loom.com/share/39631b65aebf42b386f0fee14e0b3f58?sid=fd5f26d5-1105-43ab-87a0-d66467e6ebff
Basically, what I did:
viewTransitionsAPI
in thewp-link
directive. If it is set to true, the viewTransitionsAPI is enabled while navigating. This already enables all the transitions that can be achieved only with CSS.data-wp-transition
directive:There are some aspects to review, although it is already working more or less fine for the movies: