New log UI - #6939
Conversation
common-ui had no test project of its own, so anything covering its components had to live in web-console and import through the package barrel. That barrel pulls in Monaco, and the log-search spec was carrying a comment explaining why its pure functions had to ride along in a browser project belonging to another package. Give common-ui the project instead: a Playwright-backed browser project, and `tests/logSearch.svelte.spec.ts` moved in from web-console to import `$lib` directly. `requireAssertions` is on because the suites that follow assert on geometry, where a spec that measures nothing passes silently. CI runs it ahead of the web-console suite, capped at four workers: vitest's autodetection sees every host core and oversubscribes the pod, which starves browser tests into per-test timeouts.
The log viewer that follows remounts its virtualiser whenever a fresh size cache arrives. 0.48.6 starts observing from a `tick()` microtask with no guard, so a virtualiser destroyed before that microtask resolves still attaches its observers to a detached container; 0.50.4 carries the fix (inokawa/virtua#914). Bumped in every workspace that depends on virtua so the monorepo resolves one copy. The API surface used here is unchanged between the two versions.
`useReverseScrollContainer` needed a fixed number of scroll passes to reach the bottom, which is why the scroll-to-bottom button had to be pressed twice: the target is `scrollHeight`, scrolling there mounts more content, and that moves the target. It also read user intent off scroll geometry, so a virtualiser correcting its own `scrollTop` after measuring a row cancelled the scroll that was doing the correcting. `useStickToBottom` settles by iterating to a fixed point instead, and takes release exclusively from input events, which carry their author by construction. Two collaborators keep the rest out of the anchor itself: releaseGestures what counts as the user taking over (wheel, keys, touch) viewportDormancy when the container's geometry may be believed at all Dormancy is the tab-switch case: a hidden container keeps receiving content against a frozen viewport, and the catch-up scroll event on return used to read as "the user scrolled up" and detach a streaming log for good. Both anchors are exported while the migration runs. `TabAdHocQuery` moves over here because it drives no FAB; the rest share `ScrollDownFab`, whose contract changes with the log viewer.
…iewport The rendered window was `visibleCount - 1` rows, added as a workaround for what was recorded as a flickering issue when scrolling to the bottom of a long list. The flicker was the missing row. `indexOffset` can start half a row above the viewport and the count can round half a row down; `visibleCount` carries two spare rows to pay for both, so dropping one leaves a strip up to a row tall blank along the bottom edge at some offsets — which reads as the list failing to keep up rather than as arithmetic. The spec sweeps two full row heights of scroll offsets, since a window one row short only leaves a gap at some offsets within the rounding period.
The decoder emitted each line with its `\n` or `\r\n` still attached, and every consumer then had to undo it — the log host was passing an empty join separator to the copy builder precisely to avoid doubling them. A line that carries its own newline is not a line. Worse for the log viewer that follows: its height model predicts wrapped row heights over printable ASCII and tabs, and declines anything else. A surviving terminator puts every row outside that grid, so the model declines all of them and the size cache is empty for the whole stream. A stray `\r` is the nastier half — invisible in the rendered row, identical in effect. The `\r` test is bounded by `lineStart`, so an empty line, whose whole record is the terminator, cannot read the previous line's last byte.
`LogList` virtualised by line and could not keep a row mounted, which cost it three separate behaviours: a selection collapsed the moment either endpoint scrolled out, a copy silently truncated to whatever happened to be mounted, and a search highlight had to be re-attempted every frame until the scroll happened to mount its row. Row heights depend on the container width, and a virtualiser only measures what it mounts, so halving the width left the reported scroll range 42.8% short on a 600-line corpus of mixed-length lines — two fifths of the log unreachable for the rest of the session. LogView renders in 50-line chunks and seeds the virtualiser with a full size cache instead of letting it discover heights. Monospace rows make wrapped heights arithmetic, so the offsets are right in the rebuild's first frame; that is what lets a reading position be restored with one assignment and a search jump land without a convergence loop. The parts are separated by concern: logChunks cutting on absolute-line boundaries, so keys survive eviction logRowMetrics the wrap model and the predicted layout logLayout when to re-predict, and restoring the view after the rebuild logLineAlign converging a search jump on the centre of the viewport selectionPin holding the chunks a live selection touches logCopy reconstructing clipboard text from the source lines selectScope replaces 682 lines of virtualSelect with native selection Adopting the anchor everywhere finishes the migration started two commits back: `ScrollDownFab` now reads the raw `stuck` value, so it disappears on the press that re-arms rather than a frame later, and `Query` and `ChangeStream` move with it. `useReverseScrollContainer`, `userSelect` and the already-orphaned `ReverseScrollList` go with their last consumers. Verified against the running console on a 6000-line pipeline log as well as the suites: streaming stays anchored, one press reaches the true end, and a width change never paints a frame away from it.
|
|
||
| // A pointer press begins a fresh selection, which is the one unambiguous signal that the | ||
| // select-all is over. Normal pinning resumes from the next selectionchange. | ||
| const onPointerDown = () => { |
There was a problem hiding this comment.
A pointer press is the only thing that ends a select-all, and isHeld() returns true unconditionally while selectAll is set — which logLayout.reseed() treats as "do not rebuild". So after Ctrl+A a keyboard-only user can resize the window (or toggle the gutter) as much as they like and the size cache is never re-predicted; the 42.8%-stale-heights behaviour this PR fixes comes back until some pointerdown lands anywhere in the document. Esc / a caret-moving key / focus leaving the log would all be reasonable additional exits.
No spec covers "a live selection defers the reseed, and the reseed happens once it clears" in either direction — worth one, since the retry path in logLayout hangs off pinned.length and selectAll changing and a select-all changes neither on its way out except through this handler.
| } | ||
| } | ||
|
|
||
| const onKeyDown = (event: KeyboardEvent) => { |
There was a problem hiding this comment.
stickToBottom.svelte.spec.ts exercises wheel (up/down) and all four touch cases, but nothing covers the keyboard branch — RELEASE_KEYS and this handler are entirely untested. Cheap to add alongside the wheel specs, and it is the one release path that a refactor could silently delete without turning anything red.
Minor: the header says "Every listener is passive bar the keyboard one", but keydown never calls preventDefault() either, and passive has no meaning for keydown — the asymmetry reads as deliberate when it is not.
| it('paints no frame away from the end when a width change rebuilds the view', async () => { | ||
| const { component, scroll } = mountFixture({ | ||
| initialLines: mixedCorpus(4000), | ||
| initialWidth: 800 | ||
| }) | ||
| component.stick() | ||
| await frames(30) |
There was a problem hiding this comment.
This one and stays anchored when the container is made shorter (l.343) are the only tests in the file without an explicit { timeout: 120_000 }, so they run on vitest's 5s default — while mounting 4000/3000 lines and then awaiting 150 / 70 requestAnimationFrame ticks plus a 120ms sleep. At 60fps that is already ~2.5s of pure frame budget; CI runs these four workers at a time in a container, and headless Chromium's rAF cadence drops well below 60fps under that load. Given the zero-tolerance flaky-test rule, please give both the same explicit timeout as their neighbours rather than leaving them one slow frame away from red.
| style:counter-set={showLineNumbers | ||
| ? `line ${chunk.startLine + i + 1}` | ||
| : undefined} | ||
| data-line={chunk.startLine + i} |
There was a problem hiding this comment.
The gutter counts chunk.startLine + i + 1, which is the index into lines, not the absolute line number — it ignores firstLineIndex. So a streaming log whose circular buffer has evicted from the front renumbers its rows from 1 on every eviction, which is the one case where a line number is worth anything.
Latent today (LogsStreamList passes firstLineIndex but not showLineNumbers; BundleLogsView is the reverse), but the two props are public and combine, so this is a trap for the next host. firstLineIndex + chunk.startLine + i + 1 is the fix; either way the prop docs should say which numbering they mean.
|
Reviewed against What I ran
Gates: no Uncovered cases beyond the inline notes: a live selection deferring |
No description provided.