From ef8559d060788ddd0399d9ea71005deda3839cee Mon Sep 17 00:00:00 2001 From: Johannes Date: Wed, 11 May 2022 22:30:50 +0100 Subject: [PATCH] fix: do not deselect when not zooming using touchscreen pinch (#5181) * Do not deselect when zooming * do not deselect when trackpad zooming on Safari Co-authored-by: dwelle --- src/components/App.tsx | 62 ++++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 24 deletions(-) diff --git a/src/components/App.tsx b/src/components/App.tsx index 66880b64..34389700 100644 --- a/src/components/App.tsx +++ b/src/components/App.tsx @@ -1921,23 +1921,47 @@ class App extends React.Component { } }; + /** + * returns whether user is making a gesture with >= 2 fingers (points) + * on o touch screen (not on a trackpad). Currently only relates to Darwin + * (iOS/iPadOS,MacOS), but may work on other devices in the future if + * GestureEvent is standardized. + */ + private isTouchScreenMultiTouchGesture = () => { + // we don't want to deselect when using trackpad, and multi-point gestures + // only work on touch screens, so checking for >= pointers means we're on a + // touchscreen + return gesture.pointers.size >= 2; + }; + + // fires only on Safari private onGestureStart = withBatchedUpdates((event: GestureEvent) => { event.preventDefault(); - this.setState({ - selectedElementIds: {}, - }); + + // we only want to deselect on touch screens because user may have selected + // elements by mistake while zooming + if (this.isTouchScreenMultiTouchGesture()) { + this.setState({ + selectedElementIds: {}, + }); + } gesture.initialScale = this.state.zoom.value; }); + // fires only on Safari private onGestureChange = withBatchedUpdates((event: GestureEvent) => { event.preventDefault(); // onGestureChange only has zoom factor but not the center. // If we're on iPad or iPhone, then we recognize multi-touch and will - // zoom in at the right location on the touchMove handler already. - // On Macbook, we don't have those events so will zoom in at the + // zoom in at the right location in the touchmove handler + // (handleCanvasPointerMove). + // + // On Macbook trackpad, we don't have those events so will zoom in at the // current location instead. - if (gesture.pointers.size >= 2) { + // + // As such, bail from this handler on touch devices. + if (this.isTouchScreenMultiTouchGesture()) { return; } @@ -1956,12 +1980,16 @@ class App extends React.Component { } }); + // fires only on Safari private onGestureEnd = withBatchedUpdates((event: GestureEvent) => { event.preventDefault(); - this.setState({ - previousSelectedElementIds: {}, - selectedElementIds: this.state.previousSelectedElementIds, - }); + // reselect elements only on touch screens (see onGestureStart) + if (this.isTouchScreenMultiTouchGesture()) { + this.setState({ + previousSelectedElementIds: {}, + selectedElementIds: this.state.previousSelectedElementIds, + }); + } gesture.initialScale = null; }); @@ -5636,7 +5664,6 @@ class App extends React.Component { } const { deltaX, deltaY } = event; - const { selectedElementIds, previousSelectedElementIds } = this.state; // note that event.ctrlKey is necessary to handle pinch zooming if (event.metaKey || event.ctrlKey) { const sign = Math.sign(deltaY); @@ -5646,14 +5673,6 @@ class App extends React.Component { if (absDelta > MAX_STEP) { delta = MAX_STEP * sign; } - if (Object.keys(previousSelectedElementIds).length !== 0) { - setTimeout(() => { - this.setState({ - selectedElementIds: previousSelectedElementIds, - previousSelectedElementIds: {}, - }); - }, 1000); - } let newZoom = this.state.zoom.value - delta / 100; // increase zoom steps the more zoomed-in we are (applies to >100% only) @@ -5672,11 +5691,6 @@ class App extends React.Component { }, state, ), - selectedElementIds: {}, - previousSelectedElementIds: - Object.keys(selectedElementIds).length !== 0 - ? selectedElementIds - : previousSelectedElementIds, shouldCacheIgnoreZoom: true, })); this.resetShouldCacheIgnoreZoomDebounced();