From 961decce88f7bac774fc817a80d8ac093cac800d Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Wed, 25 May 2022 16:21:12 +0000 Subject: [PATCH 1/3] fix(tap-click): ripple effect is no longer dispatched when scrolling --- core/src/utils/tap-click.ts | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/core/src/utils/tap-click.ts b/core/src/utils/tap-click.ts index 19a0ce08292..a8e76d68764 100644 --- a/core/src/utils/tap-click.ts +++ b/core/src/utils/tap-click.ts @@ -5,7 +5,6 @@ import { now, pointerCoord } from './helpers'; export const startTapClick = (config: Config) => { let lastTouch = -MOUSE_WAIT * 10; let lastActivated = 0; - let scrollingEl: HTMLElement | undefined; let activatableEle: HTMLElement | undefined; let activeRipple: Promise<() => void> | undefined; @@ -14,11 +13,6 @@ export const startTapClick = (config: Config) => { const useRippleEffect = config.getBoolean('animated', true) && config.getBoolean('rippleEffect', true); const clearDefers = new WeakMap(); - const isScrolling = () => { - // eslint-disable-next-line @typescript-eslint/prefer-optional-chain - return scrollingEl !== undefined && scrollingEl.parentElement !== null; - }; - // Touch Events const onTouchStart = (ev: TouchEvent) => { lastTouch = now(ev); @@ -58,10 +52,9 @@ export const startTapClick = (config: Config) => { }; const pointerDown = (ev: UIEvent) => { - if (activatableEle || isScrolling()) { + if (activatableEle) { return; } - scrollingEl = undefined; setActivatedElement(getActivatableTarget(ev), ev); }; @@ -146,19 +139,26 @@ export const startTapClick = (config: Config) => { }; const doc = document; - doc.addEventListener('ionScrollStart', (ev) => { - scrollingEl = ev.target as HTMLElement; - cancelActive(); - }); - doc.addEventListener('ionScrollEnd', () => { - scrollingEl = undefined; - }); doc.addEventListener('ionGestureCaptured', cancelActive); doc.addEventListener('touchstart', onTouchStart, true); doc.addEventListener('touchcancel', onTouchEnd, true); doc.addEventListener('touchend', onTouchEnd, true); + /** + * Tap click effects such as the ripple effect should + * not happen when scrolling. For example, if a user scrolls + * the page but also happens to do a touchstart on a button + * as part of the scroll, the ripple effect should not + * be dispatched. The ripple effect should only happen + * if the button is activated and the page is not scrolling. + * + * pointercancel is dispatched on a gesture when scrolling + * starts, so this lets us avoid having to listen for + * ion-content's scroll events. + */ + doc.addEventListener('pointercancel', cancelActive, true); + doc.addEventListener('mousedown', onMouseDown, true); doc.addEventListener('mouseup', onMouseUp, true); From f0fc1c1c4a389b5c850fad743d7bc62d8f763402 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Wed, 25 May 2022 18:09:10 +0000 Subject: [PATCH 2/3] fix(content): pointercancel is now dispatched on scroll --- core/src/components/content/content.scss | 9 ++++++- core/src/utils/tap-click.ts | 30 ++++++++++++------------ 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/core/src/components/content/content.scss b/core/src/components/content/content.scss index 7798b1c6290..2b167838bc7 100644 --- a/core/src/components/content/content.scss +++ b/core/src/components/content/content.scss @@ -79,7 +79,14 @@ overflow: hidden; - touch-action: manipulation; + /** + * touch-action: manipulation is an alias + * for this, but WebKit has an issue + * where pointercancel events are not fired + * when scrolling: https://bugs.webkit.org/show_bug.cgi?id=240917 + * Using the long form below avoids the issue. + */ + touch-action: pan-x pan-y pinch-zoom; } .scroll-y, diff --git a/core/src/utils/tap-click.ts b/core/src/utils/tap-click.ts index a8e76d68764..19a0ce08292 100644 --- a/core/src/utils/tap-click.ts +++ b/core/src/utils/tap-click.ts @@ -5,6 +5,7 @@ import { now, pointerCoord } from './helpers'; export const startTapClick = (config: Config) => { let lastTouch = -MOUSE_WAIT * 10; let lastActivated = 0; + let scrollingEl: HTMLElement | undefined; let activatableEle: HTMLElement | undefined; let activeRipple: Promise<() => void> | undefined; @@ -13,6 +14,11 @@ export const startTapClick = (config: Config) => { const useRippleEffect = config.getBoolean('animated', true) && config.getBoolean('rippleEffect', true); const clearDefers = new WeakMap(); + const isScrolling = () => { + // eslint-disable-next-line @typescript-eslint/prefer-optional-chain + return scrollingEl !== undefined && scrollingEl.parentElement !== null; + }; + // Touch Events const onTouchStart = (ev: TouchEvent) => { lastTouch = now(ev); @@ -52,9 +58,10 @@ export const startTapClick = (config: Config) => { }; const pointerDown = (ev: UIEvent) => { - if (activatableEle) { + if (activatableEle || isScrolling()) { return; } + scrollingEl = undefined; setActivatedElement(getActivatableTarget(ev), ev); }; @@ -139,26 +146,19 @@ export const startTapClick = (config: Config) => { }; const doc = document; + doc.addEventListener('ionScrollStart', (ev) => { + scrollingEl = ev.target as HTMLElement; + cancelActive(); + }); + doc.addEventListener('ionScrollEnd', () => { + scrollingEl = undefined; + }); doc.addEventListener('ionGestureCaptured', cancelActive); doc.addEventListener('touchstart', onTouchStart, true); doc.addEventListener('touchcancel', onTouchEnd, true); doc.addEventListener('touchend', onTouchEnd, true); - /** - * Tap click effects such as the ripple effect should - * not happen when scrolling. For example, if a user scrolls - * the page but also happens to do a touchstart on a button - * as part of the scroll, the ripple effect should not - * be dispatched. The ripple effect should only happen - * if the button is activated and the page is not scrolling. - * - * pointercancel is dispatched on a gesture when scrolling - * starts, so this lets us avoid having to listen for - * ion-content's scroll events. - */ - doc.addEventListener('pointercancel', cancelActive, true); - doc.addEventListener('mousedown', onMouseDown, true); doc.addEventListener('mouseup', onMouseUp, true); From a8d79dfa2050bedb9f125d74274e39cbd6ea8915 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Wed, 25 May 2022 18:09:39 +0000 Subject: [PATCH 3/3] chore(): clean up --- core/src/utils/tap-click.ts | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/core/src/utils/tap-click.ts b/core/src/utils/tap-click.ts index 19a0ce08292..a8e76d68764 100644 --- a/core/src/utils/tap-click.ts +++ b/core/src/utils/tap-click.ts @@ -5,7 +5,6 @@ import { now, pointerCoord } from './helpers'; export const startTapClick = (config: Config) => { let lastTouch = -MOUSE_WAIT * 10; let lastActivated = 0; - let scrollingEl: HTMLElement | undefined; let activatableEle: HTMLElement | undefined; let activeRipple: Promise<() => void> | undefined; @@ -14,11 +13,6 @@ export const startTapClick = (config: Config) => { const useRippleEffect = config.getBoolean('animated', true) && config.getBoolean('rippleEffect', true); const clearDefers = new WeakMap(); - const isScrolling = () => { - // eslint-disable-next-line @typescript-eslint/prefer-optional-chain - return scrollingEl !== undefined && scrollingEl.parentElement !== null; - }; - // Touch Events const onTouchStart = (ev: TouchEvent) => { lastTouch = now(ev); @@ -58,10 +52,9 @@ export const startTapClick = (config: Config) => { }; const pointerDown = (ev: UIEvent) => { - if (activatableEle || isScrolling()) { + if (activatableEle) { return; } - scrollingEl = undefined; setActivatedElement(getActivatableTarget(ev), ev); }; @@ -146,19 +139,26 @@ export const startTapClick = (config: Config) => { }; const doc = document; - doc.addEventListener('ionScrollStart', (ev) => { - scrollingEl = ev.target as HTMLElement; - cancelActive(); - }); - doc.addEventListener('ionScrollEnd', () => { - scrollingEl = undefined; - }); doc.addEventListener('ionGestureCaptured', cancelActive); doc.addEventListener('touchstart', onTouchStart, true); doc.addEventListener('touchcancel', onTouchEnd, true); doc.addEventListener('touchend', onTouchEnd, true); + /** + * Tap click effects such as the ripple effect should + * not happen when scrolling. For example, if a user scrolls + * the page but also happens to do a touchstart on a button + * as part of the scroll, the ripple effect should not + * be dispatched. The ripple effect should only happen + * if the button is activated and the page is not scrolling. + * + * pointercancel is dispatched on a gesture when scrolling + * starts, so this lets us avoid having to listen for + * ion-content's scroll events. + */ + doc.addEventListener('pointercancel', cancelActive, true); + doc.addEventListener('mousedown', onMouseDown, true); doc.addEventListener('mouseup', onMouseUp, true);