8000 fix(infinite-scroll): remaining in threshold after ionInfinite can tr… · ionic-team/ionic-framework@8c235fd · GitHub
[go: up one dir, main page]
Skip to content

Commit 8c235fd

Browse files
liamdebeasithetaPC
liamdebeasi
and
thetaPC
authored
fix(infinite-scroll): remaining in threshold after ionInfinite can trigger event again on scroll (#28569)
Issue number: resolves #18071 --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> 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? <!-- Please describe the behavior or changes that are being added by this PR. --> - 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 <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> Dev build: `7.5.4-dev.11700602203.1e7155a1` --------- Co-authored-by: Maria Hutt <thetaPC@users.noreply.github.com>
1 parent 65106ce commit 8c235fd

File tree

3 files changed

+108
-2
lines changed

3 files changed

+108
-2
lines changed

‎core/src/components/infinite-scroll/infinite-scroll.tsx

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,14 @@ export class InfiniteScroll implements ComponentInterface {
1212
private thrPx = 0;
1313
private thrPc = 0;
1414
private scrollEl?: HTMLElement;
15+
16+
/**
17+
* didFire exists so that ionInfinite
18+
* does not fire multiple times if
19+
* users continue to scroll after
20+
* scrolling into the infinite
21+
* scroll threshold.
22+
*/
1523
private didFire = false;
1624
private isBusy = false;
1725

@@ -127,8 +135,6 @@ export class InfiniteScroll implements ComponentInterface {
127135
this.ionInfinite.emit();
128136
return 3;
129137
}
130-
} else {
131-
this.didFire = false;
132138
}
133139

134140
return 4;
@@ -190,10 +196,13 @@ export class InfiniteScroll implements ComponentInterface {
190196
writeTask(() => {
191197
scrollEl.scrollTop = newScrollTop;
192198
this.isBusy = false;
199+
this.didFire = false;
193200
});
194201
});
195202
});
196203
});
204+
} else {
205+
this.didFire = false;
197206
}
198207
}
199208

‎core/src/components/infinite-scroll/test/small-dom-update/index.html

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<!DOCTYPE html>
2+
<html lang="en" dir="ltr">
3+
<head>
4+
<meta charset="UTF-8" />
5+
<title>Infinite Scroll - Small DOM Update</title>
6+
<meta
7+
name="viewport"
8+
content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=1.0, user-scalable=no"
9+
/>
10+
<link href="../../../../../css/ionic.bundle.css" rel="stylesheet" />
11+
<link href="../../../../../scripts/testing/styles.css" rel="stylesheet" />
12+
<script src="../../../../../scripts/testing/scripts.js"></script>
13+
<script nomodule src="../../../../../dist/ionic/ionic.js"></script>
14+
<script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
15+
<style>
16+
#list .item {
17+
width: 100%;
18+
border-bottom: 1px solid gray;
19+
20+
padding: 10px;
21+
}
22+
</style>
23+
</head>
24+
25+
<body>
26+
<ion-app>
27+
<ion-content class="ion-padding" id="content">
28+
<div id="list"></div>
29+
30+
<ion-infinite-scroll threshold="100px" id="infinite-scroll">
31+
<ion-infinite-scroll-content loading-spinner="crescent" loading-text="Loading more data...">
32+
</ion-infinite-scroll-content>
33+
</ion-infinite-scroll>
34+
</ion-content>
35+
</ion-app>
36+
37+
<script>
38+
const list = document.getElementById('list');
39+
const infiniteScroll = document.getElementById('infinite-scroll');
40+
41+
infiniteScroll.addEventListener('ionInfinite', () => {
42+
setTimeout(() => {
43+
appendItems();
44+
45+
infiniteScroll.complete();
46+
47+
// Custom event consumed in the e2e tests
48+
window.dispatchEvent(new CustomEvent('ionInfiniteComplete'));
49+
}, 500);
50+
});
51+
52+
function appendItems(count = 3) {
53+
for (var i = 0; i < count; i++) {
54+
const el = document.createElement('div');
55+
el.classList.add('item');
56+
el.textContent = `${1 + i}`;
57+
list.appendChild(el);
58+
}
59+
}
60+
appendItems(30);
61+
</script>
62+
</body>
63+
</html>

‎core/src/components/infinite-scroll/test/small-dom-update/infinite-scroll.e2e.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import { expect } from '@playwright/test';
2+
import { configs, test } from '@utils/test/playwright';
3+
4+
configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
5+
test.describe(title('infinite-scroll: appending small amounts to dom'), () => {
6+
test('should load more after remaining in threshold', async ({ page }) => {
7+
await page.goto('/src/components/infinite-scroll/test/small-dom-update', config);
8+
9+
const ionInfiniteComplete = await page.spyOnEvent('ionInfiniteComplete');
10+
const content = page.locator('ion-content');
11+
const items = page.locator('#list .item');
12+
expect(await items.count()).toBe(30);
13+
14+
await content.evaluate((el: HTMLIonContentElement) => el.scrollToBottom(0));
15+
await ionInfiniteComplete.next();
16+
17+
/**
18+
* Even after appending we'll still be within
19+
* the infinite scroll's threshold
20+
*/
21+
expect(await items.count()).toBe(33);
22+
23+
await content.evaluate((el: HTMLIonContentElement) => el.scrollToBottom(0));
24+
await ionInfiniteComplete.next();
25+
26+
/**
27+
* Scrolling down again without leaving
28+
* the threshold should still trigger
29+
* infinite scroll again.
30+
*/
31+
expect(await items.count()).toBe(36);
32+
});
33+
});
34+
});

0 commit comments

Comments
 (0)
0