Skip to content

Commit 76aa3fb

Browse files
committed
fix(infinite-scroll): prevent locking sibling min height while it's already locked, prevent useless setTimeout from being set up while preserve rerender scroll property isn't set
1 parent f9e7dcd commit 76aa3fb

File tree

5 files changed

+34
-23
lines changed

5 files changed

+34
-23
lines changed

core/api.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -919,7 +919,6 @@ ion-infinite-scroll,none
919919
ion-infinite-scroll,prop,disabled,boolean,false,false,false
920920
ion-infinite-scroll,prop,mode,"ios" | "md",undefined,false,false
921921
ion-infinite-scroll,prop,position,"bottom" | "top",'bottom',false,false
922-
ion-infinite-scroll,prop,preserveRerenderScrollPosition,boolean,false,false,false
923922
ion-infinite-scroll,prop,theme,"ios" | "md" | "ionic",undefined,false,false
924923
ion-infinite-scroll,prop,threshold,string,'15%',false,false
925924
ion-infinite-scroll,method,complete,complete() => Promise<void>

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

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export class InfiniteScroll implements ComponentInterface {
1616
private thrPx = 0;
1717
private thrPc = 0;
1818
private scrollEl?: HTMLElement;
19+
private minHeightLocked = false;
1920

2021
/**
2122
* didFire exists so that ionInfinite
@@ -84,6 +85,7 @@ export class InfiniteScroll implements ComponentInterface {
8485
* If `true`, the infinite scroll will preserve the scroll position
8586
* when the content is re-rendered. This is useful when the content is
8687
* re-rendered with new keys, and the scroll position should be preserved.
88+
* @internal
8789
*/
8890
@Prop() preserveRerenderScrollPosition: boolean = false;
8991

@@ -141,13 +143,14 @@ export class InfiniteScroll implements ComponentInterface {
141143

142144
if (distanceFromInfinite < 0) {
143145
if (!this.didFire) {
146+
if (this.preserveRerenderScrollPosition) {
147+
// Lock the min height of the siblings of the infinite scroll
148+
// if we are preserving the rerender scroll position
149+
this.lockSiblingMinHeight(true);
150+
}
151+
144152
this.isLoading = true;
145153
this.didFire = true;
146-
147-
// Lock the min height of the siblings of the infinite scroll
148-
// if we are preserving the rerender scroll position
149-
this.lockSiblingMinHeight(true);
150-
151154
this.ionInfinite.emit();
152155
return 3;
153156
}
@@ -166,19 +169,20 @@ export class InfiniteScroll implements ComponentInterface {
166169
* has been previously set by the user when we restore after complete is called.
167170
*/
168171
private lockSiblingMinHeight(lock: boolean) {
169-
if (!this.preserveRerenderScrollPosition) {
170-
return;
171-
}
172-
173-
// Loop through all the siblings of the infinite scroll, but ignore the infinite scroll itself
174172
const siblings = this.el.parentElement?.children || [];
175173
for (const sibling of siblings) {
174+
// Loop through all the siblings of the infinite scroll, but ignore ourself
176175
if (sibling !== this.el && sibling instanceof HTMLElement) {
177176
if (lock) {
178177
const elementHeight = sibling.getBoundingClientRect().height;
179-
const previousMinHeight = sibling.style.minHeight;
180-
if (previousMinHeight) {
181-
sibling.style.setProperty('--ion-previous-min-height', previousMinHeight);
178+
if (this.minHeightLocked) {
179+
// The previous min height is from us locking it before, so we can disregard it
180+
// We still need to lock the min height if we're already locked, though, because
181+
// the user could have triggered a new load before we've finished the previous one.
182+
const previousMinHeight = sibling.style.minHeight;
183+
if (previousMinHeight) {
184+
sibling.style.setProperty('--ion-previous-min-height', previousMinHeight);
185+
}
182186
}
183187
sibling.style.minHeight = `${elementHeight}px`;
184188
} else {
@@ -188,6 +192,8 @@ export class InfiniteScroll implements ComponentInterface {
188192
}
189193
}
190194
}
195+
196+
this.minHeightLocked = lock;
191197
}
192198

193199
/**
@@ -257,9 +263,11 @@ export class InfiniteScroll implements ComponentInterface {
257263

258264
// Unlock the min height of the siblings of the infinite scroll
259265
// if we are preserving the rerender scroll position
260-
setTimeout(() => {
261-
this.lockSiblingMinHeight(false);
262-
}, 100);
266+
if (this.preserveRerenderScrollPosition) {
267+
setTimeout(() => {
268+
this.lockSiblingMinHeight(false);
269+
}, 100);
270+
}
263271
}
264272

265273
private canStart(): boolean {

core/src/components/infinite-scroll/test/preserve-rerender-scroll-position/infinite-scroll.e2e.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { expect } from '@playwright/test';
22
import { configs, test } from '@utils/test/playwright';
33

4+
test.setTimeout(100000);
5+
46
configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
57
test.describe(title('infinite-scroll: preserve rerender scroll position'), () => {
68
test('should load more items when scrolled to the bottom', async ({ page }) => {
@@ -13,7 +15,7 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
1315
expect(await items.count()).toBe(30);
1416

1517
let previousScrollTop = 0;
16-
for (let i = 0; i < 10; i++) {
18+
for (let i = 0; i < 20; i++) {
1719
await content.evaluate((el: HTMLIonContentElement) => el.scrollToBottom(0));
1820
const currentScrollTop = await innerScroll.evaluate((el: HTMLIonContentElement) => el.scrollTop);
1921
expect(currentScrollTop).toBeGreaterThan(previousScrollTop);
@@ -28,7 +30,9 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
2830
// Timeout to allow the browser to catch up.
2931
// For some reason, without this, the scroll top gets reset to 0. Adding this
3032
// prevents that, which implies it's an issue with Playwright, not the feature.
31-
await new Promise((resolve) => setTimeout(resolve, 100));
33+
// For some reason, this delay needs to be longer than the time required to
34+
// reset the minimum height in infinite scroll.
35+
await new Promise((resolve) => setTimeout(resolve, 1001));
3236
}
3337
});
3438
});

packages/angular/src/directives/proxies.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -927,15 +927,15 @@ export declare interface IonImg extends Components.IonImg {
927927

928928

929929
@ProxyCmp({
930-
inputs: ['disabled', 'mode', 'position', 'preserveRerenderScrollPosition', 'theme', 'threshold'],
930+
inputs: ['disabled', 'mode', 'position', 'theme', 'threshold'],
931931
methods: ['complete']
932932
})
933933
@Component({
934934
selector: 'ion-infinite-scroll',
935935
changeDetection: ChangeDetectionStrategy.OnPush,
936936
template: '<ng-content></ng-content>',
937937
// eslint-disable-next-line @angular-eslint/no-inputs-metadata-property
938-
inputs: ['disabled', 'mode', 'position', 'preserveRerenderScrollPosition', 'theme', 'threshold'],
938+
inputs: ['disabled', 'mode', 'position', 'theme', 'threshold'],
939939
})
940940
export class IonInfiniteScroll {
941941
protected el: HTMLIonInfiniteScrollElement;

packages/angular/standalone/src/directives/proxies.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -952,15 +952,15 @@ export declare interface IonImg extends Components.IonImg {
952952

953953
@ProxyCmp({
954954
defineCustomElementFn: defineIonInfiniteScroll,
955-
inputs: ['disabled', 'mode', 'position', 'preserveRerenderScrollPosition', 'theme', 'threshold'],
955+
inputs: ['disabled', 'mode', 'position', 'theme', 'threshold'],
956956
methods: ['complete']
957957
})
958958
@Component({
959959
selector: 'ion-infinite-scroll',
960960
changeDetection: ChangeDetectionStrategy.OnPush,
961961
template: '<ng-content></ng-content>',
962962
// eslint-disable-next-line @angular-eslint/no-inputs-metadata-property
963-
inputs: ['disabled', 'mode', 'position', 'preserveRerenderScrollPosition', 'theme', 'threshold'],
963+
inputs: ['disabled', 'mode', 'position', 'theme', 'threshold'],
964964
standalone: true
965965
})
966966
export class IonInfiniteScroll {

0 commit comments

Comments
 (0)