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: refresher, quickly scrolling after cancelling refresh causes duplicate refresh #25418

Closed
4 of 7 tasks
milanharia opened this issue Jun 7, 2022 · 7 comments · Fixed by #25476
Closed
4 of 7 tasks
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@milanharia
Copy link

milanharia commented Jun 7, 2022

Prerequisites

Ionic Framework Version

  • v4.x
  • v5.x
  • v6.x
  • Nightly

Current Behavior

When using an Ion Refresher on android, if you pull the refresher down partially (so it does not trigger the onIonRefresh event) and then scroll down quickly, the refresher animation triggers (but does not fire the refresh event) which causes the scroll to jank.

Expected Behavior

Scrolling down rapidly should not cause the refresher to appear.

Steps to Reproduce

Take any ionic app page which has enough content so the page can scroll and add an IonRefresher. Swipe down on the page slightly so the refresher icon begins to enter the page (but do not trigger it). Swipe up rapidly to scroll the page down quickly and the refresher icon will enter the page and cause the scrolling to jank.

Video Example

22-06-07-11-03-01_01.mp4

An example of the code:


function doRefresh(event: CustomEvent<RefresherEventDetail>) {
  console.log("Begin async operation");

  setTimeout(() => {
    console.log('Async operation has ended');
    event.detail.complete();
  }, 2000);
}

const Tab1: React.FC = () => {
  return (
    <IonPage>
      <IonHeader>
        <IonToolbar>
          <IonTitle>Tab 1</IonTitle>
        </IonToolbar>
      </IonHeader>
      <IonContent fullscreen>
        <IonHeader collapse="condense">
          <IonToolbar>
            <IonTitle size="large">Tab 1</IonTitle>
          </IonToolbar>
        </IonHeader>
        <IonRefresher slot="fixed" onIonRefresh={doRefresh}>
          <IonRefresherContent />
        </IonRefresher>
           {*/ ...Page Content */}
      </IonContent>
    </IonPage>
  );
};

Code Reproduction URL

https://github.com/milanharia/ionic-android-refresher

Ionic Info

Ionic:

Ionic CLI : 6.19.1 (/usr/local/lib/node_modules/@ionic/cli)
Ionic Framework : @ionic/react 6.1.8

Capacitor:

Capacitor CLI : 3.5.1
@capacitor/android : 3.5.1
@capacitor/core : 3.5.1
@capacitor/ios : not installed

Utility:

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

System:

NodeJS : v16.14.2 (/usr/local/bin/node)
npm : 8.7.0
OS : macOS Monterey

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Jun 7, 2022
@liamdebeasi liamdebeasi added the ionitron: needs reproduction a code reproduction is needed from the issue author label Jun 7, 2022
@ionitron-bot
Copy link

ionitron-bot bot commented Jun 7, 2022

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.

@ionitron-bot ionitron-bot bot removed the triage label Jun 7, 2022
@milanharia
Copy link
Author

I have created a repo which has this issue within it here. Make sure after pulling the refresher down slightly you scroll down quickly for the bug to occur.

22-06-09-11-56-40.mp4

@averyjohnston averyjohnston added triage and removed ionitron: needs reproduction a code reproduction is needed from the issue author labels Jun 9, 2022
@liamdebeasi liamdebeasi self-assigned this Jun 15, 2022
@liamdebeasi
Copy link
Contributor

Thanks, I can reproduce this. The refresher has a guard built in to protect from the gesture activating when dragging up: https://github.com/ionic-team/ionic-framework/blob/main/core/src/components/refresher/refresher.tsx#L343

However, it requires that the overall progress of the gesture be 0 (I.e. not started). This is done to still allow for users to cancel the gesture by dragging up mid-gesture.

When you do not pull enough for the gesture to complete, the browser executes this section of the code: https://github.com/ionic-team/ionic-framework/blob/main/core/src/components/refresher/refresher.tsx#L379-L383

While we do wait for the animation to finish, we never reset the overall progress of the animation. As a result, the guard mentioned above never kicks in and another gesture begins. Setting this.progress = 0; in the onStart callback fixes the issue.

@liamdebeasi liamdebeasi changed the title bug: Ion Refresher triggers incorrectly on android bug: refresher, quickly scrolling after cancelling refresh causes duplicate refresh Jun 15, 2022
@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report labels Jun 15, 2022
@liamdebeasi
Copy link
Contributor

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

npm install @ionic/react@6.1.10-dev.11655302783.1c4fffdb @ionic/react-router@6.1.10-dev.11655302783.1c4fffdb

@milanharia
Copy link
Author

I can confirm it is fixed in the dev build!

@liamdebeasi
Copy link
Contributor

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

@ionitron-bot
Copy link

ionitron-bot bot commented Jul 15, 2022

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 Jul 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants