Fix image lightbox issues in new full client-side navigation logic#70416
Conversation
|
Size Change: -18 B (0%) Total Size: 1.86 MB
ℹ️ View Unchanged
|
luisherranz
left a comment
There was a problem hiding this comment.
LGTM, but I'll leave @DAreRodz to do the final review.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
DAreRodz
left a comment
There was a problem hiding this comment.
LGTM!
Just a nitpick - I would have set the router region ID to core/image-lightbox-overlay, to be a bit more specific. But it can be merged as it is. 🙂
Co-authored-by: Luis Herranz <luisherranz@gmail.com>
52c9010 to
44bcdbf
Compare
…ordPress#70416) * Add `data-wp-key` * Use `state.overlayOpened` for closing animation * Add `data-wp-router-region` * Update overlay region id --------- Co-authored-by: SantosGuillamot <santosguillamot@git.wordpress.org> Co-authored-by: luisherranz <luisherranz@git.wordpress.org> Co-authored-by: DAreRodz <darerodz@git.wordpress.org>
Warning
This PR is built on top of #70353, which includes the new logic for client-side navigation. The issues it solves are pretty hard to replicate in existing region-based navigation.
For testing purposes, in the mentioned branch, the full client-side navigation can be enabled using a filter like this:
What?
It solves a bunch of issues related to client-side navigation and the image lightbox.
The lightbox closing animation triggers when navigating to a page with the lightbox enabled.
Closing.animation.issue.mp4
Testing instructions
How?
It changes the
hideclass to rely onstate.overlayOpenedinstead ofstate.showClosingAnimation, and I've removed that property. According to this comment, it was added because the animation was triggered the first time the page was loaded, but I have been testing it and I wasn't able to reproduce it in any browser.CSN breaks after going from an empty query loop to a page with lightbox enabled.
Broken.CSN.mp4
Testing instructions
http://localhost:8888/?query-1-page=999.How?
Using the
data-wp-keydirective solves this issue.Lightbox triggers incorrect image when opening and closing multiple images in a query loop.
Query.gutenberg.-.13.June.2025.mp4
Testing instructions
How?
Using the
data-wp-keydirective solves this issue.Right now, it is really hard to replicate these issues with region-based client-side navigation because Post Content is not supported and it is tricky to replicate having different images in the query loop.
However, I added a
data-wp-router-regionto ensure this is not a problem in the future. This will also need some changes to the router JS logic that @DAreRodz will handle in the original PR.