Public: anyone can read this; the crew can change it.
Lists drop rows removed on the server without a reload #11730
Check · Review · Land · Live done — Landed and live in production.
torque/forge:torque/11663-lists-drop-rows-removed-on-the-server-wi → main 4 commits fork 1 thread · 1 unresolved
2 files
changed +172
−14
src/web/live.ts +54 −14
| @@ -102,7 +102,8 @@ export const liveClientJs = /* js */ ` | ||
| 102 | 102 | // (a burst of events is one fetch, never sooner than BATCH after the first): |
| 103 | 103 | // the page's own HTML is fetched, and every row it has that the list lacks is |
| 104 | 104 | // inserted where the fresh order puts it; a row that changed is replaced in |
| 105 | // place unless the reader is in it. A reader at the top sees rows slide in; | |
| 105 | // place unless the reader is in it, and a row the page no longer lists is | |
| 106 | // removed unless the reader is in it. A reader at the top sees rows slide in; | |
| 106 | 107 | // anyone scrolled, typing or reading has the rows added above with the |
| 107 | 108 | // scroll compensated by exactly their height, and a quiet "N above" marker |
| 108 | 109 | // to jump up when they choose. Nothing asks for a refresh. |
| @@ -151,16 +152,40 @@ export const liveClientJs = /* js */ ` | ||
| 151 | 152 | var a = document.activeElement; |
| 152 | 153 | return !!a && (a.tagName === 'INPUT' || a.tagName === 'TEXTAREA' || a.isContentEditable === true); |
| 153 | 154 | } |
| 154 | function mergeList(mine, theirs, calm) { | |
| 155 | // Whether a fetched list is only part of what the server has: the page | |
| 156 | // offers a pager link (ui.ts pager(), or a page's own nav.pager) or the | |
| 157 | // list ends in an "and N more" row ([data-more]). | |
| 158 | function offersMore(page, list) { | |
| 159 | return !!(page.querySelector('nav.pager a[href]') || list.querySelector('[data-more]')); | |
| 160 | } | |
| 161 | // Merges one fetched list into the one on screen. Returns how many rows | |
| 162 | // arrived (counted for the marker: only those above the reader when not | |
| 163 | // calm) and how many were removed. | |
| 164 | function mergeList(mine, theirs, calm, partial) { | |
| 155 | 165 | var olds = kids(mine); |
| 156 | 166 | var have = {}; |
| 157 | 167 | olds.forEach(function (li) { var k = keyOf(li); if (k) have[k] = li; }); |
| 158 | // The row the reader is looking at: it must stay where it is. | |
| 168 | // A row the server no longer lists (deleted, filtered out) goes, unless | |
| 169 | // the reader is in it. A complete page is the whole list, so every row | |
| 170 | // it lacks goes — the last, the only one, all of them. A page that offers | |
| 171 | // more is truncated: a row pushed off it, or loaded past it, is not gone, | |
| 172 | // so only rows before the last row it still lists are candidates. | |
| 173 | var listed = {}; | |
| 174 | kids(theirs).forEach(function (n) { var k = keyOf(n); if (k) listed[k] = true; }); | |
| 175 | var lastAt = partial ? -1 : olds.length; | |
| 176 | if (partial) olds.forEach(function (li, at) { var k = keyOf(li); if (k && listed[k]) lastAt = at; }); | |
| 177 | var doomed = olds.filter(function (li, at) { | |
| 178 | var k = keyOf(li); | |
| 179 | return at < lastAt && k && !listed[k] && !held(li); | |
| 180 | }); | |
| 181 | // The row the reader is looking at: it must stay where it is. Never one | |
| 182 | // that is about to be removed. | |
| 183 | var kept = olds.filter(function (li) { return doomed.indexOf(li) < 0; }); | |
| 159 | 184 | var ref = null; |
| 160 | for (var i = 0; i < olds.length; i++) { | |
| 161 | if (olds[i].getBoundingClientRect().bottom > 0) { ref = olds[i]; break; } | |
| 185 | for (var i = 0; i < kept.length; i++) { | |
| 186 | if (kept[i].getBoundingClientRect().bottom > 0) { ref = kept[i]; break; } | |
| 162 | 187 | } |
| 163 | if (!ref) ref = olds[0] || null; | |
| 188 | if (!ref) ref = kept[0] || null; | |
| 164 | 189 | var refTop = ref ? ref.getBoundingClientRect().top : 0; |
| 165 | 190 | var prev = null, added = []; |
| 166 | 191 | kids(theirs).forEach(function (n) { |
| @@ -184,7 +209,17 @@ export const liveClientJs = /* js */ ` | ||
| 184 | 209 | prev = fresh; |
| 185 | 210 | added.push(fresh); |
| 186 | 211 | }); |
| 187 | if (added.length === 0) return 0; | |
| 212 | if (doomed.length > 0) { | |
| 213 | doomed.forEach(function (li) { delete have[keyOf(li)]; li.remove(); }); | |
| 214 | } | |
| 215 | if (added.length > 0 || doomed.length > 0) { | |
| 216 | if (!calm && ref) { | |
| 217 | var shift = ref.getBoundingClientRect().top - refTop; | |
| 218 | if (shift !== 0) window.scrollBy(0, shift); | |
| 219 | } | |
| 220 | } | |
| 221 | var out = { added: 0, removed: doomed.length }; | |
| 222 | if (added.length === 0) return out; | |
| 188 | 223 | if (calm) { |
| 189 | 224 | added.forEach(function (n) { |
| 190 | 225 | if (still) return; |
| @@ -193,27 +228,32 @@ export const liveClientJs = /* js */ ` | ||
| 193 | 228 | n.addEventListener('animationend', done); |
| 194 | 229 | setTimeout(done, 400); |
| 195 | 230 | }); |
| 196 | return added.length; | |
| 231 | out.added = added.length; | |
| 232 | return out; | |
| 197 | 233 | } |
| 198 | 234 | var gone = ref ? 0 : added.length; |
| 199 | 235 | if (ref) { |
| 200 | var delta = ref.getBoundingClientRect().top - refTop; | |
| 201 | if (delta !== 0) window.scrollBy(0, delta); | |
| 202 | 236 | added.forEach(function (n) { if (n.compareDocumentPosition(ref) & 4) gone++; }); |
| 203 | 237 | } |
| 204 | return gone; | |
| 238 | out.added = gone; | |
| 239 | return out; | |
| 205 | 240 | } |
| 206 | 241 | function mergePage(fresh) { |
| 207 | 242 | var mine = document.querySelectorAll('ul.list'); |
| 208 | 243 | var theirs = fresh.querySelectorAll('ul.list'); |
| 209 | 244 | if (mine.length === 0 || mine.length !== theirs.length) return; |
| 210 | 245 | var calm = (window.pageYOffset || 0) < 8 && !typing(); |
| 211 | var total = 0; | |
| 246 | var total = 0, removed = 0; | |
| 212 | 247 | for (var i = 0; i < mine.length; i++) { |
| 213 | 248 | if (mine[i].closest('#app-previews, #app-head')) continue; |
| 214 | total += mergeList(mine[i], theirs[i], calm); | |
| 249 | var got = mergeList(mine[i], theirs[i], calm, offersMore(fresh, theirs[i])); | |
| 250 | total += got.added; | |
| 251 | removed += got.removed; | |
| 252 | } | |
| 253 | if (total === 0) { | |
| 254 | if (removed > 0) say(removed === 1 ? '1 item removed' : removed + ' items removed'); | |
| 255 | return; | |
| 215 | 256 | } |
| 216 | if (total === 0) return; | |
| 217 | 257 | if (!calm) setAbove(above + total); |
| 218 | 258 | say(total === 1 ? '1 new item' : total + ' new items'); |
| 219 | 259 | } |
test/live-arrive.test.ts +118
| @@ -6,11 +6,15 @@ | ||
| 6 | 6 | */ |
| 7 | 7 | import { expect, test } from "bun:test"; |
| 8 | 8 | import { liveClientJs, liveMount } from "../src/web/live.ts"; |
| 9 | import { pager } from "../src/web/ui.ts"; | |
| 9 | 10 | |
| 10 | 11 | const li = (n: number, extra = "") => |
| 11 | 12 | `<li><a href="/a/b/issues/${n}" class="row"><span class="row-title">Issue ${n}${extra}</span></a></li>`; |
| 12 | 13 | const list = (ns: number[], extra = "") => `<ul class="list">${ns.map((n) => li(n, extra)).join("")}</ul>`; |
| 13 | 14 | |
| 15 | /** The pager the list pages draw when there is more to see. */ | |
| 16 | const more = String(pager({ next: "/a/b/issues?before=1" })); | |
| 17 | ||
| 14 | 18 | async function boot(initial: number[]) { |
| 15 | 19 | const { Window } = await import("happy-dom"); |
| 16 | 20 | const win = new Window({ url: "http://forge.test/a/b/issues" }); |
| @@ -175,3 +179,117 @@ test("arrived rows lose their class on a timer, so reduced motion never strands | ||
| 175 | 179 | const css = await Bun.file(new URL("../src/web/style/components.ts", import.meta.url)).text(); |
| 176 | 180 | expect(css).toMatch(/prefers-reduced-motion: reduce\) \{[^}]*animation-duration: \.01ms !important/); |
| 177 | 181 | }); |
| 182 | ||
| 183 | test("a row the server no longer lists is removed on the next catch-up", async () => { | |
| 184 | const t = await boot([3, 2, 1]); | |
| 185 | t.setPage(list([3, 1])); | |
| 186 | t.send("issue.closed"); | |
| 187 | await t.wait(700); | |
| 188 | expect(t.titles()).toEqual(["Issue 3", "Issue 1"]); | |
| 189 | }); | |
| 190 | ||
| 191 | test("a removed row above the reader's place compensates the scroll by exactly its height", async () => { | |
| 192 | const t = await boot([3, 2, 1]); | |
| 193 | t.state.scrollY = 150; // Issue 3 (0-100) is above the viewport, Issue 2 is the reader's row | |
| 194 | t.setPage(list([2, 1])); | |
| 195 | t.send("issue.closed"); | |
| 196 | await t.wait(700); | |
| 197 | expect(t.titles()).toEqual(["Issue 2", "Issue 1"]); | |
| 198 | expect(t.state.scrolled).toEqual([-100]); | |
| 199 | }); | |
| 200 | ||
| 201 | test("a row the reader is in is never removed", async () => { | |
| 202 | const t = await boot([2, 1]); | |
| 203 | (t.doc.querySelectorAll("ul.list a")[1] as unknown as { focus(): void }).focus(); | |
| 204 | t.setPage(list([2])); | |
| 205 | t.send("issue.closed"); | |
| 206 | await t.wait(700); | |
| 207 | expect(t.titles()).toEqual(["Issue 2", "Issue 1"]); | |
| 208 | }); | |
| 209 | ||
| 210 | test("a complete page with no rows removes every unheld row; nothing is scrolled and nothing throws", async () => { | |
| 211 | const t = await boot([2, 1]); | |
| 212 | t.state.scrollY = 500; | |
| 213 | t.setPage(`<ul class="list"></ul>`); | |
| 214 | t.send("issue.closed"); | |
| 215 | await t.wait(700); | |
| 216 | expect(t.titles()).toEqual([]); | |
| 217 | expect(t.state.scrolled).toEqual([]); | |
| 218 | expect(t.doc.getElementById("live-status")!.textContent).toBe("2 items removed"); | |
| 219 | }); | |
| 220 | ||
| 221 | test("the last row of a short list goes when the page offers no more", async () => { | |
| 222 | const t = await boot([3, 2, 1]); | |
| 223 | t.setPage(list([3, 2])); | |
| 224 | t.send("issue.closed"); | |
| 225 | await t.wait(700); | |
| 226 | expect(t.titles()).toEqual(["Issue 3", "Issue 2"]); | |
| 227 | }); | |
| 228 | ||
| 229 | test("the only row of a filtered list goes when it is closed out of the filter", async () => { | |
| 230 | const t = await boot([1]); | |
| 231 | t.setPage(`<ul class="list"></ul>`); | |
| 232 | t.send("issue.closed"); | |
| 233 | await t.wait(700); | |
| 234 | expect(t.titles()).toEqual([]); | |
| 235 | }); | |
| 236 | ||
| 237 | test("a truncated page that lists none of ours removes nothing", async () => { | |
| 238 | const t = await boot([2, 1]); | |
| 239 | t.setPage(`<ul class="list"></ul>${more}`); | |
| 240 | t.send("issue.closed"); | |
| 241 | await t.wait(700); | |
| 242 | expect(t.titles()).toEqual(["Issue 2", "Issue 1"]); | |
| 243 | }); | |
| 244 | ||
| 245 | test("when no kept row is on screen the scroll is compensated against a kept row, never a removed one", async () => { | |
| 246 | const t = await boot([3, 2, 1]); | |
| 247 | // Every row is above the viewport (tops -1000, -900, -800). | |
| 248 | t.win.HTMLElement.prototype.getBoundingClientRect = function (this: { tagName: string; parentElement: { children: ArrayLike<unknown> } | null }) { | |
| 249 | const at = this.parentElement ? Array.from(this.parentElement.children).indexOf(this) : 0; | |
| 250 | const top = this.parentElement ? at * 100 - 1000 : 0; | |
| 251 | return { top, bottom: top + 100, left: 0, right: 0, width: 0, height: 100, x: 0, y: top } as never; | |
| 252 | } as never; | |
| 253 | t.state.scrollY = 500; | |
| 254 | t.setPage(list([2, 1])); | |
| 255 | t.send("issue.closed"); | |
| 256 | await t.wait(700); | |
| 257 | expect(t.titles()).toEqual(["Issue 2", "Issue 1"]); | |
| 258 | expect(t.state.scrolled).toEqual([-100]); | |
| 259 | }); | |
| 260 | ||
| 261 | test("inserts and removals in one catch-up compensate the scroll by the net shift of the reader's row", async () => { | |
| 262 | const t = await boot([3, 2, 1]); | |
| 263 | t.state.scrollY = 500; | |
| 264 | t.setPage(list([5, 4, 2, 1])); // 3 gone above, 4 and 5 arrive above | |
| 265 | t.send("issue.closed"); | |
| 266 | await t.wait(700); | |
| 267 | expect(t.titles()).toEqual(["Issue 5", "Issue 4", "Issue 2", "Issue 1"]); | |
| 268 | expect(t.state.scrolled).toEqual([100]); | |
| 269 | expect(t.doc.getElementById("live-above")!.textContent).toBe("2 above"); | |
| 270 | }); | |
| 271 | ||
| 272 | test("a row pushed off the fetched page, or loaded past it, is not removed", async () => { | |
| 273 | const t = await boot([3, 2, 1]); | |
| 274 | t.setPage(list([4, 3]) + more); // page of two with an Older link: 2 and 1 are past its end, not deleted | |
| 275 | t.send("issue.opened"); | |
| 276 | await t.wait(700); | |
| 277 | expect(t.titles()).toEqual(["Issue 4", "Issue 3", "Issue 2", "Issue 1"]); | |
| 278 | }); | |
| 279 | ||
| 280 | test("a truncated page still removes a row it lacks before its last listed row", async () => { | |
| 281 | const t = await boot([3, 2, 1]); | |
| 282 | t.setPage(list([3, 1]) + more); | |
| 283 | t.send("issue.closed"); | |
| 284 | await t.wait(700); | |
| 285 | expect(t.titles()).toEqual(["Issue 3", "Issue 1"]); | |
| 286 | }); | |
| 287 | ||
| 288 | test("an \"and N more\" row marks the list truncated, so its tail is kept", async () => { | |
| 289 | const t = await boot([3, 2, 1]); | |
| 290 | t.setPage(`<ul class="list">${li(3)}<li><a href="/a/b/issues?all" data-more>and 2 more</a></li></ul>`); | |
| 291 | t.send("issue.opened"); | |
| 292 | await t.wait(700); | |
| 293 | expect(t.titles()).toContain("Issue 2"); | |
| 294 | expect(t.titles()).toContain("Issue 1"); | |
| 295 | }); |
new above
offline
A position-based guard cannot see the last row (or the only row) go. Use whether the fetched page is complete (no load-more or next control) to decide, and keep
lastAtonly for a page that offers more.