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.02k stars 13.52k forks source link

bug: infinite-scroll, didFire can get stuck at true when items are removed dynamically #28272

Open fcirillo6z opened 1 year ago

fcirillo6z commented 1 year ago

Prerequisites

Describe the Feature Request

Hi,

Infinite scroll component can "lock" if used on a scrollable element whose height changes by removing elements.

In the docs is not clear whether the component officially supports removing items/reducing height, as I always used it in the classic way of just showing more content by expanding it.

In my case, without a API hook to reset state, everytime I change the content size I simply remove it from the DOM with a *ngIf, and then i put it back.

Would be nice to have a reset() method or something like that.

Describe the Use Case

After the first time you triggered the ionInfinite event, and correctly added stuff and call complete(), if for some reason you remove stuff and reduce height to 1 "page" you are stuck unable to call more content, as the infinitescroll component is in the "didFire ==true" state (you're still in the threshold zone), and wont fire the event again. The component expects the scroll component to expand and only resets state when the scroll position exits the threshold zone.

Describe Preferred Solution

The easiest way is to let the component fire the ionInfinite event in that edge case by resetting that particular state (didFire) via a public method, so it can be called every time we reduce height.

Describe Alternatives

A workaround to this behaviour is removing the infinite scroll component from the DOM and reinsert it.

Related Code

stackblitz

To reproduce pls trigger infinite scroll, then press "reduce elements" till it stops working.

Additional Information

No response

averyjohnston commented 1 year ago

Thank you for the request. Could you post a Github or Stackblitz link showing the issue you're running into? It's possible this is just a bug we should fix.

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.

fcirillo6z commented 1 year ago

Thank you for the request. Could you post a Github or Stackblitz link showing the issue you're running into? It's possible this is just a bug we should fix.

Thank you! I added a stackblitz to the issue.

averyjohnston commented 1 year ago

Thank you, and apologies for the delay in responding. I'm able to replicate the behavior you're describing, though I had to change the slice index to 16 instead of 19 on my end (likely due to differences in MD vs. iOS and/or screen size). The issue lies in this block here: https://github.com/ionic-team/ionic-framework/blob/3d96ccb7b29d1974ab284d346cac64a143ebf41c/core/src/components/infinite-scroll/infinite-scroll.tsx#L123-L132

Once the items are removed from the list, there are enough items to allow scrolling, but you can't scroll up far enough to cause distanceFromInfinite to ever rise above 0. This means that didFire never gets reset to false, so the ionInfinite event can't fire again. Ideally, we should just fix this rather than introduce a new API. We may need to tweak how distanceFromInfinite is calculated, or we could try resetting didFire if the height of the scroll container changes.

I was able to reproduce this in core, meaning it isn't an Angular-exclusive issue. For anyone testing locally, here are the exact steps I took:

  1. Host core locally and head to http://localhost:3333/src/components/infinite-scroll/test/basic
  2. Scroll down to trigger the infinite scroll (but do not scroll back up -- doing so will reset didFire back to false and prevent the bug from happening)
  3. Run the following code in the console to remove some of the items:
    ;(() => {
    let items = document.querySelectorAll('ion-list > ion-item');
    items = [...items].slice(17, items.length); // may need to tweak this number for your device
    items.forEach(item => item.remove());
    })();
  4. Scroll up and down. Note that the infinite scroll no longer triggers.