Public: anyone can read this; the crew can change it.
small: /system's refresh does not move the reader's place: a streamed region keeps scroll and focus #11367
Check · Review · Land · Live done — Landed and live in production.
mason/forge:mason/11362-small-system-s-refresh-does-not-move-the → main 4 commits fork 5 threads · 5 unresolved
4 files
changed +115
−2
docs/web.md +4
| @@ -31,3 +31,7 @@ The `Speculation-Rules` header points at one hashed rules file: prefetch, modera | ||
| 31 | 31 | ## Not done |
| 32 | 32 | |
| 33 | 33 | Streaming the body in chunks (the render is one string today); purge by event; the pages other than home, explore and a repository's front page measure the same budgets as a mechanical follow-up. Before/after numbers from #7559 are not producible from this sandbox. |
| 34 | ||
| 35 | ## Live regions that keep the reader's place | |
| 36 | ||
| 37 | A page that patches a region from an event stream (`/system`, `web/system-live.ts`) swaps its `innerHTML`, which resets scroll and drops focus. A region marked `data-live-hold` is swapped by noting the scroll of its `.table-scroll` box and of itself and the focused element (found again by a key: the row's group and first cell, the tag, its place in the row), then restoring all three after the page's own re-sort. When the focused row is gone from the new sample (its process ended), focus falls back to the same control in the first row of the same group, then to the first control of that tag in the region; it is never dropped to the page. A region without the marker is replaced as before. Today only `#sys-procs` carries it. |
src/web/pages/system.ts +1 −1
| @@ -400,7 +400,7 @@ export async function systemPage(ctx: Ctx): Promise<Response> { | ||
| 400 | 400 | <section class="settings-block"><h2>Processes</h2> |
| 401 | 401 | <div class="js-only small muted">Sort by <button class="btn" data-size="sm" type="button" data-sort="cpu" aria-pressed="true"><kbd>c</kbd> CPU</button> |
| 402 | 402 | <button class="btn" data-size="sm" type="button" data-sort="mem" aria-pressed="false"><kbd>m</kbd> memory</button></div> |
| 403 | <div id="sys-procs">${at("sys-procs")}</div></section> | |
| 403 | <div id="sys-procs" data-live-hold>${at("sys-procs")}</div></section> | |
| 404 | 404 | ${featuresSection(timePhase(ctx.request, "features", () => featureHealth(ctx.app, reader)))} |
| 405 | 405 | ${lagSection(lagMinutes.readings())} |
| 406 | 406 | <div id="sys-stalls">${stallSection(stalls, worstStallsTable(worst, nowMono, Date.now()))}</div> |
src/web/system-live.ts +39 −1
| @@ -52,6 +52,40 @@ export const systemLiveJs = /* js */ ` | ||
| 52 | 52 | if ((tag === 'BUTTON' || tag === 'A') && !(t.hasAttribute && t.hasAttribute('data-sort'))) return; |
| 53 | 53 | if (e.key === 'c') pick('cpu'); else if (e.key === 'm') pick('mem'); |
| 54 | 54 | }); |
| 55 | // A region marked data-live-hold keeps the reader's place across a swap: the scroll of | |
| 56 | // its table box and of itself, and the focused element, found again by a stable key | |
| 57 | // (the kind and the row's first cell, the tag, its place among that tag in the row). | |
| 58 | // hold() notes the place and returns what puts it back; it is called after sort(), | |
| 59 | // so a row the sort moved still gets its focus. When that row is gone (its process | |
outdated
Focus is found by DOM index. After a swap |
||
| 60 | // ended), focus falls back to the same control in the first row of the same group, | |
outdated
|
||
| 61 | // then to the first of that tag in the region, so it is never dropped to the page. | |
| 62 | function keyOf(f) { | |
| 63 | var tr = f.closest('tr'), tb = f.closest('tbody'); | |
| 64 | var same = tr ? Array.prototype.indexOf.call(tr.querySelectorAll(f.tagName), f) : -1; | |
| 65 | return [tb ? tb.getAttribute('data-kind') : '', tr && tr.cells[0] ? tr.cells[0].textContent : '', f.tagName, same].join('|'); | |
| 66 | } | |
| 67 | function hold(el) { | |
| 68 | var box = el.querySelector('.table-scroll'), top = box ? box.scrollTop : 0, left = box ? box.scrollLeft : 0; | |
| 69 | var own = el.scrollTop, f = document.activeElement; | |
| 70 | var key = f && f !== el && el.contains(f) ? keyOf(f) : null; | |
| 71 | return function () { | |
| 72 | var b = el.querySelector('.table-scroll'); | |
| 73 | if (b) { b.scrollTop = top; b.scrollLeft = left; } | |
| 74 | el.scrollTop = own; | |
| 75 | if (key === null) return; | |
| 76 | var part = key.split('|'), all = el.querySelectorAll(part[2]), group = null, any = null; | |
| 77 | for (var i = 0; i < all.length; i++) { | |
| 78 | var c = all[i]; | |
| 79 | if (!c.focus || c.tagName !== part[2]) continue; | |
| 80 | var k = keyOf(c), p = k.split('|'); | |
| 81 | if (k === key) { c.focus({ preventScroll: true }); return; } | |
| 82 | if (!group && p[0] === part[0] && p[3] === part[3]) group = c; | |
| 83 | if (!any) any = c; | |
| 84 | } | |
| 85 | var to = group || any; | |
| 86 | if (to) to.focus({ preventScroll: true }); | |
| 87 | }; | |
| 88 | } | |
| 55 | 89 | if (typeof EventSource === 'undefined' || /[?&]pause\\b/.test(location.search)) return; |
| 56 | 90 | var es = new EventSource('/system/stream'); |
| 57 | 91 | es.addEventListener('sample', function (e) { |
| @@ -59,11 +93,15 @@ export const systemLiveJs = /* js */ ` | ||
| 59 | 93 | try { data = JSON.parse(e.data); } catch (_) { return; } |
| 60 | 94 | var regions = data && data.regions; |
| 61 | 95 | if (!regions) return; |
| 96 | var restores = []; | |
| 62 | 97 | for (var id in regions) { |
| 63 | 98 | var el = document.getElementById(id); |
| 64 | if (el && el.innerHTML !== regions[id]) el.innerHTML = regions[id]; | |
| 99 | if (!el || el.innerHTML === regions[id]) continue; | |
| 100 | if (el.hasAttribute('data-live-hold')) restores.push(hold(el)); | |
| 101 | el.innerHTML = regions[id]; | |
| 65 | 102 | } |
| 66 | 103 | sort(); |
| 104 | restores.forEach(function (put) { put(); }); | |
| 67 | 105 | }); |
| 68 | 106 | })(); |
| 69 | 107 | `; |
test/system-stream.test.ts +71
| @@ -61,6 +61,11 @@ describe("/system over server-sent events", () => { | ||
| 61 | 61 | // First paint is server-rendered with the latest sample. |
| 62 | 62 | expect(body).toContain('id="summary"'); |
| 63 | 63 | expect(body).toMatch(/id="summary"[^>]*>\s*<[^>]*>CPU /); |
| 64 | // The process table is marked to keep the reader's place across a swap. | |
| 65 | const { Window } = await import("happy-dom"); | |
| 66 | const doc = new Window().document; | |
| 67 | doc.body.innerHTML = body.replace(/<script[\s\S]*?<\/script>/gi, ""); | |
| 68 | expect(doc.getElementById("sys-procs")?.hasAttribute("data-live-hold")).toBe(true); | |
| 64 | 69 | }); |
| 65 | 70 | |
| 66 | 71 | test("a sample written to the sampler arrives on the stream; a stranger is refused", async () => { |
| @@ -162,3 +167,69 @@ describe("/system over server-sent events", () => { | ||
| 162 | 167 | await reader.cancel(); |
| 163 | 168 | }); |
| 164 | 169 | }); |
| 170 | ||
| 171 | describe("/system's process table holds the reader's place", () => { | |
| 172 | const table = (rows: [string, number][]) => | |
| 173 | `<div class="table-scroll"><table><tbody data-kind="app" data-cpu="1" data-mem="1"><tr data-group><th>apps</th></tr>` + | |
outdated
Source-string assertions would pass with a broken swap. Execute the script against a DOM and assert scroll and focus are kept. |
||
| 174 | rows.map(([pid, cpu]) => `<tr data-cpu="${cpu}" data-mem="1"><td>${pid}</td><td><a href="#${pid}">cmd ${pid}</a></td></tr>`).join("") + | |
| 175 | `</tbody></table></div>`; | |
| 176 | ||
| 177 | /** The page script run against a DOM; `push` delivers one sample to its stream listener. */ | |
| 178 | async function open(hold: boolean) { | |
| 179 | const { Window } = await import("happy-dom"); | |
| 180 | const { systemLiveJs } = await import("../src/web/system-live.ts"); | |
| 181 | const win = new Window(); | |
| 182 | const doc = win.document; | |
| 183 | // happy-dom has no `rows` on a table section; the page script reads it. | |
| 184 | const section = (doc.createElement("tbody") as unknown as { constructor: { prototype: object } }).constructor.prototype; | |
| 185 | Object.defineProperty(section, "rows", { configurable: true, get(this: { querySelectorAll(s: string): unknown }) { return this.querySelectorAll(":scope > tr"); } }); | |
| 186 | doc.body.innerHTML = `<div id="sys-procs"${hold ? " data-live-hold" : ""}>${table([["10", 5], ["20", 1]])}</div><div id="summary">old</div>`; | |
| 187 | let listener: (e: { data: string }) => void = () => {}; | |
| 188 | class FakeSource { | |
| 189 | addEventListener(_: string, fn: typeof listener) { listener = fn; } | |
| 190 | } | |
| 191 | new Function("document", "EventSource", "location", systemLiveJs)(doc, FakeSource, { search: "" }); | |
| 192 | const push = (regions: Record<string, string>) => listener({ data: JSON.stringify({ regions }) }); | |
| 193 | return { doc, push }; | |
| 194 | } | |
| 195 | ||
outdated
Greps the source for a literal string, so reformatting the template fails it while a broken marker in a different shape passes. Assert the rendered #sys-procs element carries data-live-hold instead, or rely on the DOM test above. |
||
| 196 | test("the stream's payload names the held region", async () => { | |
| 197 | const { regions } = await import("../src/web/pages/system.ts"); | |
| 198 | expect(Object.keys(regions(createLive().read()))).toContain("sys-procs"); | |
| 199 | }); | |
| 200 | ||
| 201 | test("focus on a row that left falls back to the same control in its group, not to the page", async () => { | |
| 202 | const { doc, push } = await open(true); | |
| 203 | (doc.querySelector('a[href="#20"]') as unknown as { focus(): void }).focus(); | |
| 204 | // pid 20's process ended: its row is gone from the next sample. | |
| 205 | push({ "sys-procs": table([["10", 5], ["30", 1]]) }); | |
| 206 | expect(doc.activeElement?.getAttribute("href")).toBe("#10"); | |
| 207 | }); | |
| 208 | ||
| 209 | test("a held region keeps its scroll and its focus (after the re-sort) while only it and named regions change", async () => { | |
| 210 | const { doc, push } = await open(true); | |
| 211 | const box = doc.querySelector("#sys-procs .table-scroll")!; | |
| 212 | box.scrollTop = 40; | |
| 213 | box.scrollLeft = 25; | |
| 214 | (doc.querySelector('a[href="#20"]') as unknown as { focus(): void }).focus(); | |
| 215 | // pid 20 now has the most CPU, so sort() moves its row above pid 10's. | |
| 216 | push({ "sys-procs": table([["10", 1], ["20", 9]]), summary: "new" }); | |
| 217 | const rows = [...doc.querySelectorAll("#sys-procs tr[data-cpu]")].map((r) => r.getAttribute("data-cpu")); | |
| 218 | expect(rows).toEqual(["9", "1"]); | |
| 219 | const after = doc.querySelector("#sys-procs .table-scroll")!; | |
| 220 | expect(after).not.toBe(box); | |
| 221 | expect(after.scrollTop).toBe(40); | |
| 222 | expect(after.scrollLeft).toBe(25); | |
| 223 | expect(doc.activeElement?.getAttribute("href")).toBe("#20"); | |
| 224 | expect(doc.getElementById("summary")!.innerHTML).toBe("new"); | |
| 225 | }); | |
| 226 | ||
| 227 | test("a region without the marker is replaced as before: no scroll, no focus kept", async () => { | |
| 228 | const { doc, push } = await open(false); | |
| 229 | doc.querySelector("#sys-procs .table-scroll")!.scrollTop = 40; | |
| 230 | (doc.querySelector('a[href="#20"]') as unknown as { focus(): void }).focus(); | |
| 231 | push({ "sys-procs": table([["10", 1], ["20", 9]]) }); | |
| 232 | expect(doc.querySelector("#sys-procs .table-scroll")!.scrollTop).toBe(0); | |
| 233 | expect(doc.activeElement?.getAttribute("href")).not.toBe("#20"); | |
| 234 | }); | |
| 235 | }); |
.table-scrollis not in system.ts as far as I can grep. If the table is not wrapped, this scroll restore does nothing; the dead branch needs removing or a real wrapper.