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.

Sign up for GitHub

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

Jump to bottom

Image lightbox: move image data from context to state #63348

Merged
merged 8 commits into from
Jul 15, 2024
Merged

Image lightbox: move image data from context to state #63348

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b1e8c2c
move the image context data to state
madhusudhand Jul 10, 2024
a24fa62
move image and button refs from context to state
madhusudhand Jul 11, 2024
189c7f5
Use hash instead of counter for the unique id
luisherranz Jul 12, 2024
3c3c3f2
Don't use immutability
luisherranz Jul 12, 2024
64ad89e
Fix reading prop from old context
luisherranz Jul 12, 2024
56d9487
Only store the current image ID in the state
luisherranz Jul 12, 2024
d0d8a85
Replace hash based on url with PHP's uniqid()
luisherranz Jul 15, 2024
86e1887
fix lightbox trigger image position
madhusudhand Jul 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 27 additions & 12 deletions packages/block-library/src/image/index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,22 +185,37 @@ function block_core_image_render_lightbox( $block_content, $block ) {
$p->seek( 'figure' );
$figure_class_names = $p->get_attribute( 'class' );
$figure_styles = $p->get_attribute( 'style' );

// Create unique id and set the image metadata in the state.
$unique_image_id = uniqid();

wp_interactivity_state(
'core/image',
array(
'metadata' => array(
$unique_image_id => array(
'uploadedSrc' => $img_uploaded_src,
'figureClassNames' => $figure_class_names,
'figureStyles' => $figure_styles,
'imgClassNames' => $img_class_names,
'imgStyles' => $img_styles,
'targetWidth' => $img_width,
'targetHeight' => $img_height,
'scaleAttr' => $block['attrs']['scale'] ?? false,
'ariaLabel' => $aria_label,
'alt' => $alt,
),
),
)
);

$p->add_class( 'wp-lightbox-container' );
$p->set_attribute( 'data-wp-interactive', 'core/image' );
$p->set_attribute(
'data-wp-context',
wp_json_encode(
array(
'uploadedSrc' => $img_uploaded_src,
'figureClassNames' => $figure_class_names,
'figureStyles' => $figure_styles,
'imgClassNames' => $img_class_names,
'imgStyles' => $img_styles,
'targetWidth' => $img_width,
'targetHeight' => $img_height,
'scaleAttr' => $block['attrs']['scale'] ?? false,
'ariaLabel' => $aria_label,
'alt' => $alt,
'imageId' => $unique_image_id,
),
JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_HEX_AMP
)
Expand Down Expand Up @@ -231,8 +246,8 @@ class="lightbox-trigger"
aria-label="' . esc_attr( $aria_label ) . '"
data-wp-init="callbacks.initTriggerButton"
data-wp-on-async--click="actions.showLightbox"
data-wp-style--right="context.imageButtonRight"
data-wp-style--top="context.imageButtonTop"
data-wp-style--right="state.imageButtonRight"
data-wp-style--top="state.imageButtonTop"
>
<svg xmlns="http://www.w3.org/2000/svg" width="12" height="12" fill="none" viewBox="0 0 12 12">
<path fill="#fff" d="M2 0a2 2 0 0 0-2 2v2h1.5V2a.5.5 0 0 1 .5-.5h2V0H2Zm2 10.5H2a.5.5 0 0 1-.5-.5V8H0v2a2 2 0 0 0 2 2h2v-1.5ZM8 12v-1.5h2a.5.5 0 0 0 .5-.5V8H12v2a2 2 0 0 1-2 2H8Zm2-12a2 2 0 0 1 2 2v2h-1.5V2a.5.5 0 0 0-.5-.5H8V0h2Z" />
Expand Down
70 changes: 43 additions & 27 deletions packages/block-library/src/image/view.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,12 @@ const { state, actions, callbacks } = store(
'core/image',
{
state: {
currentImage: {},
currentImageId: null,
get currentImage() {
return state.metadata[ state.currentImageId ];
},
get overlayOpened() {
return state.currentImage.currentSrc;
return state.currentImageId !== null;
},
get roleAttribute() {
return state.overlayOpened ? 'dialog' : null;
Expand All @@ -48,31 +51,43 @@ const { state, actions, callbacks } = store(
) }; object-fit:cover;`
);
},
get imageButtonRight() {
const { imageId } = getContext();
return state.metadata[ imageId ].imageButtonRight;
},
get imageButtonTop() {
const { imageId } = getContext();
return state.metadata[ imageId ].imageButtonTop;
},
},
actions: {
showLightbox() {
const ctx = getContext();
const { imageId } = getContext();

// Bails out if the image has not loaded yet.
if ( ! ctx.imageRef?.complete ) {
if ( ! state.metadata[ imageId ].imageRef?.complete ) {
return;
}

// Stores the positons of the scroll to fix it until the overlay is
// Stores the positions of the scroll to fix it until the overlay is
// closed.
state.scrollTopReset = document.documentElement.scrollTop;
state.scrollLeftReset = document.documentElement.scrollLeft;

// Moves the information of the expaned image to the state.
ctx.currentSrc = ctx.imageRef.currentSrc;
state.currentImage = ctx;
// Sets the current expanded image in the state and enables the overlay.
state.currentImageId = imageId;
state.overlayEnabled = true;

// Computes the styles of the overlay for the animation.
callbacks.setOverlayStyles();
},
hideLightbox() {
if ( state.overlayEnabled ) {
// Starts the overlay closing animation. The showClosingAnimation
// class is used to avoid showing it on page load.
state.showClosingAnimation = true;
state.overlayEnabled = false;

// Waits until the close animation has completed before allowing a
// user to scroll again. The duration of this animation is defined in
// the `styles.scss` file, but in any case we should wait a few
Expand All @@ -86,14 +101,9 @@ const { state, actions, callbacks } = store(
preventScroll: true,
} );

// Resets the current image to mark the overlay as closed.
state.currentImage = {};
// Resets the current image id to mark the overlay as closed.
state.currentImageId = null;
}, 450 );

// Starts the overlay closing animation. The showClosingAnimation
// class is used to avoid showing it on page load.
state.showClosingAnimation = true;
state.overlayEnabled = false;
}
},
handleKeydown( event ) {
Expand Down Expand Up @@ -213,6 +223,7 @@ const { state, actions, callbacks } = store(
let containerMaxHeight = imgMaxHeight;
let containerWidth = imgMaxWidth;
let containerHeight = imgMaxHeight;

// Checks if the target image has a different ratio than the original
// one (thumbnail). Recalculates the width and height.
if ( naturalRatio.toFixed( 2 ) !== imgRatio.toFixed( 2 ) ) {
Expand Down Expand Up @@ -323,9 +334,11 @@ const { state, actions, callbacks } = store(
`;
},
setButtonStyles() {
const ctx = getContext();
const { imageId } = getContext();
const { ref } = getElement();
ctx.imageRef = ref;

state.metadata[ imageId ].imageRef = ref;
state.metadata[ imageId ].currentSrc = ref.currentSrc;

const {
naturalWidth,
Expand Down Expand Up @@ -368,10 +381,13 @@ const { state, actions, callbacks } = store(
const buttonOffsetTop = figureHeight - offsetHeight;
const buttonOffsetRight = figureWidth - offsetWidth;

let imageButtonTop = buttonOffsetTop + 16;
let imageButtonRight = buttonOffsetRight + 16;

// In the case of an image with object-fit: contain, the size of the
// <img> element can be larger than the image itself, so it needs to
// calculate where to place the button.
if ( ctx.scaleAttr === 'contain' ) {
if ( state.metadata[ imageId ].scaleAttr === 'contain' ) {
// Natural ratio of the image.
const naturalRatio = naturalWidth / naturalHeight;
// Offset ratio of the image.
Expand All @@ -381,25 +397,25 @@ const { state, actions, callbacks } = store(
// If it reaches the width first, it keeps the width and compute the
// height.
const referenceHeight = offsetWidth / naturalRatio;
ctx.imageButtonTop =
imageButtonTop =
( offsetHeight - referenceHeight ) / 2 +
buttonOffsetTop +
16;
ctx.imageButtonRight = buttonOffsetRight + 16;
imageButtonRight = buttonOffsetRight + 16;
} else {
// If it reaches the height first, it keeps the height and compute
// the width.
const referenceWidth = offsetHeight * naturalRatio;
ctx.imageButtonTop = buttonOffsetTop + 16;
ctx.imageButtonRight =
imageButtonTop = buttonOffsetTop + 16;
imageButtonRight =
( offsetWidth - referenceWidth ) / 2 +
buttonOffsetRight +
16;
}
} else {
ctx.imageButtonTop = buttonOffsetTop + 16;
ctx.imageButtonRight = buttonOffsetRight + 16;
}

state.metadata[ imageId ].imageButtonTop = imageButtonTop;
state.metadata[ imageId ].imageButtonRight = imageButtonRight;
Comment on lines +417 to +418
Copy link
Member Author

@madhusudhand madhusudhand Jul 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@luisherranz we might have to revert to keep the data in context instead, because it is being used in the template, not anywhere else.

https://github.com/WordPress/gutenberg/blob/trunk/packages/block-library/src/image/index.php#L234-L235

It is breaking the icon placement.

image

Copy link
Contributor

Choose a reason for hiding this comment

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

I can reproduce this as well.

Copy link
Member

Choose a reason for hiding this comment

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

We can use derived state, similar to how we are using in other places:

store('...', {
  state: {
    get imageButtonRight() {
      const { imageId } = getContext();
      return state.metadata[imageId].imageButtonRight;
    },
    get imageButtonTop() ...
  },
});
<button
  data-wp-style--right="state.imageButtonRight"
  data-wp-style--top="state.imageButtonTop"
>
  ...
</button>

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. The issue is fixed now.

},
setOverlayFocus() {
if ( state.overlayEnabled ) {
Expand All @@ -409,9 +425,9 @@ const { state, actions, callbacks } = store(
}
},
initTriggerButton() {
const ctx = getContext();
const { imageId } = getContext();
const { ref } = getElement();
ctx.buttonRef = ref;
state.metadata[ imageId ].buttonRef = ref;
},
},
},
Expand Down
Loading