From 4dcec4b9c764974f145c0afa6447518300d08c2f Mon Sep 17 00:00:00 2001 From: Harvey Tindall Date: Fri, 23 May 2025 13:53:16 +0100 Subject: [PATCH] accounts: fix infinite scroll over-loading, use scrollend+polyfill calculation for number of rows to be drawn was wrong, fixed now. To compensate for overshooting with fast scrolling, speed is calculated using previous scrollY in rows/scroll, and used to render more rows. Also, the "scrollend" event is used to load more at the end of a scroll always. Since this isn't available on safari/webkit(2gtk), a polyfill has been added. --- package-lock.json | 12 +++++++ package.json | 1 + ts/admin.ts | 11 ++++--- ts/modules/list.ts | 79 ++++++++++++++++++++++++++++++++++------------ ts/modules/tabs.ts | 3 +- ts/typings/d.ts | 2 +- 6 files changed, 82 insertions(+), 26 deletions(-) diff --git a/package-lock.json b/package-lock.json index cd02357..23aaab8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,6 +9,7 @@ "version": "1.0.0", "license": "ISC", "dependencies": { + "@af-utils/scrollend-polyfill": "^0.0.14", "@ts-stack/markdown": "^1.4.0", "@types/node": "^20.3.0", "a17t": "^0.10.1", @@ -36,6 +37,12 @@ "esbuild": "^0.18.20" } }, + "node_modules/@af-utils/scrollend-polyfill": { + "version": "0.0.14", + "resolved": "https://registry.npmjs.org/@af-utils/scrollend-polyfill/-/scrollend-polyfill-0.0.14.tgz", + "integrity": "sha512-pThXK3XqbWeJHJJAEzhNqCEgOiZ7Flk/Wj/uM6+TGJuA/3n/NeKP3C+5o4jt79i46Cc18iA0kJaMd056GQTfYQ==", + "license": "MIT" + }, "node_modules/@alloc/quick-lru": { "version": "5.2.0", "resolved": "https://registry.npmjs.org/@alloc/quick-lru/-/quick-lru-5.2.0.tgz", @@ -7293,6 +7300,11 @@ } }, "dependencies": { + "@af-utils/scrollend-polyfill": { + "version": "0.0.14", + "resolved": "https://registry.npmjs.org/@af-utils/scrollend-polyfill/-/scrollend-polyfill-0.0.14.tgz", + "integrity": "sha512-pThXK3XqbWeJHJJAEzhNqCEgOiZ7Flk/Wj/uM6+TGJuA/3n/NeKP3C+5o4jt79i46Cc18iA0kJaMd056GQTfYQ==" + }, "@alloc/quick-lru": { "version": "5.2.0", "resolved": "https://registry.npmjs.org/@alloc/quick-lru/-/quick-lru-5.2.0.tgz", diff --git a/package.json b/package.json index 441a052..64a083a 100644 --- a/package.json +++ b/package.json @@ -17,6 +17,7 @@ }, "homepage": "https://github.com/hrfee/jfa-go#readme", "dependencies": { + "@af-utils/scrollend-polyfill": "^0.0.14", "@ts-stack/markdown": "^1.4.0", "@types/node": "^20.3.0", "a17t": "^0.10.1", diff --git a/ts/admin.ts b/ts/admin.ts index a8d054d..925e58d 100644 --- a/ts/admin.ts +++ b/ts/admin.ts @@ -127,7 +127,7 @@ let isInviteURL = window.invites.isInviteURL(); let isAccountURL = accounts.isAccountURL(); // load tabs -const tabs: { id: string, url: string, reloader: () => void }[] = [ +const tabs: { id: string, url: string, reloader: () => void, unloader?: () => void }[] = [ { id: "invites", url: "", @@ -148,16 +148,19 @@ const tabs: { id: string, url: string, reloader: () => void }[] = [ // Don't keep loading the same item on every tab refresh isAccountURL = false; } - window.onscroll = accounts.detectScroll; + accounts.bindPageEvents(); }), + unloader: accounts.unbindPageEvents + }, { id: "activity", url: "activity", reloader: () => { activity.reload() - window.onscroll = activity.detectScroll; + activity.bindPageEvents(); }, + unloader: activity.unbindPageEvents }, { id: "settings", @@ -171,7 +174,7 @@ const defaultTab = tabs[0]; window.tabs = new Tabs(); for (let tab of tabs) { - window.tabs.addTab(tab.id, window.pages.Admin + "/" + tab.url, null, tab.reloader); + window.tabs.addTab(tab.id, window.pages.Admin + "/" + tab.url, null, tab.reloader, tab.unloader || null); } let matchedTab = false diff --git a/ts/modules/list.ts b/ts/modules/list.ts index cf4a507..9f2507e 100644 --- a/ts/modules/list.ts +++ b/ts/modules/list.ts @@ -1,5 +1,6 @@ import { _get, _post, addLoader, removeLoader, throttle } from "./common"; import { Search, SearchConfiguration } from "./search"; +import "@af-utils/scrollend-polyfill"; declare var window: GlobalWindow; @@ -106,10 +107,11 @@ export abstract class PaginatedList { screenHeight: 0, // Render this many screen's worth of content below the viewport. renderNExtraScreensWorth: 3, - rowsOnPage: 0, rendered: 0, initialRenderCount: 0, - scrollLoading: false + scrollLoading: false, + // Used to calculate scroll speed, so more pages are loaded when scrolling fast. + lastScrollY: 0, }; protected _search: Search; @@ -230,7 +232,6 @@ export abstract class PaginatedList { } } - // FIXME: Call on window resize/zoom // FIXME: On reload, load enough pages to fill required space. // FIXME: Might have broken _counter.shown! // Sets the elements with "name"s in "elements" as visible or not. @@ -241,17 +242,20 @@ export abstract class PaginatedList { else this._visible = this._search.ordering.filter(v => !elements.includes(v)); if (this._visible.length == 0) return; - this._scroll.screenHeight = Math.max( - document.documentElement.clientHeight, - window.innerHeight || 0 - ); - if (!appendedItems) { // Wipe old elements and render 1 new one, so we can take the element height. this._container.replaceChildren(this._search.items[this._visible[0]].asElement()) } - this.computeScrollInfo(); + this._computeScrollInfo(); + + // Initial render of min(_visible.length, max(rowsOnPage*renderNExtraScreensWorth, itemsPerPage)), skipping 1 as we already did it. + this._scroll.initialRenderCount = Math.floor(Math.min( + this._visible.length, + Math.max( + ((this._scroll.renderNExtraScreensWorth+1)*this._scroll.screenHeight)/this._scroll.rowHeight, + this._c.itemsPerPage) + )); let baseIndex = 1; if (appendedItems) { @@ -272,21 +276,20 @@ export abstract class PaginatedList { } // Computes required scroll info, requiring one on-DOM item. Should be computed on page resize and this._visible change. - computeScrollInfo = () => { + _computeScrollInfo = () => { + this._scroll.screenHeight = Math.max( + document.documentElement.clientHeight, + window.innerHeight || 0 + ); + this._scroll.rowHeight = this._search.items[this._visible[0]].asElement().offsetHeight; - - // We want to have _scroll.renderNScreensWorth*_scroll.screenHeight or more elements rendered always. - this._scroll.rowsOnPage = Math.floor(this._scroll.screenHeight / this._scroll.rowHeight); - - // Initial render of min(_visible.length, max(rowsOnPage*renderNExtraScreensWorth, itemsPerPage)), skipping 1 as we already did it. - this._scroll.initialRenderCount = Math.min(this._visible.length, Math.max((this._scroll.renderNExtraScreensWorth+1)*this._scroll.rowsOnPage, this._c.itemsPerPage)); } // returns the item index to render up to for the given scroll position. // might return a value greater than this._visible.length, indicating a need for a page load. maximumItemsToRender = (scrollY: number): number => { const bottomScroll = scrollY + ((this._scroll.renderNExtraScreensWorth+1)*this._scroll.screenHeight); - const bottomIdx = Math.floor(bottomScroll / this._scroll.rowsOnPage); + const bottomIdx = Math.floor(bottomScroll / this._scroll.rowHeight); return bottomIdx; } @@ -425,12 +428,24 @@ export abstract class PaginatedList { } } + _detectScroll = () => { if (!this._hasLoaded || this._scroll.scrollLoading) return; if (this._visible.length == 0) return; - const endIdx = this.maximumItemsToRender(window.scrollY); + const scrollY = window.scrollY; + const scrollSpeed = scrollY - this._scroll.lastScrollY; + this._scroll.lastScrollY = scrollY; // If you've scrolled back up, do nothing - if (endIdx <= this._scroll.rendered) return; + if (scrollSpeed < 0) return; + let endIdx = this.maximumItemsToRender(scrollY); + + // Throttling this function means we might not catch up in time if the user scrolls fast, + // so we calculate the scroll speed (in rows/call) from the previous scrollY value. + // This still might not be enough, so hackily we'll just scale it up. + // With onscrollend, this is less necessary, but with both I wasn't able to hit the bottom of the page on my mouse. + const rowsPerScroll = Math.round((scrollSpeed / this._scroll.rowHeight)); + // Render extra pages depending on scroll speed + endIdx += rowsPerScroll*2; const realEndIdx = Math.min(endIdx, this._visible.length); const frag = document.createDocumentFragment(); @@ -439,7 +454,7 @@ export abstract class PaginatedList { } this._scroll.rendered = realEndIdx; this._container.appendChild(frag); - + if (endIdx >= this._visible.length) { if (this.lastPage || this._lastLoad + 500 > Date.now()) return; this._scroll.scrollLoading = true; @@ -448,6 +463,7 @@ export abstract class PaginatedList { this.loadMore(cb, false) return; } + this._scroll.scrollLoading = false; this._detectScroll(); }; @@ -458,6 +474,29 @@ export abstract class PaginatedList { // Should be assigned to window.onscroll whenever the list is in view. detectScroll = throttle(this._detectScroll, 200); + + computeScrollInfo = throttle(this._computeScrollInfo, 200); + + // Should be called in window resize + redrawScroll = throttle(() => { + // FIXME: Make sure this is enough when rows resize, and that we don't need to re-setVisibility. + this._computeScrollInfo(); + // this.setVisibility(this._visible, true, false); + }, 200); + + // bindPageEvents binds window event handlers for when this list/tab containing it is visible. + bindPageEvents = () => { + window.addEventListener("scroll", this.detectScroll); + // Not available on safari, we include a polyfill though. + window.addEventListener("scrollend", this.detectScroll); + window.addEventListener("resize", this.redrawScroll); + }; + + unbindPageEvents = () => { + window.removeEventListener("scroll", this.detectScroll); + window.removeEventListener("scrollend", this.detectScroll); + window.removeEventListener("resize", this.redrawScroll); + } } diff --git a/ts/modules/tabs.ts b/ts/modules/tabs.ts index fc700cf..ade6824 100644 --- a/ts/modules/tabs.ts +++ b/ts/modules/tabs.ts @@ -24,7 +24,7 @@ export class Tabs implements Tabs { }); } - addTab = (tabID: string, url: string, preFunc = () => void {}, postFunc = () => void {},) => { + addTab = (tabID: string, url: string, preFunc = () => void {}, postFunc = () => void {}, unloadFunc = () => void {}) => { let tab: Tab = { page: null, tabEl: document.getElementById("tab-" + tabID) as HTMLDivElement, @@ -50,6 +50,7 @@ export class Tabs implements Tabs { tab.buttonEl.classList.remove("active"); tab.buttonEl.classList.remove("~urge"); tab.tabEl.classList.add("unfocused"); + if (unloadFunc) unloadFunc(); return true; }, shouldSkip: () => false, diff --git a/ts/typings/d.ts b/ts/typings/d.ts index 8447fbd..a1df5c8 100644 --- a/ts/typings/d.ts +++ b/ts/typings/d.ts @@ -91,7 +91,7 @@ declare interface NotificationBox { declare interface Tabs { current: string; - addTab: (tabID: string, url: string, preFunc?: () => void, postFunc?: () => void) => void; + addTab: (tabID: string, url: string, preFunc?: () => void, postFunc?: () => void, unloadFunc?: () => void) => void; switch: (tabID: string, noRun?: boolean, keepURL?: boolean) => void; }