Ionic-framework: bug(slides): ion-button not working after page scrolled

Created on 4 Dec 2018  Â·  9Comments  Â·  Source: ionic-team/ionic-framework

Bug Report

Ionic version:


[x] 4.x

I'm submitting a ...

[x] bug report
[ ] feature request

Current behavior:
If you have a button inside ion-slides and press it, all is working. But if you scroll on the page and then go to the button again and press it, it's not working anymore. See the following gif: Press button working, now I scroll go back and click again - not working.

We have to find out if this is an issue of swiper.js or ion-slides / ion-button. If I lock the swipes ( this.slides.lockSwipes(true)) the button is working. Maybe it's the scrolling of ion-slides, that is blocking the clicking?

2018-12-04_11-16-27

Expected behavior:
Button should be tappable after scrolling in ion-slides

Steps to reproduce:
@persn ( #15301 ) created an repository for that which is a good example -> https://github.com/persn/ionic-slides-scroll-bug

If you tap the button, sometimes it works, but after scrolling, it will not.

core bug

Most helpful comment

I have a PR coming soon, but depends on a PR in @types/swiper first.

We can set the touchStartPreventDefault Swiper API param to false by default in slides.tsx to effectively do what my suggested fix wants to do without any drastic changes to swiper code.

Workaround:
In the meantime, you can explicitly add the following param to your ion-slides options property:

your.page.html

<ion-slides [options]="swiperParams">
  ...
</ion-slides>

your.page.ts

@Component({
  templateUrl: './your.page.html'
  ...
})
export class YourPage {
  swiperParams = {
    touchStartPreventDefault: false
  }
}

All 9 comments

Appears to be a bug with swiper's onTouchStart().

I will follow up with them to see if touchstart can be simply replaced with swiper's touchEvents.start which adapts to whether it is listening to touch or mouse events based on device/browser and the simulateTouch API param.

What's happening in this bug:

  1. Scroll event inside of ion-content emits ionScrollStart()
  2. tap-click bound to ionScrollStart sets cancelled = true on scroll
  3. On the mousedown event after scrolling in an ion-slide, swiper's onTouchStart() preventDefault's the event because the event is not a touchstart (this is where I think the bug is — a mousedown should probably should behave the same way as touchstart).
  4. The event isn't handled and doesn't propagate to tap-click's onMouseDown(), which would set cancelled = false in pointerDown(), but doesn't.
  5. tap-click's onBodyClick() will eat all remaining click events due to cancelled flag still erroneously set. In this case, clicking on the ion-button inside the slide won't receive the click event because of the stopPropagation() call inside onBodyClick().

This bug is isolated to the usage of both ion-content and ion-slides in the same document, e.g.:
```


...



If you replace ``ion-content`` with ``<div style="overflow: auto">``, click events will propagate as normal on buttons, etc.:



...


```

I have a PR coming soon, but depends on a PR in @types/swiper first.

We can set the touchStartPreventDefault Swiper API param to false by default in slides.tsx to effectively do what my suggested fix wants to do without any drastic changes to swiper code.

Workaround:
In the meantime, you can explicitly add the following param to your ion-slides options property:

your.page.html

<ion-slides [options]="swiperParams">
  ...
</ion-slides>

your.page.ts

@Component({
  templateUrl: './your.page.html'
  ...
})
export class YourPage {
  swiperParams = {
    touchStartPreventDefault: false
  }
}

@CFT-Chris Amazing debugging! I will also look into tap-click so it's more robust in this sense, so other external libraries are less likely to break ionic!

Will take a look at your PR tomorrow! Good job!

@manucorporat Thanks!

I submitted a new test for slides in the PR which needed to include tap-click. I noticed that for all other tests though, tap-click is excluded via the _testing flag. I may have missed an important reason for this. I elected to ignore the flag to make the test give proper results, but let me know if it is necessary to keep _testing=true in e2e.ts.

I think if tap-click can be included in some tests it will help to immediately pin down a few bugs and compatibility issues with future versions of external libraries.

I can confirm that setting touchStartPreventDefault=true solve this issue.

Hi there,

Apologies for the delay! @CFT-Chris, are you still experiencing this issue? I am trying to reproduce with the repo you provided, but everything seems to be working fine (tested on desktop Chrome and mobile Safari).

Thanks!

Hi @liamdebeasi,

I ran the test I made in the PR without the touchStartPreventDefault set to false by default, and it passes the test. So it technically doesn't repro anymore, and it appears to be fixed by #17170.

However, I can confirm that there is still some, albeit minor, undesirable behavior without the changes in my PR: the button effect doesn't show after scrolling the ion-content and then clicking the ion-button. The (click) action though does execute, changing the background to blue in the test. If touchStartPreventDefault = false, the ion-button will correctly show the swoop animation effect on click.

Not sure if that's worth checking in the fix, but if not, perhaps just integrating the test to catch future bugs might be helpful.

Also it might also be helpful to merge the change in the PR's package.json. It updates the required dev dependency swiper/types since slides.tsx was failing to build for me with a typescript error. This was happening without my touchStartPreventDefault change (updateAutoHeight added by #17208 caused my build to break without latest swiper/types).

Hi there,

This issue has been fixed via https://github.com/ionic-team/ionic/pull/16728 and will be in the next release of Ionic.

Thanks!

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.

Was this page helpful?
0 / 5 - 0 ratings