From 8c235fd30c50f317de1f37f69068507aa0979068 Mon Sep 17 00:00:00 2001 From: Liam DeBeasi Date: Tue, 5 Dec 2023 12:20:49 -0500 Subject: [PATCH] fix(infinite-scroll): remaining in threshold after ionInfinite can trigger event again on scroll (#28569) Issue number: resolves #18071 --------- ## What is the current behavior? When adding elements to the DOM in the `ionInfinite` callback, scrolling again would sometimes not cause `ionInfinite` to trigger again. We [set the didFire flag to `true`](https://github.com/ionic-team/ionic-framework/blob/388d19e04f83f85abd4602adb04cc71ac575764a/core/src/components/infinite-scroll/infinite-scroll.tsx#L126) before calling `ionInfinite`. This flag ensures that `ionInfinite` is not called multiple times if users continue to scroll after `ionInfinite` is fired but before the `complete` method is called. The [didFire flag is reset](https://github.com/ionic-team/ionic-framework/blob/388d19e04f83f85abd4602adb04cc71ac575764a/core/src/components/infinite-scroll/infinite-scroll.tsx#L131) once the user scrolls outside of the threshold. Normally this is fine: If an application adds several new items to a list the current scroll position will be outside of the threshold. However, if the scroll position remains in the threshold (such as if an application append a small number of new items to a list) then the `didFire` flag will not get reset. Additionally, there are some instances where the scroll position restoration when `position="top"` may not work which can cause this bug to trigger as well. For example, if users quickly scroll to the top, the scroll position will not be restored correctly and the scroll position will still be at the top of the screen. That is another instance where this bug can trigger even if a large number of items were added to the DOM. ## What is the new behavior? - The `didFire` flag is reset when the `complete` method is called. This ensures that even if the scroll position is still in the threshold `ionInfinite` can fire again. Note that developers may notice `ionInfinite` firing more times as a result of this change. This can happen when appending a small number of items to the DOM such that the scroll position remains in the threshold. Previously `ionInfinite` would not fire again, but now it does since users are scrolling in the threshold. I decided to target this change for a minor release to minimize any surprises for developers. ## Does this introduce a breaking change? - [ ] Yes - [x] No ## Other information Dev build: `7.5.4-dev.11700602203.1e7155a1` --------- Co-authored-by: Maria Hutt --- .../infinite-scroll/infinite-scroll.tsx | 13 +++- .../test/small-dom-update/index.html | 63 +++++++++++++++++++ .../small-dom-update/infinite-scroll.e2e.ts | 34 ++++++++++ 3 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 core/src/components/infinite-scroll/test/small-dom-update/index.html create mode 100644 core/src/components/infinite-scroll/test/small-dom-update/infinite-scroll.e2e.ts diff --git a/core/src/components/infinite-scroll/infinite-scroll.tsx b/core/src/components/infinite-scroll/infinite-scroll.tsx index 190fab5863..25de713eeb 100644 --- a/core/src/components/infinite-scroll/infinite-scroll.tsx +++ b/core/src/components/infinite-scroll/infinite-scroll.tsx @@ -12,6 +12,14 @@ export class InfiniteScroll implements ComponentInterface { private thrPx = 0; private thrPc = 0; private scrollEl?: HTMLElement; + + /** + * didFire exists so that ionInfinite + * does not fire multiple times if + * users continue to scroll after + * scrolling into the infinite + * scroll threshold. + */ private didFire = false; private isBusy = false; @@ -127,8 +135,6 @@ export class InfiniteScroll implements ComponentInterface { this.ionInfinite.emit(); return 3; } - } else { - this.didFire = false; } return 4; @@ -190,10 +196,13 @@ export class InfiniteScroll implements ComponentInterface { writeTask(() => { scrollEl.scrollTop = newScrollTop; this.isBusy = false; + this.didFire = false; }); }); }); }); + } else { + this.didFire = false; } } diff --git a/core/src/components/infinite-scroll/test/small-dom-update/index.html b/core/src/components/infinite-scroll/test/small-dom-update/index.html new file mode 100644 index 0000000000..16a67ae763 --- /dev/null +++ b/core/src/components/infinite-scroll/test/small-dom-update/index.html @@ -0,0 +1,63 @@ + + + + + Infinite Scroll - Small DOM Update + + + + + + + + + + + + +
+ + + + + + + + + + + diff --git a/core/src/components/infinite-scroll/test/small-dom-update/infinite-scroll.e2e.ts b/core/src/components/infinite-scroll/test/small-dom-update/infinite-scroll.e2e.ts new file mode 100644 index 0000000000..c76140657d --- /dev/null +++ b/core/src/components/infinite-scroll/test/small-dom-update/infinite-scroll.e2e.ts @@ -0,0 +1,34 @@ +import { expect } from '@playwright/test'; +import { configs, test } from '@utils/test/playwright'; + +configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => { + test.describe(title('infinite-scroll: appending small amounts to dom'), () => { + test('should load more after remaining in threshold', async ({ page }) => { + await page.goto('/src/components/infinite-scroll/test/small-dom-update', config); + + const ionInfiniteComplete = await page.spyOnEvent('ionInfiniteComplete'); + const content = page.locator('ion-content'); + const items = page.locator('#list .item'); + expect(await items.count()).toBe(30); + + await content.evaluate((el: HTMLIonContentElement) => el.scrollToBottom(0)); + await ionInfiniteComplete.next(); + + /** + * Even after appending we'll still be within + * the infinite scroll's threshold + */ + expect(await items.count()).toBe(33); + + await content.evaluate((el: HTMLIonContentElement) => el.scrollToBottom(0)); + await ionInfiniteComplete.next(); + + /** + * Scrolling down again without leaving + * the threshold should still trigger + * infinite scroll again. + */ + expect(await items.count()).toBe(36); + }); + }); +});