From 2956be68bd813ace86e94d8bad0e4c6c59d2e6c3 Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Tue, 10 Oct 2023 10:29:47 -0500 Subject: [PATCH 01/20] Reimplement lightbox trigger as a minimal button in corner of image --- packages/block-library/src/image/index.php | 11 +++- packages/block-library/src/image/style.scss | 29 +++++++-- packages/block-library/src/image/view.js | 66 +-------------------- 3 files changed, 35 insertions(+), 71 deletions(-) diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php index e1f71964622c0..ce33f3b429c54 100644 --- a/packages/block-library/src/image/index.php +++ b/packages/block-library/src/image/index.php @@ -167,6 +167,7 @@ function block_core_image_render_lightbox( $block_content, $block ) { $w->next_tag( 'figure' ); $w->add_class( 'wp-lightbox-container' ); $w->set_attribute( 'data-wp-interactive', true ); + $w->set_attribute( 'data-wp-on--click', 'actions.core.image.showLightbox' ); $w->set_attribute( 'data-wp-context', @@ -199,7 +200,6 @@ function block_core_image_render_lightbox( $block_content, $block ) { $w->next_tag( 'img' ); $w->set_attribute( 'data-wp-init', 'effects.core.image.setCurrentSrc' ); $w->set_attribute( 'data-wp-on--load', 'actions.core.image.handleLoad' ); - $w->set_attribute( 'data-wp-effect', 'effects.core.image.setButtonStyles' ); $w->set_attribute( 'data-wp-effect--setStylesOnResize', 'effects.core.image.setStylesOnResize' ); $body_content = $w->get_updated_html(); @@ -218,7 +218,14 @@ function block_core_image_render_lightbox( $block_content, $block ) { data-wp-style--height="context.core.image.imageButtonHeight" data-wp-style--left="context.core.image.imageButtonLeft" data-wp-style--top="context.core.image.imageButtonTop" - >'; + > + + + + + + + '; $body_content = preg_replace( '/]+>/', $button, $body_content ); diff --git a/packages/block-library/src/image/style.scss b/packages/block-library/src/image/style.scss index 2ef602982e57b..d8dff375911fb 100644 --- a/packages/block-library/src/image/style.scss +++ b/packages/block-library/src/image/style.scss @@ -156,15 +156,28 @@ position: relative; display: flex; flex-direction: column; + cursor: zoom-in; + + &:hover { + button { + opacity: 1; + } + } button { + opacity: 0; border: none; - background: none; + background: #1e1e1e; cursor: zoom-in; - width: 100%; - height: 100%; + width: 25px; + height: 25px; position: absolute; z-index: 100; + top: 5px; + right: 5px; + text-align: center; + padding: 0; + border-radius: 10%; &:focus-visible { outline: 5px auto #212121; @@ -172,10 +185,18 @@ outline-offset: 5px; } + &:hover { + cursor: pointer; + } + + &:focus { + opacity: 1; + } + &:hover, &:focus, &:not(:hover):not(:active):not(.has-background) { - background: none; + background: #1e1e1e; border: none; } } diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js index 3f2242ad737f0..22cb4aa01019d 100644 --- a/packages/block-library/src/image/view.js +++ b/packages/block-library/src/image/view.js @@ -191,14 +191,9 @@ store( } } }, - handleLoad: ( { state, context, effects, ref } ) => { + handleLoad: ( { context, ref } ) => { context.core.image.imageLoaded = true; context.core.image.imageCurrentSrc = ref.currentSrc; - effects.core.image.setButtonStyles( { - state, - context, - ref, - } ); }, handleTouchStart: () => { isTouching = true; @@ -282,65 +277,6 @@ store( ref.querySelector( '.close-button' ).focus(); } }, - setButtonStyles: ( { state, context, ref } ) => { - const { - naturalWidth, - naturalHeight, - offsetWidth, - offsetHeight, - } = ref; - - // If the image isn't loaded yet, we can't - // calculate how big the button should be. - if ( naturalWidth === 0 || naturalHeight === 0 ) { - return; - } - - // Subscribe to the window dimensions so we can - // recalculate the styles if the window is resized. - if ( - ( state.core.image.windowWidth || - state.core.image.windowHeight ) && - context.core.image.scaleAttr === 'contain' - ) { - // In the case of an image with object-fit: contain, the - // size of the img element can be larger than the image itself, - // so we need to calculate the size of the button to match. - - // Natural ratio of the image. - const naturalRatio = naturalWidth / naturalHeight; - // Offset ratio of the image. - const offsetRatio = offsetWidth / offsetHeight; - - if ( naturalRatio > offsetRatio ) { - // If it reaches the width first, keep - // the width and recalculate the height. - context.core.image.imageButtonWidth = - offsetWidth; - const buttonHeight = offsetWidth / naturalRatio; - context.core.image.imageButtonHeight = - buttonHeight; - context.core.image.imageButtonTop = - ( offsetHeight - buttonHeight ) / 2; - } else { - // If it reaches the height first, keep - // the height and recalculate the width. - context.core.image.imageButtonHeight = - offsetHeight; - const buttonWidth = offsetHeight * naturalRatio; - context.core.image.imageButtonWidth = - buttonWidth; - context.core.image.imageButtonLeft = - ( offsetWidth - buttonWidth ) / 2; - } - } else { - // In all other cases, we can trust that the size of - // the image is the right size for the button as well. - - context.core.image.imageButtonWidth = offsetWidth; - context.core.image.imageButtonHeight = offsetHeight; - } - }, setStylesOnResize: ( { state, context, ref } ) => { if ( context.core.image.lightboxEnabled && From 5a084d4cf424e64c39cb3f3949332b5a4d15f404 Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Tue, 10 Oct 2023 18:00:06 -0500 Subject: [PATCH 02/20] Remove obsolete directives --- packages/block-library/src/image/index.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php index ce33f3b429c54..2e3807c1539a5 100644 --- a/packages/block-library/src/image/index.php +++ b/packages/block-library/src/image/index.php @@ -214,10 +214,6 @@ function block_core_image_render_lightbox( $block_content, $block ) { aria-haspopup="dialog" aria-label="' . esc_attr( $aria_label ) . '" data-wp-on--click="actions.core.image.showLightbox" - data-wp-style--width="context.core.image.imageButtonWidth" - data-wp-style--height="context.core.image.imageButtonHeight" - data-wp-style--left="context.core.image.imageButtonLeft" - data-wp-style--top="context.core.image.imageButtonTop" > From c264096fac2401a54c49a58ec1de5283c31ecb5d Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Tue, 10 Oct 2023 18:09:18 -0500 Subject: [PATCH 03/20] Update directives to fire logic properly via image or button click --- packages/block-library/src/image/index.php | 4 +-- packages/block-library/src/image/view.js | 33 ++++++++++++++++++---- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php index 2e3807c1539a5..57c977c7d2967 100644 --- a/packages/block-library/src/image/index.php +++ b/packages/block-library/src/image/index.php @@ -167,7 +167,6 @@ function block_core_image_render_lightbox( $block_content, $block ) { $w->next_tag( 'figure' ); $w->add_class( 'wp-lightbox-container' ); $w->set_attribute( 'data-wp-interactive', true ); - $w->set_attribute( 'data-wp-on--click', 'actions.core.image.showLightbox' ); $w->set_attribute( 'data-wp-context', @@ -200,6 +199,7 @@ function block_core_image_render_lightbox( $block_content, $block ) { $w->next_tag( 'img' ); $w->set_attribute( 'data-wp-init', 'effects.core.image.setCurrentSrc' ); $w->set_attribute( 'data-wp-on--load', 'actions.core.image.handleLoad' ); + $w->set_attribute( 'data-wp-on--click', 'actions.core.image.callShowLightboxFromImage' ); $w->set_attribute( 'data-wp-effect--setStylesOnResize', 'effects.core.image.setStylesOnResize' ); $body_content = $w->get_updated_html(); @@ -213,7 +213,7 @@ function block_core_image_render_lightbox( $block_content, $block ) { type="button" aria-haspopup="dialog" aria-label="' . esc_attr( $aria_label ) . '" - data-wp-on--click="actions.core.image.showLightbox" + data-wp-on--click="actions.core.image.callShowLightboxFromButton" > diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js index 22cb4aa01019d..d18c6e2091249 100644 --- a/packages/block-library/src/image/view.js +++ b/packages/block-library/src/image/view.js @@ -93,7 +93,7 @@ store( actions: { core: { image: { - showLightbox: ( { context, event } ) => { + showLightbox: ( { context }, image ) => { // We can't initialize the lightbox until the reference // image is loaded, otherwise the UX is broken. if ( ! context.core.image.imageLoaded ) { @@ -105,10 +105,7 @@ store( context.core.image.scrollDelta = 0; context.core.image.lightboxEnabled = true; - setStyles( - context, - event.target.previousElementSibling - ); + setStyles( context, image ); context.core.image.scrollTopReset = window.pageYOffset || @@ -137,6 +134,32 @@ store( false ); }, + callShowLightboxFromImage: ( { + context, + event, + actions, + } ) => { + actions.core.image.showLightbox( + { + context, + event, + }, + event.target + ); + }, + callShowLightboxFromButton: ( { + context, + event, + actions, + } ) => { + actions.core.image.showLightbox( + { + context, + event, + }, + event.target.parentElement.previousElementSibling + ); + }, hideLightbox: async ( { context } ) => { context.core.image.hideAnimationEnabled = true; if ( context.core.image.lightboxEnabled ) { From 6302aea3f044c08b38ffb2bf8b6ef0c58f8ba11b Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Tue, 10 Oct 2023 18:11:13 -0500 Subject: [PATCH 04/20] Ensure lightbox button only appears when hovering over image, not whole figure --- packages/block-library/src/image/index.php | 3 +++ packages/block-library/src/image/style.scss | 12 +++++++----- packages/block-library/src/image/view.js | 6 ++++++ 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php index 57c977c7d2967..dcbbf79d59516 100644 --- a/packages/block-library/src/image/index.php +++ b/packages/block-library/src/image/index.php @@ -200,6 +200,8 @@ function block_core_image_render_lightbox( $block_content, $block ) { $w->set_attribute( 'data-wp-init', 'effects.core.image.setCurrentSrc' ); $w->set_attribute( 'data-wp-on--load', 'actions.core.image.handleLoad' ); $w->set_attribute( 'data-wp-on--click', 'actions.core.image.callShowLightboxFromImage' ); + $w->set_attribute( 'data-wp-on--mouseover', 'actions.core.image.handleMouseOver' ); + $w->set_attribute( 'data-wp-on--mouseout', 'actions.core.image.handleMouseOut' ); $w->set_attribute( 'data-wp-effect--setStylesOnResize', 'effects.core.image.setStylesOnResize' ); $body_content = $w->get_updated_html(); @@ -214,6 +216,7 @@ function block_core_image_render_lightbox( $block_content, $block ) { aria-haspopup="dialog" aria-label="' . esc_attr( $aria_label ) . '" data-wp-on--click="actions.core.image.callShowLightboxFromButton" + data-wp-class--show="context.core.image.isHovering" > diff --git a/packages/block-library/src/image/style.scss b/packages/block-library/src/image/style.scss index d8dff375911fb..4c942a14ab6cc 100644 --- a/packages/block-library/src/image/style.scss +++ b/packages/block-library/src/image/style.scss @@ -156,12 +156,9 @@ position: relative; display: flex; flex-direction: column; - cursor: zoom-in; - &:hover { - button { - opacity: 1; - } + img { + cursor: zoom-in; } button { @@ -179,6 +176,10 @@ padding: 0; border-radius: 10%; + &.show { + opacity: 1; + } + &:focus-visible { outline: 5px auto #212121; outline: 5px auto -webkit-focus-ring-color; @@ -187,6 +188,7 @@ &:hover { cursor: pointer; + opacity: 1; } &:focus { diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js index d18c6e2091249..962fef0e930f4 100644 --- a/packages/block-library/src/image/view.js +++ b/packages/block-library/src/image/view.js @@ -182,6 +182,12 @@ store( } ); } }, + handleMouseOver( { context } ) { + context.core.image.isHovering = true; + }, + handleMouseOut( { context } ) { + context.core.image.isHovering = false; + }, handleKeydown: ( { context, actions, event } ) => { if ( context.core.image.lightboxEnabled ) { if ( event.key === 'Tab' || event.keyCode === 9 ) { From de70bcb5469a15aca6dddb95b9e044efb1dfff84 Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Tue, 10 Oct 2023 18:33:53 -0500 Subject: [PATCH 05/20] Ensure close button does not receive focus when opening lightbox via mouse --- packages/block-library/src/image/view.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js index 962fef0e930f4..d65d048af58da 100644 --- a/packages/block-library/src/image/view.js +++ b/packages/block-library/src/image/view.js @@ -93,7 +93,7 @@ store( actions: { core: { image: { - showLightbox: ( { context }, image ) => { + showLightbox: ( { context, event }, image ) => { // We can't initialize the lightbox until the reference // image is loaded, otherwise the UX is broken. if ( ! context.core.image.imageLoaded ) { @@ -103,6 +103,7 @@ store( context.core.image.lastFocusedElement = window.document.activeElement; context.core.image.scrollDelta = 0; + context.core.image.pointerType = event.pointerType; context.core.image.lightboxEnabled = true; setStyles( context, image ); @@ -303,7 +304,9 @@ store( focusableElements.length - 1 ]; - ref.querySelector( '.close-button' ).focus(); + if ( context.core.image.pointerType !== 'mouse' ) { + ref.querySelector( '.close-button' ).focus(); + } } }, setStylesOnResize: ( { state, context, ref } ) => { From 25a91bc290f46b1093a223e33b329200c7fe3075 Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Tue, 10 Oct 2023 18:56:28 -0500 Subject: [PATCH 06/20] Ensure button only receives focus when lightbox is closed via keyboard --- packages/block-library/src/image/view.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js index d65d048af58da..4985267e67788 100644 --- a/packages/block-library/src/image/view.js +++ b/packages/block-library/src/image/view.js @@ -161,7 +161,7 @@ store( event.target.parentElement.previousElementSibling ); }, - hideLightbox: async ( { context } ) => { + hideLightbox: async ( { context, event } ) => { context.core.image.hideAnimationEnabled = true; if ( context.core.image.lightboxEnabled ) { // We want to wait until the close animation is completed @@ -178,9 +178,12 @@ store( }, 450 ); context.core.image.lightboxEnabled = false; - context.core.image.lastFocusedElement.focus( { - preventScroll: true, - } ); + + if ( event.pointerType !== 'mouse' ) { + context.core.image.lastFocusedElement.focus( { + preventScroll: true, + } ); + } } }, handleMouseOver( { context } ) { From 994e04c53811832353822366036e0b85e6403f59 Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Tue, 10 Oct 2023 18:57:15 -0500 Subject: [PATCH 07/20] Add comments --- packages/block-library/src/image/index.php | 3 +++ packages/block-library/src/image/view.js | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php index dcbbf79d59516..2c9c915951372 100644 --- a/packages/block-library/src/image/index.php +++ b/packages/block-library/src/image/index.php @@ -199,6 +199,9 @@ function block_core_image_render_lightbox( $block_content, $block ) { $w->next_tag( 'img' ); $w->set_attribute( 'data-wp-init', 'effects.core.image.setCurrentSrc' ); $w->set_attribute( 'data-wp-on--load', 'actions.core.image.handleLoad' ); + // We need to set an event callback on the `img` specifically + // because the `figure` element can also contain a caption, and + // we don't want to trigger the lightbox when the caption is clicked. $w->set_attribute( 'data-wp-on--click', 'actions.core.image.callShowLightboxFromImage' ); $w->set_attribute( 'data-wp-on--mouseover', 'actions.core.image.handleMouseOver' ); $w->set_attribute( 'data-wp-on--mouseout', 'actions.core.image.handleMouseOut' ); diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js index 4985267e67788..9063eb9e87b96 100644 --- a/packages/block-library/src/image/view.js +++ b/packages/block-library/src/image/view.js @@ -135,6 +135,9 @@ store( false ); }, + // When opening the lightbox via clicking an image, + // we can use the event target directly and pass it to the + // showLightbox action, which uses it to create the styles. callShowLightboxFromImage: ( { context, event, @@ -148,6 +151,9 @@ store( event.target ); }, + // When opening the lightbox via clicking the button, + // we need to reach into event target's parent element to + // get the image element needed to create the styles. callShowLightboxFromButton: ( { context, event, @@ -158,6 +164,9 @@ store( context, event, }, + // The event target we receive when clicking the button + // is the SVG element inside of it, so we need to go to + // the parent element's sibling to get the image. event.target.parentElement.previousElementSibling ); }, @@ -186,6 +195,9 @@ store( } } }, + // We need to use a handler to know whether the mouse is hovering + // so we know when to show the lightbox trigger button. We are unable + // to use just CSS for this because the button is not a child of the image. handleMouseOver( { context } ) { context.core.image.isHovering = true; }, @@ -307,6 +319,12 @@ store( focusableElements.length - 1 ]; + // We want to avoid drawing unnecessary attention to the + // close button for mouse users. Note that even if opening + // the lightbox via keyboard, the event fired is of type + // `pointerEvent`, so we need to rely on the `event.pointerType` + // property, which returns `mouse` for mouse events and + // as an empty string for keyboard events. if ( context.core.image.pointerType !== 'mouse' ) { ref.querySelector( '.close-button' ).focus(); } From 0e86105566c1ea8308fe90fce05d19fb37567c06 Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Tue, 10 Oct 2023 19:13:13 -0500 Subject: [PATCH 08/20] Prevent unnecessary focus being shown on mobile --- packages/block-library/src/image/view.js | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js index 9063eb9e87b96..5524ca3fff414 100644 --- a/packages/block-library/src/image/view.js +++ b/packages/block-library/src/image/view.js @@ -188,7 +188,11 @@ store( context.core.image.lightboxEnabled = false; - if ( event.pointerType !== 'mouse' ) { + // We want to avoid drawing attention to the button + // after the lightbox closes for mouse and touch users. + // Note that the `event.pointerType` property returns + // as an empty string if a keyboard fired the event. + if ( event.pointerType === '' ) { context.core.image.lastFocusedElement.focus( { preventScroll: true, } ); @@ -319,13 +323,12 @@ store( focusableElements.length - 1 ]; - // We want to avoid drawing unnecessary attention to the - // close button for mouse users. Note that even if opening + // We want to avoid drawing unnecessary attention to the close + // button for mouse and touch users. Note that even if opening // the lightbox via keyboard, the event fired is of type // `pointerEvent`, so we need to rely on the `event.pointerType` - // property, which returns `mouse` for mouse events and - // as an empty string for keyboard events. - if ( context.core.image.pointerType !== 'mouse' ) { + // property, which returns an empty string for keyboard events. + if ( context.core.image.pointerType === '' ) { ref.querySelector( '.close-button' ).focus(); } } From d9357e5f6f15cbd2015136ebfc1e28f587228707 Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Wed, 11 Oct 2023 15:00:15 -0500 Subject: [PATCH 09/20] Add dynamic positioning for button when image uses 'contain' setting --- packages/block-library/src/image/index.php | 3 ++ packages/block-library/src/image/view.js | 48 ++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php index 2c9c915951372..1ca8cb99b0120 100644 --- a/packages/block-library/src/image/index.php +++ b/packages/block-library/src/image/index.php @@ -199,6 +199,7 @@ function block_core_image_render_lightbox( $block_content, $block ) { $w->next_tag( 'img' ); $w->set_attribute( 'data-wp-init', 'effects.core.image.setCurrentSrc' ); $w->set_attribute( 'data-wp-on--load', 'actions.core.image.handleLoad' ); + $w->set_attribute( 'data-wp-effect', 'effects.core.image.setButtonStyles' ); // We need to set an event callback on the `img` specifically // because the `figure` element can also contain a caption, and // we don't want to trigger the lightbox when the caption is clicked. @@ -220,6 +221,8 @@ function block_core_image_render_lightbox( $block_content, $block ) { aria-label="' . esc_attr( $aria_label ) . '" data-wp-on--click="actions.core.image.callShowLightboxFromButton" data-wp-class--show="context.core.image.isHovering" + data-wp-style--right="context.core.image.imageButtonRight" + data-wp-style--top="context.core.image.imageButtonTop" > diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js index 5524ca3fff414..6326241420e7e 100644 --- a/packages/block-library/src/image/view.js +++ b/packages/block-library/src/image/view.js @@ -333,6 +333,54 @@ store( } } }, + setButtonStyles: ( { context, ref } ) => { + const { + naturalWidth, + naturalHeight, + offsetWidth, + offsetHeight, + } = ref; + + // If the image isn't loaded yet, we can't + // calculate where the button should be. + if ( naturalWidth === 0 || naturalHeight === 0 ) { + return; + } + + // In the case of an image with object-fit: contain, the + // size of the element can be larger than the image itself, + // so we need to calculate where to place the button. + if ( context.core.image.scaleAttr === 'contain' ) { + // Natural ratio of the image. + const naturalRatio = naturalWidth / naturalHeight; + // Offset ratio of the image. + const offsetRatio = offsetWidth / offsetHeight; + + if ( naturalRatio > offsetRatio ) { + // If it reaches the width first, use a fixed + // position for the X axis and calculate Y position. + context.core.image.imageButtonRight = 25; + const imageHeight = offsetWidth / naturalRatio; + context.core.image.imageButtonTop = + ( offsetHeight - imageHeight ) / 2 + 25; + } else { + // If it reaches the height first, use a fixed + // position for the Y axis and calculate X position. + context.core.image.imageButtonTop = 25; + const imageWidth = offsetHeight * naturalRatio; + context.core.image.imageButtonRight = + ( offsetWidth - imageWidth ) / 2 + 25; + } + } else { + // TO DO : Add handling for custom widths and heights. + + // In most other cases, we can just put the button in + // the top right corner of the containing element. + + context.core.image.imageButtonTop = 25; + context.core.image.imageButtonRight = 25; + } + }, setStylesOnResize: ( { state, context, ref } ) => { if ( context.core.image.lightboxEnabled && From cea71f1e57712d370e503e5232a806e6fda562ed Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Wed, 11 Oct 2023 16:33:51 -0500 Subject: [PATCH 10/20] WORK IN PROGRESS - Begin accounting for various edge cases We need to account for the fact that an image can have custom dimensions, aspect ratio, cover or contain, captions, thumbnail dimensions, and potentially other scenarios. This commit begins to address those issues but notably fails in cases where one uses a horizontal image, at full scale, with custom aspect ratio, using 'contain'. It seems to work in all other cases that I've checked but needs more thorough testing and the code can probably be cleaner, and may contain some unnecessary items. --- packages/block-library/src/image/view.js | 56 +++++++++++++++++++----- 1 file changed, 45 insertions(+), 11 deletions(-) diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js index 6326241420e7e..24d8cc17980e7 100644 --- a/packages/block-library/src/image/view.js +++ b/packages/block-library/src/image/view.js @@ -347,6 +347,33 @@ store( return; } + const figure = ref.parentElement; + const figureComputedStyle = + window.getComputedStyle( figure ); + + const figureWidth = + ref.parentElement.offsetWidth - + parseFloat( figureComputedStyle.marginLeft ) - + parseFloat( figureComputedStyle.marginRight ); + + // We need special handling for the height because + // a caption will cause the figure to be taller than + // the image, which means we need to account for that + // when calculating the placement of the button in the + // top right corner of the image. + let figureHeight = ref.parentElement.offsetHeight; + const caption = figure.querySelector( 'figcaption' ); + if ( caption ) { + figureHeight = + figureHeight - + parseFloat( figureComputedStyle.marginTop ) - + parseFloat( figureComputedStyle.marginBottom ) - + caption.offsetHeight; + } + + const buttonOffsetTop = figureHeight - offsetHeight; + const buttonOffsetRight = figureWidth - offsetWidth; + // In the case of an image with object-fit: contain, the // size of the element can be larger than the image itself, // so we need to calculate where to place the button. @@ -356,29 +383,36 @@ store( // Offset ratio of the image. const offsetRatio = offsetWidth / offsetHeight; - if ( naturalRatio > offsetRatio ) { + if ( naturalRatio >= offsetRatio ) { // If it reaches the width first, use a fixed // position for the X axis and calculate Y position. - context.core.image.imageButtonRight = 25; + context.core.image.imageButtonRight = + buttonOffsetRight + 25; const imageHeight = offsetWidth / naturalRatio; context.core.image.imageButtonTop = - ( offsetHeight - imageHeight ) / 2 + 25; + ( offsetHeight - imageHeight ) / 2 + + buttonOffsetTop + + 25; } else { // If it reaches the height first, use a fixed // position for the Y axis and calculate X position. - context.core.image.imageButtonTop = 25; + context.core.image.imageButtonTop = + buttonOffsetTop + 25; const imageWidth = offsetHeight * naturalRatio; context.core.image.imageButtonRight = - ( offsetWidth - imageWidth ) / 2 + 25; + ( offsetWidth - imageWidth ) / 2 + + buttonOffsetRight + + 25; } } else { - // TO DO : Add handling for custom widths and heights. - - // In most other cases, we can just put the button in - // the top right corner of the containing element. - + // In other cases, we can just put the button in + // the top right corner of the containing element, + // accounting for the fact that users may have resized + // the image, which will require us to reposition + // the button on the X axis but not the Y axis. context.core.image.imageButtonTop = 25; - context.core.image.imageButtonRight = 25; + context.core.image.imageButtonRight = + buttonOffsetRight + 25; } }, setStylesOnResize: ( { state, context, ref } ) => { From e3c0d003cc603e8c4105c836b40d0ca628cd3a78 Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Mon, 16 Oct 2023 15:34:49 -0500 Subject: [PATCH 11/20] Simplify and improve button placement logic --- packages/block-library/src/image/view.js | 52 ++++++++++++------------ 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js index 24d8cc17980e7..607068b0762bb 100644 --- a/packages/block-library/src/image/view.js +++ b/packages/block-library/src/image/view.js @@ -240,9 +240,13 @@ store( } } }, - handleLoad: ( { context, ref } ) => { + handleLoad: ( { context, effects, ref } ) => { context.core.image.imageLoaded = true; context.core.image.imageCurrentSrc = ref.currentSrc; + effects.core.image.setButtonStyles( { + context, + ref, + } ); }, handleTouchStart: () => { isTouching = true; @@ -348,13 +352,7 @@ store( } const figure = ref.parentElement; - const figureComputedStyle = - window.getComputedStyle( figure ); - - const figureWidth = - ref.parentElement.offsetWidth - - parseFloat( figureComputedStyle.marginLeft ) - - parseFloat( figureComputedStyle.marginRight ); + const figureWidth = ref.parentElement.offsetWidth; // We need special handling for the height because // a caption will cause the figure to be taller than @@ -364,11 +362,13 @@ store( let figureHeight = ref.parentElement.offsetHeight; const caption = figure.querySelector( 'figcaption' ); if ( caption ) { + const captionComputedStyle = + window.getComputedStyle( caption ); figureHeight = figureHeight - - parseFloat( figureComputedStyle.marginTop ) - - parseFloat( figureComputedStyle.marginBottom ) - - caption.offsetHeight; + caption.offsetHeight - + parseFloat( captionComputedStyle.marginTop ) - + parseFloat( captionComputedStyle.marginBottom ); } const buttonOffsetTop = figureHeight - offsetHeight; @@ -384,33 +384,31 @@ store( const offsetRatio = offsetWidth / offsetHeight; if ( naturalRatio >= offsetRatio ) { - // If it reaches the width first, use a fixed - // position for the X axis and calculate Y position. - context.core.image.imageButtonRight = - buttonOffsetRight + 25; - const imageHeight = offsetWidth / naturalRatio; + // If it reaches the width first, keep + // the width and compute the height. + const referenceHeight = + offsetWidth / naturalRatio; context.core.image.imageButtonTop = - ( offsetHeight - imageHeight ) / 2 + + ( offsetHeight - referenceHeight ) / 2 + buttonOffsetTop + 25; + context.core.image.imageButtonRight = + buttonOffsetRight + 25; } else { - // If it reaches the height first, use a fixed - // position for the Y axis and calculate X position. + // If it reaches the height first, keep + // the height and compute the width. + const referenceWidth = + offsetHeight * naturalRatio; context.core.image.imageButtonTop = buttonOffsetTop + 25; - const imageWidth = offsetHeight * naturalRatio; context.core.image.imageButtonRight = - ( offsetWidth - imageWidth ) / 2 + + ( offsetWidth - referenceWidth ) / 2 + buttonOffsetRight + 25; } } else { - // In other cases, we can just put the button in - // the top right corner of the containing element, - // accounting for the fact that users may have resized - // the image, which will require us to reposition - // the button on the X axis but not the Y axis. - context.core.image.imageButtonTop = 25; + context.core.image.imageButtonTop = + buttonOffsetTop + 25; context.core.image.imageButtonRight = buttonOffsetRight + 25; } From 6bae728fec4c1564d4f3268fa0b8dcffe9c3fee6 Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Mon, 16 Oct 2023 15:45:31 -0500 Subject: [PATCH 12/20] Simplify logic to show button on hover --- packages/block-library/src/image/index.php | 3 --- packages/block-library/src/image/style.scss | 8 ++++---- packages/block-library/src/image/view.js | 9 --------- 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php index 1ca8cb99b0120..716f5e093afc8 100644 --- a/packages/block-library/src/image/index.php +++ b/packages/block-library/src/image/index.php @@ -204,8 +204,6 @@ function block_core_image_render_lightbox( $block_content, $block ) { // because the `figure` element can also contain a caption, and // we don't want to trigger the lightbox when the caption is clicked. $w->set_attribute( 'data-wp-on--click', 'actions.core.image.callShowLightboxFromImage' ); - $w->set_attribute( 'data-wp-on--mouseover', 'actions.core.image.handleMouseOver' ); - $w->set_attribute( 'data-wp-on--mouseout', 'actions.core.image.handleMouseOut' ); $w->set_attribute( 'data-wp-effect--setStylesOnResize', 'effects.core.image.setStylesOnResize' ); $body_content = $w->get_updated_html(); @@ -220,7 +218,6 @@ function block_core_image_render_lightbox( $block_content, $block ) { aria-haspopup="dialog" aria-label="' . esc_attr( $aria_label ) . '" data-wp-on--click="actions.core.image.callShowLightboxFromButton" - data-wp-class--show="context.core.image.isHovering" data-wp-style--right="context.core.image.imageButtonRight" data-wp-style--top="context.core.image.imageButtonTop" > diff --git a/packages/block-library/src/image/style.scss b/packages/block-library/src/image/style.scss index 4c942a14ab6cc..71e1b3de2519c 100644 --- a/packages/block-library/src/image/style.scss +++ b/packages/block-library/src/image/style.scss @@ -161,6 +161,10 @@ cursor: zoom-in; } + img:hover + button { + opacity: 1; + } + button { opacity: 0; border: none; @@ -176,10 +180,6 @@ padding: 0; border-radius: 10%; - &.show { - opacity: 1; - } - &:focus-visible { outline: 5px auto #212121; outline: 5px auto -webkit-focus-ring-color; diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js index 607068b0762bb..2621131f32777 100644 --- a/packages/block-library/src/image/view.js +++ b/packages/block-library/src/image/view.js @@ -199,15 +199,6 @@ store( } } }, - // We need to use a handler to know whether the mouse is hovering - // so we know when to show the lightbox trigger button. We are unable - // to use just CSS for this because the button is not a child of the image. - handleMouseOver( { context } ) { - context.core.image.isHovering = true; - }, - handleMouseOut( { context } ) { - context.core.image.isHovering = false; - }, handleKeydown: ( { context, actions, event } ) => { if ( context.core.image.lightboxEnabled ) { if ( event.key === 'Tab' || event.keyCode === 9 ) { From c7bfa37f0166cf66f8006c02d0154569ee844bbd Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Mon, 16 Oct 2023 15:52:17 -0500 Subject: [PATCH 13/20] Fix styles --- packages/block-library/src/image/style.scss | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/block-library/src/image/style.scss b/packages/block-library/src/image/style.scss index 71e1b3de2519c..df70de12fe8b0 100644 --- a/packages/block-library/src/image/style.scss +++ b/packages/block-library/src/image/style.scss @@ -168,10 +168,10 @@ button { opacity: 0; border: none; - background: #1e1e1e; + background: #000; cursor: zoom-in; - width: 25px; - height: 25px; + width: 24px; + height: 24px; position: absolute; z-index: 100; top: 5px; @@ -198,7 +198,7 @@ &:hover, &:focus, &:not(:hover):not(:active):not(.has-background) { - background: #1e1e1e; + background: #000; border: none; } } From bd9d8b82e4e48e1e7019da3b5eee7955ede0b846 Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Mon, 16 Oct 2023 16:12:13 -0500 Subject: [PATCH 14/20] Simplify calls to showLightbox --- packages/block-library/src/image/index.php | 4 +-- packages/block-library/src/image/view.js | 40 ++-------------------- 2 files changed, 5 insertions(+), 39 deletions(-) diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php index 716f5e093afc8..0ca9c8e6afe45 100644 --- a/packages/block-library/src/image/index.php +++ b/packages/block-library/src/image/index.php @@ -203,7 +203,7 @@ function block_core_image_render_lightbox( $block_content, $block ) { // We need to set an event callback on the `img` specifically // because the `figure` element can also contain a caption, and // we don't want to trigger the lightbox when the caption is clicked. - $w->set_attribute( 'data-wp-on--click', 'actions.core.image.callShowLightboxFromImage' ); + $w->set_attribute( 'data-wp-on--click', 'actions.core.image.showLightbox' ); $w->set_attribute( 'data-wp-effect--setStylesOnResize', 'effects.core.image.setStylesOnResize' ); $body_content = $w->get_updated_html(); @@ -217,7 +217,7 @@ function block_core_image_render_lightbox( $block_content, $block ) { type="button" aria-haspopup="dialog" aria-label="' . esc_attr( $aria_label ) . '" - data-wp-on--click="actions.core.image.callShowLightboxFromButton" + data-wp-on--click="actions.core.image.showLightbox" data-wp-style--right="context.core.image.imageButtonRight" data-wp-style--top="context.core.image.imageButtonTop" > diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js index 2621131f32777..eac9f8ecdf9e3 100644 --- a/packages/block-library/src/image/view.js +++ b/packages/block-library/src/image/view.js @@ -93,7 +93,7 @@ store( actions: { core: { image: { - showLightbox: ( { context, event }, 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 ) { @@ -106,7 +106,7 @@ store( context.core.image.pointerType = event.pointerType; context.core.image.lightboxEnabled = true; - setStyles( context, image ); + setStyles( context, context.core.image.imageRef ); context.core.image.scrollTopReset = window.pageYOffset || @@ -135,41 +135,6 @@ store( false ); }, - // When opening the lightbox via clicking an image, - // we can use the event target directly and pass it to the - // showLightbox action, which uses it to create the styles. - callShowLightboxFromImage: ( { - context, - event, - actions, - } ) => { - actions.core.image.showLightbox( - { - context, - event, - }, - event.target - ); - }, - // When opening the lightbox via clicking the button, - // we need to reach into event target's parent element to - // get the image element needed to create the styles. - callShowLightboxFromButton: ( { - context, - event, - actions, - } ) => { - actions.core.image.showLightbox( - { - context, - event, - }, - // The event target we receive when clicking the button - // is the SVG element inside of it, so we need to go to - // the parent element's sibling to get the image. - event.target.parentElement.previousElementSibling - ); - }, hideLightbox: async ( { context, event } ) => { context.core.image.hideAnimationEnabled = true; if ( context.core.image.lightboxEnabled ) { @@ -233,6 +198,7 @@ store( }, handleLoad: ( { context, effects, ref } ) => { context.core.image.imageLoaded = true; + context.core.image.imageRef = ref; context.core.image.imageCurrentSrc = ref.currentSrc; effects.core.image.setButtonStyles( { context, From cd54522fac45007abd411eb38ca9468ca99fc218 Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Mon, 16 Oct 2023 16:30:34 -0500 Subject: [PATCH 15/20] Fix style inconsistency between browsers --- packages/block-library/src/image/view.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js index eac9f8ecdf9e3..f40f06b49cc75 100644 --- a/packages/block-library/src/image/view.js +++ b/packages/block-library/src/image/view.js @@ -309,14 +309,14 @@ store( } const figure = ref.parentElement; - const figureWidth = ref.parentElement.offsetWidth; + const figureWidth = ref.parentElement.clientWidth; // We need special handling for the height because // a caption will cause the figure to be taller than // the image, which means we need to account for that // when calculating the placement of the button in the // top right corner of the image. - let figureHeight = ref.parentElement.offsetHeight; + let figureHeight = ref.parentElement.clientHeight; const caption = figure.querySelector( 'figcaption' ); if ( caption ) { const captionComputedStyle = From 09b2f7bab022442d04295cd63ea50894198e2ecb Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Mon, 16 Oct 2023 16:33:17 -0500 Subject: [PATCH 16/20] Change button position slightly --- packages/block-library/src/image/view.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js index f40f06b49cc75..d945de65abb73 100644 --- a/packages/block-library/src/image/view.js +++ b/packages/block-library/src/image/view.js @@ -348,26 +348,26 @@ store( context.core.image.imageButtonTop = ( offsetHeight - referenceHeight ) / 2 + buttonOffsetTop + - 25; + 15; context.core.image.imageButtonRight = - buttonOffsetRight + 25; + buttonOffsetRight + 15; } else { // If it reaches the height first, keep // the height and compute the width. const referenceWidth = offsetHeight * naturalRatio; context.core.image.imageButtonTop = - buttonOffsetTop + 25; + buttonOffsetTop + 15; context.core.image.imageButtonRight = ( offsetWidth - referenceWidth ) / 2 + buttonOffsetRight + - 25; + 15; } } else { context.core.image.imageButtonTop = - buttonOffsetTop + 25; + buttonOffsetTop + 15; context.core.image.imageButtonRight = - buttonOffsetRight + 25; + buttonOffsetRight + 15; } }, setStylesOnResize: ( { state, context, ref } ) => { From 94d8c3ac13a100118640a7a2fa32ae48a1ebc9c5 Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Mon, 16 Oct 2023 16:38:36 -0500 Subject: [PATCH 17/20] Reduce button offset --- packages/block-library/src/image/style.scss | 4 ++-- packages/block-library/src/image/view.js | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/block-library/src/image/style.scss b/packages/block-library/src/image/style.scss index df70de12fe8b0..0fde1262fdec2 100644 --- a/packages/block-library/src/image/style.scss +++ b/packages/block-library/src/image/style.scss @@ -174,8 +174,8 @@ height: 24px; position: absolute; z-index: 100; - top: 5px; - right: 5px; + top: 10px; + right: 10px; text-align: center; padding: 0; border-radius: 10%; diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js index d945de65abb73..bbf19bcd18071 100644 --- a/packages/block-library/src/image/view.js +++ b/packages/block-library/src/image/view.js @@ -348,26 +348,26 @@ store( context.core.image.imageButtonTop = ( offsetHeight - referenceHeight ) / 2 + buttonOffsetTop + - 15; + 10; context.core.image.imageButtonRight = - buttonOffsetRight + 15; + buttonOffsetRight + 10; } else { // If it reaches the height first, keep // the height and compute the width. const referenceWidth = offsetHeight * naturalRatio; context.core.image.imageButtonTop = - buttonOffsetTop + 15; + buttonOffsetTop + 10; context.core.image.imageButtonRight = ( offsetWidth - referenceWidth ) / 2 + buttonOffsetRight + - 15; + 10; } } else { context.core.image.imageButtonTop = - buttonOffsetTop + 15; + buttonOffsetTop + 10; context.core.image.imageButtonRight = - buttonOffsetRight + 15; + buttonOffsetRight + 10; } }, setStylesOnResize: ( { state, context, ref } ) => { From dce84bc512e0b2cf438b729e83f4e82c709d02d1 Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Mon, 16 Oct 2023 17:02:42 -0500 Subject: [PATCH 18/20] Add style override for better consistency across themes --- packages/block-library/src/image/index.php | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php index 0ca9c8e6afe45..f5df3dfdbdd6c 100644 --- a/packages/block-library/src/image/index.php +++ b/packages/block-library/src/image/index.php @@ -220,6 +220,7 @@ function block_core_image_render_lightbox( $block_content, $block ) { data-wp-on--click="actions.core.image.showLightbox" data-wp-style--right="context.core.image.imageButtonRight" data-wp-style--top="context.core.image.imageButtonTop" + style="background: #000" > From c1e79f9dc315c12fa46d2ca76cdfcef31010ef46 Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Mon, 16 Oct 2023 23:11:51 -0500 Subject: [PATCH 19/20] Fix logic so lightbox animates as intended; remove extraneous code --- packages/block-library/src/image/index.php | 2 +- packages/block-library/src/image/view.js | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php index f5df3dfdbdd6c..d604a1356148f 100644 --- a/packages/block-library/src/image/index.php +++ b/packages/block-library/src/image/index.php @@ -197,7 +197,7 @@ function block_core_image_render_lightbox( $block_content, $block ) { ) ); $w->next_tag( 'img' ); - $w->set_attribute( 'data-wp-init', 'effects.core.image.setCurrentSrc' ); + $w->set_attribute( 'data-wp-init', 'effects.core.image.initOriginImage' ); $w->set_attribute( 'data-wp-on--load', 'actions.core.image.handleLoad' ); $w->set_attribute( 'data-wp-effect', 'effects.core.image.setButtonStyles' ); // We need to set an event callback on the `img` specifically diff --git a/packages/block-library/src/image/view.js b/packages/block-library/src/image/view.js index bbf19bcd18071..30d1259637e3d 100644 --- a/packages/block-library/src/image/view.js +++ b/packages/block-library/src/image/view.js @@ -196,9 +196,10 @@ store( } } }, + // This is fired just by lazily loaded + // images on the page, not all images. handleLoad: ( { context, effects, ref } ) => { context.core.image.imageLoaded = true; - context.core.image.imageRef = ref; context.core.image.imageCurrentSrc = ref.currentSrc; effects.core.image.setButtonStyles( { context, @@ -263,17 +264,14 @@ store( effects: { core: { image: { - setCurrentSrc: ( { context, ref } ) => { + initOriginImage: ( { context, ref } ) => { + context.core.image.imageRef = ref; if ( ref.complete ) { context.core.image.imageLoaded = true; context.core.image.imageCurrentSrc = ref.currentSrc; } }, initLightbox: async ( { context, ref } ) => { - context.core.image.figureRef = - ref.querySelector( 'figure' ); - context.core.image.imageRef = - ref.querySelector( 'img' ); if ( context.core.image.lightboxEnabled ) { const focusableElements = ref.querySelectorAll( focusableSelectors ); From ef8f9e2cd2d97e2eb3acf2f4b28113ac932aea05 Mon Sep 17 00:00:00 2001 From: Ricardo Artemio Morales Date: Mon, 16 Oct 2023 23:12:32 -0500 Subject: [PATCH 20/20] Update comment --- packages/block-library/src/image/index.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-library/src/image/index.php b/packages/block-library/src/image/index.php index d604a1356148f..c1b0b53ae13d0 100644 --- a/packages/block-library/src/image/index.php +++ b/packages/block-library/src/image/index.php @@ -207,7 +207,7 @@ function block_core_image_render_lightbox( $block_content, $block ) { $w->set_attribute( 'data-wp-effect--setStylesOnResize', 'effects.core.image.setStylesOnResize' ); $body_content = $w->get_updated_html(); - // Wrap the image in the body content with a button. + // Add a button alongside image in the body content. $img = null; preg_match( '/]+>/', $body_content, $img );