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
50.93k stars 13.51k forks source link

bug: ios native refresher overlaps content when hiding #22358

Open EinfachHans opened 3 years ago

EinfachHans commented 3 years ago

Bug Report

It MAY be related to #21291, if so pls close. But the behaviour is different, so i'm opening it to be sure

Ionic version:

[ ] 4.x [x] 5.x

Current behavior:

Please have a look into the attached video:

  1. When the content (list in this case) changes/ is reassigned while refreshing the content kind of jumps to the top and back.
  2. When the refresher is refreshing and i scroll up again, the spinner hides to late and don't hide behind the content. I think it is overlayed by the content in native ios (see at the end of the video)

Expected behavior:

  1. to not happen^^
  2. to hide the spinner behind the content when scrolling up

Reproduction / Video:

Video: Uploaded here

Repo of the App seen in the video: https://github.com/EinfachHans/ion-refresher-bug2

Other information:

Ionic info (of the test repo):

Ionic CLI                     : 6.12.0 (/usr/local/lib/node_modules/@ionic/cli)
   Ionic Framework               : @ionic/angular 5.4.0
   @angular-devkit/build-angular : 0.1000.8
   @angular-devkit/schematics    : 10.0.8
   @angular/cli                  : 10.0.8
   @ionic/angular-toolkit        : 2.3.3

Cordova:

   Cordova CLI       : 10.0.0
   Cordova Platforms : ios 6.1.1
   Cordova Plugins   : cordova-plugin-ionic-keyboard 2.2.0, cordova-plugin-ionic-webview 4.2.1, (and 4 other plugins)

Utility:

   cordova-res                          : 0.15.1
   native-run (update available: 1.2.2) : 1.0.0

System:

   Android SDK Tools : 25.2.3 (/Users/hanskrywalsky/Library/Android/sdk)
   ios-deploy        : 1.10.0
   ios-sim           : 8.0.2
   NodeJS            : v12.18.3 (/usr/local/bin/node)
   npm               : 6.14.8
   OS                : macOS Catalina
   Xcode             : Xcode 12.0.1 Build version 12A7300
liamdebeasi commented 3 years ago

Thanks for the issue.

Issue 1 is that WebKit issue that causes scrolling to be reset: https://bugs.webkit.org/show_bug.cgi?id=209968. The good news is that this has been fixed and will be in an upcoming version of iOS!

Issue 2 probably needs an adjustment to the pull to refresh threshold for showing/hiding the spinner.

ionitron-bot[bot] commented 3 years ago

This issue has been labeled as help wanted. This label is added to issues that we believe would be good for contributors.

If you'd like to work on this issue, please comment here letting us know that you would like to submit a pull request for it. This helps us to keep track of the pull request and make sure there isn't duplicated effort.

For a guide on how to create a pull request and test this project locally to see your changes, see our contributing documentation.

Thank you!

EinfachHans commented 3 years ago

@liamdebeasi just looked into this. The "native feeling" can easily set by setting a z-index on the scrollEl of the IonContent, that should be higher than the one of the native refresher (currently 1). Is there a reason why IonContent's scrollEl doesn't have a z-index? Can it be easily added?

liamdebeasi commented 3 years ago

If we add a higher z-index on the scrollEl in ion-content, wouldn't that end up covering the refresher entirely?

EinfachHans commented 3 years ago

@liamdebeasi This is the Refresher like it is at the moment (without z-index specified on scrollEl of ion-content and with z-index 1 on refresher-native):

This is with setting z-index of refresher-native to 0 and the one of ion-content's scrollEl to 1:

eduardoRoth commented 3 years ago

Thanks for the issue.

Issue 1 is that WebKit issue that causes scrolling to be reset: https://bugs.webkit.org/show_bug.cgi?id=209968. The good news is that this has been fixed and will be in an upcoming version of iOS!

Issue 2 probably needs an adjustment to the pull to refresh threshold for showing/hiding the spinner.

@liamdebeasi For Issue 1 I can confirm this issue still happens. I'm testing it with iOS 12 and iOS 14.5 and it's still there. Your workaround (https://github.com/ionic-team/ionic-framework/issues/21291#issuecomment-629244683) works partially, if I go all the way down the refresher "bumps" and the spinner is put on top of the content (kind of like on android), and then it doesn't disappear.

EinfachHans commented 2 years ago

@liamdebeasi Small follow up: Setting z-index: 2 here would fix this issue here, as the ion-refresher.refresher-native sets z-index: 1.

But i'm unsure about the comment above and your PR if this will break anything? 🤔.

mikelhamer commented 2 months ago

Is this still relevant?

EinfachHans commented 2 months ago

@mikelhamer yes, it is

mikelhamer commented 2 months ago

Thanks. I'm a big lost trying to wrap my head around what the current state of the issue is / what needs to be addressed. I want to help, but not sure where to start.