Skip to content

Commit 1c41280

Browse files
committed
fix(infinite-scroll): awaiting for DOM writes before firing infinite scroll event
1 parent 76aa3fb commit 1c41280

File tree

6 files changed

+52
-45
lines changed

6 files changed

+52
-45
lines changed

core/api.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -919,6 +919,7 @@ 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
922923
ion-infinite-scroll,prop,theme,"ios" | "md" | "ionic",undefined,false,false
923924
ion-infinite-scroll,prop,threshold,string,'15%',false,false
924925
ion-infinite-scroll,method,complete,complete() => Promise<void>

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

Lines changed: 44 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ export class InfiniteScroll implements ComponentInterface {
8585
* If `true`, the infinite scroll will preserve the scroll position
8686
* when the content is re-rendered. This is useful when the content is
8787
* re-rendered with new keys, and the scroll position should be preserved.
88-
* @internal
8988
*/
9089
@Prop() preserveRerenderScrollPosition: boolean = false;
9190

@@ -143,15 +142,18 @@ export class InfiniteScroll implements ComponentInterface {
143142

144143
if (distanceFromInfinite < 0) {
145144
if (!this.didFire) {
145+
this.isLoading = true;
146+
this.didFire = true;
147+
146148
if (this.preserveRerenderScrollPosition) {
147149
// Lock the min height of the siblings of the infinite scroll
148150
// if we are preserving the rerender scroll position
149-
this.lockSiblingMinHeight(true);
151+
this.lockSiblingMinHeight(true).then(() => {
152+
this.ionInfinite.emit();
153+
});
154+
} else {
155+
this.ionInfinite.emit();
150156
}
151-
152-
this.isLoading = true;
153-
this.didFire = true;
154-
this.ionInfinite.emit();
155157
return 3;
156158
}
157159
}
@@ -168,32 +170,44 @@ export class InfiniteScroll implements ComponentInterface {
168170
* We preserve existing min-height values, if they're set, so we don't erase what
169171
* has been previously set by the user when we restore after complete is called.
170172
*/
171-
private lockSiblingMinHeight(lock: boolean) {
172-
const siblings = this.el.parentElement?.children || [];
173-
for (const sibling of siblings) {
174-
// Loop through all the siblings of the infinite scroll, but ignore ourself
175-
if (sibling !== this.el && sibling instanceof HTMLElement) {
176-
if (lock) {
177-
const elementHeight = sibling.getBoundingClientRect().height;
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-
}
173+
private lockSiblingMinHeight(lock: boolean): Promise<void> {
174+
return new Promise((resolve) => {
175+
const siblings = this.el.parentElement?.children || [];
176+
const writes: (() => void)[] = [];
177+
178+
for (const sibling of siblings) {
179+
// Loop through all the siblings of the infinite scroll, but ignore ourself
180+
if (sibling !== this.el && sibling instanceof HTMLElement) {
181+
if (lock) {
182+
const elementHeight = sibling.getBoundingClientRect().height;
183+
writes.push(() => {
184+
if (this.minHeightLocked) {
185+
// The previous min height is from us locking it before, so we can disregard it
186+
// We still need to lock the min height if we're already locked, though, because
187+
// the user could have triggered a new load before we've finished the previous one.
188+
const previousMinHeight = sibling.style.minHeight;
189+
if (previousMinHeight) {
190+
sibling.style.setProperty('--ion-previous-min-height', previousMinHeight);
191+
}
192+
}
193+
sibling.style.minHeight = `${elementHeight}px`;
194+
});
195+
} else {
196+
writes.push(() => {
197+
const previousMinHeight = sibling.style.getPropertyValue('--ion-previous-min-height');
198+
sibling.style.minHeight = previousMinHeight || 'auto';
199+
sibling.style.removeProperty('--ion-previous-min-height');
200+
});
186201
}
187-
sibling.style.minHeight = `${elementHeight}px`;
188-
} else {
189-
const previousMinHeight = sibling.style.getPropertyValue('--ion-previous-min-height');
190-
sibling.style.minHeight = previousMinHeight || 'auto';
191-
sibling.style.removeProperty('--ion-previous-min-height');
192202
}
193203
}
194-
}
195204

196-
this.minHeightLocked = lock;
205+
writeTask(() => {
206+
writes.forEach((w) => w());
207+
this.minHeightLocked = lock;
208+
resolve();
209+
});
210+
});
197211
}
198212

199213
/**
@@ -264,8 +278,8 @@ export class InfiniteScroll implements ComponentInterface {
264278
// Unlock the min height of the siblings of the infinite scroll
265279
// if we are preserving the rerender scroll position
266280
if (this.preserveRerenderScrollPosition) {
267-
setTimeout(() => {
268-
this.lockSiblingMinHeight(false);
281+
setTimeout(async () => {
282+
await this.lockSiblingMinHeight(false);
269283
}, 100);
270284
}
271285
}

core/src/components/infinite-scroll/test/preserve-rerender-scroll-position/index.html

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@
6767
}
6868
loading = true;
6969

70-
await wait(300);
7170
replaceAllItems();
7271
infiniteScroll.complete();
7372

@@ -99,7 +98,7 @@
9998

10099
// Add new items with new "keys" (different content/identifiers)
101100
// Start with more items to ensure scrollable content
102-
const totalItems = generation === 1 ? 30 : 30 + generation * 20;
101+
const totalItems = generation === 1 ? 50 : 30 + generation * 20;
103102
itemCount = 0;
104103

105104
for (let i = 0; i < totalItems; i++) {

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

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
1212
const content = page.locator('ion-content');
1313
const items = page.locator('ion-item');
1414
const innerScroll = page.locator('.inner-scroll');
15-
expect(await items.count()).toBe(30);
15+
expect(await items.count()).toBe(50);
1616

1717
let previousScrollTop = 0;
18-
for (let i = 0; i < 20; i++) {
18+
for (let i = 0; i < 30; i++) {
1919
await content.evaluate((el: HTMLIonContentElement) => el.scrollToBottom(0));
2020
const currentScrollTop = await innerScroll.evaluate((el: HTMLIonContentElement) => el.scrollTop);
2121
expect(currentScrollTop).toBeGreaterThan(previousScrollTop);
@@ -26,13 +26,6 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, config }) => {
2626
previousScrollTop
2727
);
2828
previousScrollTop = currentScrollTop;
29-
30-
// Timeout to allow the browser to catch up.
31-
// For some reason, without this, the scroll top gets reset to 0. Adding this
32-
// prevents that, which implies it's an issue with Playwright, not the feature.
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));
3629
}
3730
});
3831
});

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', 'theme', 'threshold'],
930+
inputs: ['disabled', 'mode', 'position', 'preserveRerenderScrollPosition', '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', 'theme', 'threshold'],
938+
inputs: ['disabled', 'mode', 'position', 'preserveRerenderScrollPosition', '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', 'theme', 'threshold'],
955+
inputs: ['disabled', 'mode', 'position', 'preserveRerenderScrollPosition', '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', 'theme', 'threshold'],
963+
inputs: ['disabled', 'mode', 'position', 'preserveRerenderScrollPosition', 'theme', 'threshold'],
964964
standalone: true
965965
})
966966
export class IonInfiniteScroll {

0 commit comments

Comments
 (0)