Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: scroll assist cancels first click event #21871

Closed
iphilgood opened this issue Aug 4, 2020 · 23 comments
Closed

bug: scroll assist cancels first click event #21871

iphilgood opened this issue Aug 4, 2020 · 23 comments

Comments

@iphilgood
Copy link

@iphilgood iphilgood commented Aug 4, 2020

Bug Report

Ionic version:
[ ] 4.x
[x] 5.x

Current behavior:
An ion-item in a ion-list needs two clicks/taps to trigger the click event. This only happens on iOS (Safari/WebKit).

Expected behavior:
The event should trigger on the first click.

Steps to reproduce:

  1. Create an ion-list
  2. Within the ion-list create an ion-item with a (click) event handler
  3. Within the ion-item create an ion-label
  4. Within the ion-item create an ion-input
  5. Within the ion-item create an ion-button

Related code:

This is the markup with which you can reproduce the issue. You can also find a GIF at the bottom of this issue.

<ion-list>
  <ion-item (click)="onTitleClicked()" button detail="false">
    <ion-label position="floating">
      Title
    </ion-label>
    <ion-input [value]="title" readonly></ion-input>
    <ion-button
      *ngIf="title"
      slot="end"
      fill="clear"
      size="small"
      (click)="resetTitle($event)"
    >
      <ion-icon name="close"></ion-icon>
    </ion-button>
  </ion-item>
</ion-list>

I tried to narrow it down to the following, but the issue still persists.

<ion-list>
  <ion-item (click)="onTitleClicked()" button detail="false">
    <ion-label>
      Title
    </ion-label>
    <ion-input [value]="title"></ion-input>
  </ion-item>
</ion-list>

You can find a sample application on my GitHub.

Other information:
I found out that this change of behaviour was introduced with version v5.0.6. I've checked the changes between the versions v5.0.5 and v5.0.6 but I didn't find any suspicious code 🤔

ionic-bug

Ionic info:

Ionic:

   Ionic CLI                     : 6.11.0 (/Users/phil/.asdf/installs/nodejs/12.17.0/.npm/lib/node_modules/@ionic/cli)
   Ionic Framework               : @ionic/angular 5.3.1
   @angular-devkit/build-angular : 0.901.12
   @angular-devkit/schematics    : 9.1.12
   @angular/cli                  : 9.1.12
   @ionic/angular-toolkit        : 2.3.0

Capacitor:

   Capacitor CLI   : 2.4.0
   @capacitor/core : 2.4.0

Utility:

   cordova-res : not installed
   native-run  : 1.0.0

System:

   NodeJS : v12.17.0 (/Users/phil/.asdf/installs/nodejs/12.17.0/bin/node)
   npm    : 6.14.4
   OS     : macOS Catalina
@ionitron-bot ionitron-bot bot added the triage label Aug 4, 2020
@iphilgood
Copy link
Author

@iphilgood iphilgood commented Aug 4, 2020

I've found out what the issue was. I will submit a PR shortly 🙌🏽

iphilgood added a commit to iphilgood/ionic-framework that referenced this issue Aug 4, 2020
@ValisStigma
Copy link

@ValisStigma ValisStigma commented Aug 5, 2020

Can confirm this

@iphilgood
Copy link
Author

@iphilgood iphilgood commented Aug 5, 2020

Well, I thought I found the issue but actually I didn't. 😅 For more information check out the comment from @liamdebeasi.

Hopefully, this gets fixed soon. If I can do anything, please let me know. I'm happy to help.

@lincolnthree
Copy link

@lincolnthree lincolnthree commented Aug 29, 2020

I have been able to reproduce this sporadically as well using the (tap) event, however, in my case, switching to (click) seems to have been a passable workaround.

@longgt
Copy link

@longgt longgt commented Sep 9, 2020

Maybe related to #20986

@lincolnthree
Copy link

@lincolnthree lincolnthree commented Sep 9, 2020

Now that you mention it, this does only occur on iOS from the testing I've seen.

@fromage9747
Copy link

@fromage9747 fromage9747 commented Oct 5, 2020

At least you can get it to click. My ion-item button does not fire a click event at all...

@lincolnthree
Copy link

@lincolnthree lincolnthree commented Oct 5, 2020

Actually it's been happening on Android as well. Do you have any custom Hammer.js recognizers configured?

@longgt
Copy link

@longgt longgt commented Oct 6, 2020

Changing event listeners, like touchstart and touchmove to passive wherever possible.

may help a little bit. That's what I got from release notes in 4.10.4 of Mobiscroll.

@iphilgood
Copy link
Author

@iphilgood iphilgood commented Nov 25, 2020

Thanks to this discussion on StackOverflow I found out that two clicks/taps are only necessary when using an <ion-label> with position="floating" or position="stacked".

@florianmonfort
Copy link

@florianmonfort florianmonfort commented Dec 30, 2020

Thanks to this discussion on StackOverflow I found out that two clicks/taps are only necessary when using an <ion-label> with position="floating" or position="stacked".

@iphilgood hi, we have tried this solution and it doesn't work for us, so if anyone has suggestions for how to fix this is would be well appreciated.

@lydemann
Copy link
Contributor

@lydemann lydemann commented Jan 18, 2021

Can confirm as well in this demo with these steps:
Ion content with some padding (50px here) and scrollEvents true
--background-activated-opacity: 1 on item
Scroll to very bottom of list and click on an item
See list i "moving" and click is ignored, second click will work

<ion-content [scrollEvents]="true" style="--padding-bottom: 50px;">
  <ion-item
    button
    style="--background-activated-opacity: 1"
    *ngFor="let item of items"
    (click)="onClick(item)"
  >
    <ion-label>{{ item }}</ion-label>
  </ion-item>
</ion-content>

https://github.com/lydemann/ionic-item-scrollend-click-bug

@liamdebeasi liamdebeasi changed the title bug: ion-item needs two clicks to trigger click event bug: scroll assist cancels first click event Jan 19, 2021
@liamdebeasi
Copy link
Member

@liamdebeasi liamdebeasi commented Jan 19, 2021

I can reproduce this. The issue is with the scroll assist utility. When scroll assist starts after an input is tapped, Ionic Framework calls preventDefault and stopPropagation on the touchend event, which cancels the click event: https://github.com/ionic-team/ionic-framework/blob/master/core/src/utils/input-shims/hacks/scroll-assist.ts#L29-L30

I can remove those lines, but I need to make sure it does not interfere with anything else.

@liamdebeasi
Copy link
Member

@liamdebeasi liamdebeasi commented Jan 25, 2021

Can you try the following dev build and let me know if it resolves the issue?

Angular

npm install @ionic/[email protected]

React

npm install @ionic/[email protected] @ionic/[email protected]

Vue

npm install @ionic/[email protected] @ionic/[email protected]

Stencil/Vanilla JavaScript

npm install @ionic/[email protected]
@liamdebeasi
Copy link
Member

@liamdebeasi liamdebeasi commented Jan 25, 2021

Can confirm as well in this demo with these steps:
Ion content with some padding (50px here) and scrollEvents true
--background-activated-opacity: 1 on item
Scroll to very bottom of list and click on an item
See list i "moving" and click is ignored, second click will work

<ion-content [scrollEvents]="true" style="--padding-bottom: 50px;">
  <ion-item
    button
    style="--background-activated-opacity: 1"
    *ngFor="let item of items"
    (click)="onClick(item)"
  >
    <ion-label>{{ item }}</ion-label>
  </ion-item>
</ion-content>

https://github.com/lydemann/ionic-item-scrollend-click-bug

The dev build I posted appears to fix the repo you posted in #22740, but https://github.com/lydemann/ionic-item-scrollend-click-bug seems like a different issue.

@liamdebeasi liamdebeasi mentioned this issue Jan 27, 2021
@iphilgood
Copy link
Author

@iphilgood iphilgood commented Jan 27, 2021

Thanks for your dev build! Awesome that this issue gets further investigated 🎉

The click event gets now fired on first click but a funny thing is happening. The lower (like lower in the y axis) the item with input gets rendered, the longer it takes to fire the click event 🤔 In other words, the click on the very first item of a page gets fired faster than the click on the very last item of a page.

Do you have an idea why this is happening?

I've updated the demo repo from my very first post with the new version and additional examples. This behaviour is actually reproducable in my demo repo.

I also tested this only on iOS. Do you think I should try that on Android as well? Furthermore, if there's anything I can do to support you please let me know.

@kensodemann
Copy link
Member

@kensodemann kensodemann commented Feb 1, 2021

@liamdebeasi - I have been working with someone on an issue related to this one and your dev build appears to fix the issue for them without causing other problems.

@liamdebeasi
Copy link
Member

@liamdebeasi liamdebeasi commented Feb 1, 2021

@iphilgood Can you try this dev build? It should fix the issue you mentioned.

Angular

npm install @ionic/[email protected]

React

npm install @ionic/[email protected] @ionic/[email protected]

Vue

npm install @ionic/[email protected] @ionic/[email protected]

Stencil/Vanilla JavaScript

npm install @ionic/[email protected]
@iphilgood
Copy link
Author

@iphilgood iphilgood commented Feb 1, 2021

@liamdebeasi Thank you so much 🎉 I just tried it out and it works like a charm 😍 I tested it in our production app and in my demo repo. Finally, the issue seems to be resolved. I run some tests on an iPhone 8, iPhone 12 Pro and also on an Android device and everything works as expected.

Should I run some additional testing or can I support you in some other way? Please let me know.

@liamdebeasi liamdebeasi added this to Backlog 🤖 in Ionic Core via automation Feb 1, 2021
@liamdebeasi liamdebeasi moved this from Backlog 🤖 to In progress 🤺 in Ionic Core Feb 1, 2021
@liamdebeasi liamdebeasi added this to the 5.5.4 milestone Feb 1, 2021
Ionic Core automation moved this from In progress 🤺 to Done 🎉 Feb 1, 2021
@liamdebeasi
Copy link
Member

@liamdebeasi liamdebeasi commented Feb 1, 2021

Glad the issue is resolved. I merged in #22845, the fix for this issue. This fix will be available in the next release of Ionic Framework.

Thanks!

@crodriguezdominguez
Copy link

@crodriguezdominguez crodriguezdominguez commented Feb 13, 2021

I still have the same issue with a checkbox within a ion-item. The checkbox is not checked/unchecked on the first tap (only in iOS, in Android it works like a charm).

@lincolnthree
Copy link

@lincolnthree lincolnthree commented Feb 15, 2021

Still having this issue sporadically on things like buttons, the ion-back-button, and other elements. It seems to happen more near the edge of the screen than other places, though I'm not sure why that would be. Possibly menu gesture interaction?

@ionitron-bot
Copy link

@ionitron-bot ionitron-bot bot commented Mar 17, 2021

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.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Mar 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Ionic Core
  
Done 🎉
Linked pull requests

Successfully merging a pull request may close this issue.

None yet