Skip to content

Commit

Permalink
🐛Fixes swipe to dismiss badly ordered swipes on amp-lightbox-gallery (
Browse files Browse the repository at this point in the history
#26788)

* Experimenting

* Fixes swipe to dismiss badly ordered swipes

* Remvoe console.log
  • Loading branch information
wassgha committed Feb 13, 2020
1 parent 5db0dcd commit 2520b4e
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 51 deletions.
25 changes: 0 additions & 25 deletions extensions/amp-lightbox-gallery/0.1/amp-lightbox-gallery.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,6 @@ export class AmpLightboxGallery extends AMP.BaseElement {
/** @private @const */
this.boundMeasureMutate_ = this.measureMutateElement.bind(this);

/** @private {boolean} */
this.swipeStarted_ = false;

/** @private @const */
this.swipeToDismiss_ = new SwipeToDismiss(
this.win,
Expand Down Expand Up @@ -638,14 +635,6 @@ export class AmpLightboxGallery extends AMP.BaseElement {
*/
swipeGesture_(data) {
if (data.first) {
if (this.swipeStarted_) {
dev().error(
TAG,
'badly ordered swipe gestures: second first without last'
);
}
this.swipeStarted_ = true;

const {sourceElement} = this.getCurrentElement_();
const parentCarousel = this.getSourceElementParentCarousel_(
sourceElement
Expand All @@ -660,20 +649,6 @@ export class AmpLightboxGallery extends AMP.BaseElement {
return;
}

if (!this.swipeStarted_) {
dev().error(
TAG,
'badly ordered swipe gestures: subsequent without first'
);
return;
}

if (data.last) {
this.swipeToDismiss_.endSwipe(data);
this.swipeStarted_ = false;
return;
}

this.swipeToDismiss_.swipeMove(data);
}

Expand Down
47 changes: 21 additions & 26 deletions extensions/amp-lightbox-gallery/0.1/swipe-to-dismiss.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,11 @@ export class SwipeToDismiss {
* @private {?function()}
*/
this.preventScrollUnlistener_ = null;

/**
* @private {boolean}
*/
this.isSwiping_ = false;
}

/**
Expand All @@ -163,28 +168,13 @@ export class SwipeToDismiss {
this.hiddenElement_ = hiddenElement;
this.mask_ = mask;
this.overlay_ = overlay;
this.isSwiping_ = true;

this.mutateElement_(() => {
this.startSwipeToDismiss_();
});
}

/**
* Handles a swipe move.
* @param {!SwipeDef} data
*/
swipeMove(data) {
this.swipeMove_(data, false);
}

/**
* Handles the end of a swipe.
* @param {!SwipeDef} data
*/
endSwipe(data) {
this.swipeMove_(data, true);
}

/**
* Carries momentum for the swipe forwards to a final destination, with the
* duration depending on the velocity.
Expand Down Expand Up @@ -350,12 +340,15 @@ export class SwipeToDismiss {
}

/**
*
* @param {!SwipeDef} data The data for the swipe.
* @param {boolean} isLast If this move is the last movement for the swipe.
*/
swipeMove_(data, isLast) {
const {deltaX, deltaY, velocityX, velocityY} = data;
swipeMove(data) {
const {deltaX, deltaY, velocityX, velocityY, last} = data;
const wasSwiping = this.isSwiping_;
if (last) {
this.isSwiping_ = false;
}

// Need to capture these as they will no longer be available after closing.
const distance = calculateDistance(0, 0, deltaX, deltaY);
const releasePercentage = Math.min(distance / SWIPE_TO_CLOSE_DISTANCE, 1);
Expand All @@ -368,7 +361,7 @@ export class SwipeToDismiss {
const overlayOpacity = lerp(1, 0, hideOverlayPercentage);

this.mutateElement_(() => {
if (isLast) {
if (data.last && wasSwiping) {
this.releaseSwipe_(scale, velocityX, velocityY, deltaX, deltaY).then(
() => {
// TODO(sparhami) These should be called in a `mutateElement`,
Expand All @@ -381,11 +374,13 @@ export class SwipeToDismiss {
return;
}

this.adjustForSwipePosition_(
`scale(${scale}) translate(${deltaX}px, ${deltaY}px)`,
maskOpacity,
overlayOpacity
);
if (this.isSwiping_) {
this.adjustForSwipePosition_(
`scale(${scale}) translate(${deltaX}px, ${deltaY}px)`,
maskOpacity,
overlayOpacity
);
}
});
}
}

0 comments on commit 2520b4e

Please sign in to comment.