bug: tap click activates buttons without scrollEvents, but scrollEvents introduces perf issues

[zrev2220]

Bug Report

Ionic version:

[ ] 4.x
[x] 5.x

Current behavior:

When scrolling an ion-list with clickable ion-items (button="true"), the item the user touches in order to scroll plays its "activated" style or animation (i.e. ripple effect on Android, darkened background on iOS). This happens even if the user is not "clicking" on that item.

Expected behavior:

When scrolling the list, the touched item should not change in appearance (play the ripple effect or change the background). These styles should only activate when the user taps/clicks an item to select it.

Steps to reproduce:

1. Create a basic Ionic app (ionic start)
2. Add an ion-list to one of the pages containing enough ion-items to enable scrolling
3. Scroll the list with the app running in a mobile-ish environment (i.e. emulated touch enabled via devtools in a browser, or run on a physical device)

Related code:

StackBlitz: https://stackblitz.com/edit/ionic-list-ripple-bug

Snippet from Angular template:

<ion-list>
  <ion-item *ngFor="let i of range(100)" button="true" detail="false">
    Item #{{i}}
  </ion-item>
</ion-list>

The range function is defined as follows:

range(n) {
  return Array.from(Array(n).keys());
}

Other information:

This bug seems similar/identical to that identified in #15752. That issue references 7f38d37 as fixing the problem, but since I can reproduce it in a recent Ionic version, it seems to have reappeared(?).

Ionic info:

Note: I created the above StackBlitz example by forking https://stackblitz.com/edit/ionic-v4-angular-tabs. That codebase does not run as-is via ionic serve or npm start, and ionic info fails with an error:

[WARN] You are not in an Ionic project directory. Project context may be missing.

To at least provide some useful ionic info output, I ran ionic init to make the directory an Ionic project. However, some Angular packages are still not installed (since StackBlitz magically makes things work), and the example may not be runnable locally without some additional installations and setup work.

[WARN] Error loading @ionic/angular-toolkit package.json: Error: Cannot find module '@ionic/angular-toolkit/package'

       Require stack:
       - <path to global node_modules>\@ionic\cli\lib\project\index.js
       - <path to global node_modules>\@ionic\cli\lib\index.js
       - <path to global node_modules>\@ionic\cli\index.js
       - <path to global node_modules>\@ionic\cli\bin\ionic
[WARN] Error loading @angular/cli package.json: Error: Cannot find module '@angular/cli/package'

       Require stack:
       - <path to global node_modules>\@ionic\cli\lib\project\index.js
       - <path to global node_modules>\@ionic\cli\lib\index.js
       - <path to global node_modules>\@ionic\cli\index.js
       - <path to global node_modules>\@ionic\cli\bin\ionic
[WARN] Error loading @angular-devkit/build-angular package.json: Error: Cannot find module
       '@angular-devkit/build-angular/package'

       Require stack:
       - <path to global node_modules>\@ionic\cli\lib\project\index.js
       - <path to global node_modules>\@ionic\cli\lib\index.js
       - <path to global node_modules>\@ionic\cli\index.js
       - <path to global node_modules>\@ionic\cli\bin\ionic

Ionic:

   Ionic CLI                     : 6.11.0
   Ionic Framework               : @ionic/angular 5.3.2
   @angular-devkit/build-angular : not installed
   @angular-devkit/schematics    : 10.1.0
   @angular/cli                  : not installed
   @ionic/angular-toolkit        : not installed

Utility:

   cordova-res : 0.15.1
   native-run  : 1.0.0

System:

   NodeJS : v14.4.0
   npm    : 6.14.5
   OS     : Windows 10

[danbn-100295]

Same issuse on ios

[zrev2220]

Any update on this? Issue still exists.

[DavidWiesner]

I found a fix. Just add the empty attribute 'scroll-events' to the ion-content e.g.: <ion-content scroll-events>...</ion-content>. This workaround has some performance implications. This was the breaking commit fd1b44a by @liamdebeasi. This disable also ionScrollStart and ionScrollEnd events not just ionScroll as stated in the docs:

ionic-framework/core/src/components/content/content.tsx

Lines 86 to 90 in 181fc59

	/**
	* Because of performance reasons, ionScroll events are disabled by default, in order to enable them
	* and start listening from (ionScroll), set this property to `true`.
	*/
	@Prop() scrollEvents = false;

ionScrollStart and ionScrollEnd are required to disable and enable the button activation on ionScrollStart see here:

ionic-framework/core/src/utils/tap-click.ts

Lines 143 to 149 in 181fc59

	doc.addEventListener('ionScrollStart', ev => {
	scrollingEl = ev.target as HTMLElement;
	cancelActive();
	});
	doc.addEventListener('ionScrollEnd', () => {
	scrollingEl = undefined;
	});

Update

Here is another workaround without performance regression

import { AfterContentInit, Component, ViewChild } from "@angular/core";
import { IonContent } from "@ionic/angular";

export let supportsPassiveListener = (function checkPassiveListener() {
    let supportsPassive = false;
    try {
        const opts = Object.defineProperty({}, 'passive', {
            // eslint-disable-next-line
            get() {
                supportsPassive = true;
            }
        });
        window.addEventListener('testPassiveListener', null, opts);
        window.removeEventListener('testPassiveListener', null, opts)
    } catch (e) {
        // No support
    }
    return supportsPassive;
}());

export const fixIonContentScrollEvents = (ionContent: IonContent) => {
    const scrollStuff = {isScrolling: false, watchDog: null, lastScroll: 0};
    const onScrollEnd = () => {
        clearInterval(scrollStuff.watchDog);
        scrollStuff.watchDog = null;
        if (scrollStuff.isScrolling) {
            scrollStuff.isScrolling = false;
            // @ts-ignore
            ionContent.el.dispatchEvent(new CustomEvent('ionScrollEnd', {bubbles: true, detail: {isScrolling: false}}));
        }
    };
    const onScrollStart = () => {
        scrollStuff.isScrolling = true;
        // @ts-ignore
        ionContent.el.dispatchEvent(new CustomEvent('ionScrollStart', {bubbles: true, detail: {isScrolling: true}}));
        if (scrollStuff.watchDog) {
            clearInterval(scrollStuff.watchDog);
        }
        scrollStuff.watchDog = setInterval(() => {
            if (scrollStuff.lastScroll < Date.now() - 120) {
                onScrollEnd();
            }
        }, 100);
    };
    const options = supportsPassiveListener ? {passive: true, capture: false} : false;
    ionContent.getScrollElement().then((element) => {
        element.addEventListener('scroll', () => {
            const timeStamp = Date.now();
            const shouldStart = !scrollStuff.isScrolling;
            scrollStuff.lastScroll = timeStamp;
            if (shouldStart) {
                onScrollStart();
            }
        }, options);
    });
};

@Component({
    templateUrl: 'list.html',
    styleUrls: ['list.css']
})
export class ListPage implements AfterContentInit {
    @ViewChild(IonContent, {static: true}) ionContent: IonContent;

    ngAfterContentInit(): void {
        // remove when this issue is closed https://github.com/ionic-team/ionic-framework/issues/22030
        fixIonContentScrollEvents(this.ionContent);
    }
}

BTW @liamdebeasi you may refactor all the tap-click stuff and the scroll listeners to work with passive listeners where possible (keep ssr in mind while refactoring, update: here https://github.com/ionic-team/ionic-framework/blob/dea9248763737164e17678119c775cdfc0e53ccd/core/src/utils/gesture/listener.ts you will have all the functionality you will need)

[SvenBudak]

Any news about this bug?

[digaus]

Yes please fix this :/

[maciejgoscinski]

Still there. For such a basic building block of a mobile-first UI framework, it should rather be considered quite important to fix.
In fact, there are numerous other scrolling issues on mobile, e.g. with toggle switches.

[nosTa1337]

I can confirm the bug and a fix would be great. As @maciejgoscinski mentioned, this seems pretty important as the user UI experience is not so nice right now.
@DavidWiesner thanks for the workaround. Does work well.

[avalanche1]

This is also the issue for ripple effect, if a component has a routerLink prop.

<IonItem routerLink={`/edit-customer/${customerRec._id}`}>

Ripple effect is activated whenever I scroll a list of IonItem's. Very ugly.

[maciejgoscinski]

Similar issue for <ion-card> when it has a routerLink on it. You can swipe content inside of it, which triggers the animation of the card tap (but not the click handler / routerLink itself).

[liamdebeasi]

Thanks for the issue. If you enable set the scrollEvents property on ion-content to true, does it resolve the issue? We have some logic in our tap-click utility that is supposed to prevent this from happening, but the ionScrollStart and ionScrollEnd events are disabled by default for performance reasons until scrollEvents is turned on: https://github.com/ionic-team/ionic-framework/blob/main/core/src/utils/tap-click.ts#L143-L149

[DavidWiesner]

@liamdebeasi yes this will resolves the problem but will introduce the performance problems you mentioned. See my comments above for more information #22030 (comment) enabling scroll events is not an option to resolve this issue because of the performance impact. An passive listener may also resolve some performance issues for the scroll event part.

[liamdebeasi]

Thanks for the update. We are looking into updating the tap click utility for modern browsers, so this context helps.

[avalanche1]

@liamdebeasi Will this be resolved in v6 or v5 as well?

[seopei]

@liamdebeasi any updates?

[muchbetterug]

Any Updates?

[liamdebeasi]

Hi everyone,

I have an experimental fix that I am working on to resolve this issue. Can you all give this a try and let me know if you run into any issues? Using the new PointerEvents API, we can listen for the pointercancel event to ensure that the activated/ripple effect state is not added while scrolling. This is good as it lets us avoid having to listen for the scroll event on ion-content which adds consistent main thread work while scrolling.

6.1.7-dev.11653503099.1a8d79df

This build works on all Ionic packages (@ionic/angular, @ionic/react, etc)

Note: This is an Ionic 6 build and is subject to all Ionic 6 breaking changes.

[liamdebeasi]

Thanks for the issue. This has been resolved via #25352, and a fix will be available in an upcoming release of Ionic Framework.

[ionitron-bot]

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.