Image block: Lightbox animation improvements #51721
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,32 +22,43 @@ store( { | |
| core: { | ||
| image: { | ||
| showLightbox: ( { context, event } ) => { | ||
| // We can't initialize the lightbox until the reference | ||
| // image is loaded, otherwise the UX is broken. | ||
| if ( ! context.core.image.imageLoaded ) { | ||
| return; | ||
| } | ||
| context.core.image.initialized = true; | ||
| context.core.image.lastFocusedElement = | ||
| window.document.activeElement; | ||
| context.core.image.scrollDelta = 0; | ||
|
|
||
| context.core.image.lightboxEnabled = true; | ||
| if ( context.core.image.lightboxAnimation === 'zoom' ) { | ||
| setZoomStyles( | ||
| event.target.nextElementSibling, | ||
| context, | ||
| event | ||
| ); | ||
| } | ||
| // Hide overflow only when the animation is in progress, | ||
| // otherwise the removal of the scrollbars will draw attention | ||
| // to itself and look like an error | ||
| document.documentElement.classList.add( | ||
| 'has-lightbox-open' | ||
| ); | ||
|
|
||
| // Since the img is hidden and its src not loaded until | ||
| // the lightbox is opened, let's create an img element on the fly | ||
| // so we can get the dimensions we need to calculate the styles | ||
| context.core.image.preloadInitialized = true; | ||
| const imgDom = document.createElement( 'img' ); | ||
| imgDom.onload = function () { | ||
| context.core.image.activateLargeImage = true; | ||
| }; | ||
| imgDom.setAttribute( | ||
| 'src', | ||
| context.core.image.imageUploadedSrc | ||
| ); | ||
| }, | ||
| hideLightbox: async ( { context, event } ) => { | ||
| context.core.image.hideAnimationEnabled = true; | ||
|
|
@@ -131,9 +142,14 @@ store( { | |
| roleAttribute: ( { context } ) => { | ||
| return context.core.image.lightboxEnabled ? 'dialog' : ''; | ||
| }, | ||
| responsiveImgSrc: ( { context } ) => { | ||
| return context.core.image.activateLargeImage | ||
| ? '' | ||
| : context.core.image.imageCurrentSrc; | ||
| }, | ||
| enlargedImgSrc: ( { context } ) => { | ||
| return context.core.image.initialized | ||
| ? context.core.image.imageUploadedSrc | ||
| : ''; | ||
| }, | ||
| }, | ||
|
|
@@ -142,6 +158,30 @@ store( { | |
| effects: { | ||
| core: { | ||
| image: { | ||
| setCurrentSrc: ( { context, ref } ) => { | ||
| if ( ref.complete ) { | ||
| context.core.image.imageLoaded = true; | ||
| context.core.image.imageCurrentSrc = ref.currentSrc; | ||
| } else { | ||
| ref.addEventListener( 'load', function () { | ||
| context.core.image.imageLoaded = true; | ||
| context.core.image.imageCurrentSrc = | ||
| this.currentSrc; | ||
| } ); | ||
| } | ||
| }, | ||
| preloadLightboxImage: ( { context, ref } ) => { | ||
| ref.addEventListener( 'mouseover', () => { | ||
|
Comment on lines
+173
to
+174
There was a problem hiding this comment. Hey @artemiomorales, out of curiosity, why did you use There was a problem hiding this comment. Thanks for catching this! It's an oversight on my part. Will revise in this upcoming PR. There was a problem hiding this comment. Oh, no problem. Thanks, Artemio. There was a problem hiding this comment. @luisherranz Update: I decided to break this out into a separate PR for easier review and approval. |
||
| if ( ! context.core.image.preloadInitialized ) { | ||
| context.core.image.preloadInitialized = true; | ||
| const imgDom = document.createElement( 'img' ); | ||
| imgDom.setAttribute( | ||
| 'src', | ||
| context.core.image.imageUploadedSrc | ||
| ); | ||
| } | ||
| } ); | ||
| }, | ||
| initLightbox: async ( { context, ref } ) => { | ||
| context.core.image.figureRef = | ||
| ref.querySelector( 'figure' ); | ||
|
|
@@ -163,8 +203,8 @@ store( { | |
| } ); | ||
|
|
||
| function setZoomStyles( imgDom, context, event ) { | ||
| let targetWidth = context.core.image.targetWidth; | ||
| let targetHeight = context.core.image.targetHeight; | ||
|
|
||
| const verticalPadding = 40; | ||
|
|
||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I hope time to interactive does not being affected here too much.
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.
It depends on the user's bandwidth. But also, I think it's reasonable to prevent a user from zooming in on an image that hasn't fully loaded yet? 🤔
Another note: We can likely make the time to interactivity instant with the fade animation; it's just the zoom animation that causes the trouble.
If it's worth exploring, I could make this initialization step specific to the zoom animation.
Another note: Is the concern here that users may want to go immediately to the zoomed image without needing to wait for the rest of the page to load? @mtias has mentioned before the idea of having a separate URL for the zoomed version of the image inside of the lightbox as an overall usability improvement for WordPress sites.
Perhaps we can continue to explore this in a subsequent PR where we start to identify how this would behave, (what should the URL be, how to implement the experience of initializing the page with the lightbox already open, crossover or not with the Media page, etc.)
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.
It is indeed. But, for example, IGDB on their Lightbox, shows a loading text if the image is not fully loaded. Still, is something that we can explore in future PRs.
Yes, this could happen. Usually, clicking on an image and wait for the zoom to appear is not ideal.
We can wait and see how that ticket is evolving.