Skip to content

Commit

Permalink
fix jitter scrolling by adopting css based overflow
Browse files Browse the repository at this point in the history
  • Loading branch information
madhusudhand committed Jun 11, 2024
1 parent fd587da commit 9928f25
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 65 deletions.
4 changes: 0 additions & 4 deletions packages/block-library/src/image/index.php
Original file line number Diff line number Diff line change
Expand Up @@ -258,12 +258,8 @@ class="wp-lightbox-overlay zoom"
data-wp-class--show-closing-animation="state.showClosingAnimation"
data-wp-watch="callbacks.setOverlayFocus"
data-wp-on--keydown="actions.handleKeydown"
data-wp-on-async--touchstart="actions.handleTouchStart"
data-wp-on--touchmove="actions.handleTouchMove"
data-wp-on-async--touchend="actions.handleTouchEnd"
data-wp-on-async--click="actions.hideLightbox"
data-wp-on-async-window--resize="callbacks.setOverlayStyles"
data-wp-on-async-window--scroll="actions.handleScroll"
tabindex="-1"
>
<button type="button" aria-label="$close_button_label" style="fill: $close_button_color" class="close-button">
Expand Down
2 changes: 1 addition & 1 deletion packages/block-library/src/image/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@

.lightbox-image-container {
position: absolute;
overflow: hidden;
// overflow: hidden;
top: 50%;
left: 50%;
transform-origin: top left;
Expand Down
68 changes: 8 additions & 60 deletions packages/block-library/src/image/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,6 @@
*/
import { store, getContext, getElement } from '@wordpress/interactivity';

/**
* Tracks whether user is touching screen; used to differentiate behavior for
* touch and mouse input.
*
* @type {boolean}
*/
let isTouching = false;

/**
* Tracks the last time the screen was touched; used to differentiate behavior
* for touch and mouse input.
*
* @type {number}
*/
let lastTouchTime = 0;

/**
* Stores the image reference of the currently opened lightbox.
*
Expand Down Expand Up @@ -77,7 +61,11 @@ const { state, actions, callbacks } = store(
state.scrollTopReset = document.documentElement.scrollTop;
state.scrollLeftReset = document.documentElement.scrollLeft;

// Moves the information of the expaned image to the state.
// store the overflow state of the document to restore it later
state.documentOverflow = document.body.style.overflow;
document.body.style.overflow = 'hidden';

// Moves the information of the expanded image to the state.
ctx.currentSrc = ctx.imageRef.currentSrc;
imageRef = ctx.imageRef;
buttonRef = ctx.buttonRef;
Expand Down Expand Up @@ -106,6 +94,9 @@ const { state, actions, callbacks } = store(
state.currentImage = {};
imageRef = null;
buttonRef = null;

// Restore document overflow
document.body.style.overflow = state.documentOverflow;
}, 450 );

// Starts the overlay closing animation. The showClosingAnimation
Expand All @@ -128,49 +119,6 @@ const { state, actions, callbacks } = store(
}
}
},
handleTouchMove( event ) {
// On mobile devices, prevents triggering the scroll event because
// otherwise the page jumps around when it resets the scroll position.
// This also means that closing the lightbox requires that a user
// perform a simple tap. This may be changed in the future if there is a
// better alternative to override or reset the scroll position during
// swipe actions.
if ( state.overlayEnabled ) {
event.preventDefault();
}
},
handleTouchStart() {
isTouching = true;
},
handleTouchEnd() {
// Waits a few milliseconds before resetting to ensure that pinch to
// zoom works consistently on mobile devices when the lightbox is open.
lastTouchTime = Date.now();
isTouching = false;
},
handleScroll() {
// Prevents scrolling behaviors that trigger content shift while the
// lightbox is open. It would be better to accomplish through CSS alone,
// but using overflow: hidden is currently the only way to do so and
// that causes a layout to shift and prevents the zoom animation from
// working in some cases because it's not possible to account for the
// layout shift when doing the animation calculations. Instead, it uses
// JavaScript to prevent and reset the scrolling behavior.
if ( state.overlayOpened ) {
// Avoids overriding the scroll behavior on mobile devices because
// doing so breaks the pinch to zoom functionality, and users should
// be able to zoom in further on the high-res image.
if ( ! isTouching && Date.now() - lastTouchTime > 450 ) {
// It doesn't rely on `event.preventDefault()` to prevent scrolling
// because the scroll event can't be canceled, so it resets the
// position instead.
window.scrollTo(
state.scrollLeftReset,
state.scrollTopReset
);
}
}
},
},
callbacks: {
setOverlayStyles() {
Expand Down

0 comments on commit 9928f25

Please sign in to comment.