ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
51.01k stars 13.51k forks source link

bug: gesture events are not fired when touching and moving vertically on a SwiperJS container #27819

Closed greg-md closed 1 year ago

greg-md commented 1 year ago

Prerequisites

Ionic Framework Version

v7.x

Current Behavior

The only events that are triggered are canStart and onWillStart. Other events are not triggered.

const gesture = this.gestureCtrl.create({
  el: this.contentRef.nativeElement,
  gestureName: 'swipe',
  direction: 'y',
  canStart: (event) => {
    console.log('canStart');
  },
  onWillStart: async (event) => {
    console.log('onWillStart');
  },
  onStart: (event) => {
    console.log('onStart');
  },
  onMove: (event) => {
    console.log('onMove');
  },
  onEnd: (event) => {
    console.log('onEnd');
  },
});

Expected Behavior

To produce onStart, onMove and onEnd events.

Steps to Reproduce

  1. create a scrollable container;
  2. add a swiper inside;
  3. listen for gesture events when swiping vertically;

Code Reproduction URL

https://stackblitz.com/edit/angular-7mwuay

Ionic Info

Ionic:

   Ionic CLI                     : 7.1.1 (/Users/greg/.nvm/versions/node/v16.16.0/lib/node_modules/@ionic/cli)
   Ionic Framework               : @ionic/angular 7.1.1
   @angular-devkit/build-angular : 16.0.2
   @angular-devkit/schematics    : 16.0.2
   @angular/cli                  : 16.0.2
   @ionic/angular-toolkit        : 9.0.0

Capacitor:

   Capacitor CLI      : 5.0.3
   @capacitor/android : 5.0.3
   @capacitor/core    : 5.0.3
   @capacitor/ios     : 5.0.3

Utility:

   cordova-res : not installed globally
   native-run  : 1.7.2

System:

   NodeJS : v16.16.0 (/Users/greg/.nvm/versions/node/v16.16.0/bin/node)
   npm    : 8.11.0
   OS     : macOS Unknown

Additional Information

I couldn't find anything related on SwiperJS docs to change this behaviour. But if I listen directly to touchmove events on the same container, the events are fired, so I expect there is something broken on the ionic side.

ionitron-bot[bot] commented 1 year ago

Thanks for the issue! This issue has been labeled as needs reproduction. This label is added to issues that need a code reproduction.

Please reproduce this issue in an Ionic starter application and provide a way for us to access it (GitHub repo, StackBlitz, etc). Without a reliable code reproduction, it is unlikely we will be able to resolve the issue, leading to it being closed.

If you have already provided a code snippet and are seeing this message, it is likely that the code snippet was not enough for our team to reproduce the issue.

For a guide on how to create a good reproduction, see our Contributing Guide.

liamdebeasi commented 1 year ago

Can you provide a runnable sample? For complex things like gestures, runnable samples help us triage quickly.

greg-md commented 1 year ago

@liamdebeasi here is a sample https://stackblitz.com/edit/angular-7mwuay

greg-md commented 1 year ago

Another thing to mention, if I listen directly to touchmove events, then first 4 events are CustomEvent events if I start swiping from a swiper container. This could be a reason why gesture events are not catch after that.

this.listModal.nativeElement.addEventListener("touchmove", (e: any) => {
  console.log('touchmove', e.touches ? e.touches[0].clientY : null, e);
}, false);
liamdebeasi commented 1 year ago

Thanks, I can reproduce this behavior. The reason this is happening is Swiper is emitting artificial touchstart and touchmove events via CustomEvent. The native events should have information regarding x and y coordinates. Ionic's gesture utility uses this information to calculate whether or not a gesture should activate. Swiper's artificial events do not contain this information, so Ionic's gestures never activates.

This isn't an Ionic bug, so I recommend reporting this to the Swiper team. It's generally a bad practice to interfere or simulate native events since it can cause issues with other libraries that depend on them.

greg-md commented 1 year ago

@liamdebeasi I understand, but why gesture should care about custom events? They might contain anything. I think they just needs to be ignored.

From my knowledge, all non trusted events should be ignored. https://developer.mozilla.org/en-US/docs/Web/API/Event/isTrusted

liamdebeasi commented 1 year ago

I understand, but why gesture should care about custom events?

The gesture utility was created before I started working on this project, but my best guess is that this was just never considered as a use case. That being said, it's probably safe to only listen for trusted events as you noted.

liamdebeasi commented 1 year ago

Actually it looks like this causes issues with testing. Libraries such as Playwright generate trusted events, but Cypress does not seem to. Making this change would likely break developers tests, so this isn't a change we can make.

I'll see if there's an alternative.

greg-md commented 1 year ago

@liamdebeasi I think you can try to validate if the untrusted event meets some requirements, if not, to ignore it.

liamdebeasi commented 1 year ago

The way we'd check is if isTrusted: false. We could check for the existence of other data such as pointer coordinates, but I think that is too brittle since that data can be incorrect when creating an untrusted event.

As a workaround, you can call stopPropagation() on the untrusted events:

<swiper-container style="width: 100%; height: 100px; background: azure;" (touchstart)="suppress($event)">
  ...
</swiper-container>
suppress(ev: any) {
  if (!ev.isTrusted) {
    ev.stopPropagation();
  }
}

A couple things to note about this:

  1. Test this in an app outside of StackBlitz. I noticed some event propagation quirks in StackBlitz, but the workaround worked as expected when I ran the app locally.
  2. This will only work on a device. Swiper still emits touchstart events even when there was never a touch event to begin with. This code works by suppressing the untrusted touchstart event and letting the trusted touchstart event bubble.

The root issue here is Swiper is doing something it should not be doing. This is impacting other libraries that are used in conjunction with Swiper. While it is unfortunate that Swiper is impacting Ionic's gestures in this way, this is not an Ionic bug. I recommend filing feedback with the Swiper team to explain how Swiper's behavior has impacted your application.

greg-md commented 1 year ago

I think it is debatable on which side is the issue. I still think the issue is on the Ionic's side, as Ionic do not handle custom events properly. This way you might have conflicts with any library which introduces untrusted custom events incompatible with ionic's business logic. And they have all the right to do it, cause that's how custom events works and the only common parameter is the detail which could be of any type. 😞 https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent/CustomEvent

liamdebeasi commented 1 year ago

The issue here is not that Ionic is incompatible with CustomEvent. CustomEvent as an API does not impact Ionic's gesture utility. The issue here is Swiper has chosen to use CustomEvent to simulate a native browser event but without the expected payload.

In other words, Swiper is attempting to create a touchstart event, but it is not following the touchstart specification. This is not a pattern Ionic should account for as Swiper's simulated touchstart payload is incorrect.

Swiper can fix this issue on their end by using the TouchEvent constructor to generate an actual touchstart event with the correct payload. The event will not be a trusted event, but the payload will match the specification and Ionic's gesture utility should work as intended.

ionitron-bot[bot] commented 1 year ago

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.