Make the log list the scroll container #43

New issue

Open

opened 2026-11-12 17:37:51 +01:00 by omp-deck · 0 comments

omp-deck commented

2026-11-12 17:37:51 +01:00

Collaborator

Problem

The log view currently scrolls the window. assets/scss/main.scss:18-25 states this as a
deliberate decision — "Both bars stay visible while the log scrolls; the window remains the
scroller" — and implements it by pinning .navbar and .view-bar with position: sticky
(main.scss:21-25, :48-51). The list itself sets overflow-x: auto (main.scss:81-85) so that
nowrap mode can scroll sideways.

That arrangement blocks the next step, and it carries a latent rendering bug.

It cannot host a sticky column header

Planned work (#39) adds a column header row, which is the prerequisite for the resize handles in
#41. Two things are required of it: it must sit inside the horizontal scroller, or it desyncs
from the columns as soon as nowrap mode scrolls sideways; and it must stay visible while the log
scrolls.

Both cannot hold today. Per the CSS overflow rules, when one axis is not visible the other
computes from visible to auto — so overflow-x: auto makes .log-list a scroll container on
both axes. A position: sticky header inside it therefore sticks relative to the list's own
scrollport rather than the viewport, and because the list's height equals its content height there
is no vertical scrolling inside it for the header to stick against. The header simply rides along
and scrolls off the top of the page.

The three requirements — header inside the scroller, header sticky, window as scroller — are
mutually unsatisfiable. Giving up window-as-scroller is the one that costs least: the sticky
positioning on the navbar and view-bar exists only to compensate for the window scrolling, so
removing it is a net simplification rather than a trade.

Row backgrounds do not span the scroll width

In nowrap mode each row is sized width: max-content; min-width: 100% (main.scss:176-179), so
rows are only as wide as their own content. Once the list is scrolled right, a row shorter than the
scroll width has no background under the scrolled-to region — the hover highlight stops at ragged
horizontal positions.

This is invisible enough to miss today, because hover affects one row at a time. It stops being
subtle the moment #42 adds alternating row backgrounds, where every row is painted and the ragged
edges become a visible staircase. Fixing it here keeps #42 from inheriting it.

Why now

This change alters existing behaviour and reverses a documented decision, but adds no feature
surface. Separating it from #39 keeps that issue to the layout and model work, and means the
scroller behaviour can be judged on its own.

Plan

1. Make `.log-list` the scroll container

In assets/scss/main.scss:

Give .log-list a bounded height so it scrolls internally: the viewport height minus the chrome
above it. Use dvh rather than vh — mobile browser chrome resizes, and a wrong value here
produces either a double scrollbar or a clipped final row.
Keep overflow-x: auto; the computed overflow-y: auto that previously had nothing to do now
becomes the vertical scroller, which is the point of the change.

2. Unpin the chrome

Still in main.scss:

Remove position: sticky / top / border-bottom positioning concerns from .navbar
(:21-25) and .view-bar (:48-51), keeping the hairline rules that bound the chrome visually.
With a bounded-height list below them, both are simply in normal flow and stay put.
Re-examine the zero-padding rules at :36-45. Their stated justification is that no sticky layer
may stop short of the viewport edge, because Firefox flashes any strip an opaque background does
not cover while it scrolls. With nothing sticky, that reason is gone; the edge-to-edge density
argument in the same comment still stands, so the rules likely remain but the comments must stop
citing a hazard that no longer applies.
Remove the z-index on .view-bar if nothing else needs it.

Update the comment block at :18-20: it currently documents "the window remains the scroller" as
the governing decision. It must state the new arrangement and why — the sticky header a column
header row needs cannot work inside a horizontally scrolling list while the window owns vertical
scrolling.

3. Fix nowrap row width

Move the max-content sizing from the row up to the list (main.scss:171-184), so the list is as
wide as its widest row and rows fill it. Every row then paints across the full scroll width and
backgrounds no longer stop at ragged positions.

Re-examine the padding-bottom: 0.75rem at :172-174. It compensates for Firefox's overlay
horizontal scrollbar covering the newest row when the window scrolled the list; with the list
scrolling itself, verify whether it is still needed rather than carrying it over unexamined.

4. Scroll to the newest row against the new scroller

In assets/js/view-all.js:4-8, window.scrollTo(0, document.body.scrollHeight) no longer targets
the right element. Replace it with scrollIntoView on the last row rather than assigning
list.scrollTop = list.scrollHeight: the two are equivalent today, but scrollIntoView forces the
target to render and so stays correct if content-visibility is adopted later and makes
scrollHeight an estimate.

The .is-ready opacity gate (main.scss:87-103) stays as is — it still hides the initial jump, and
still needs its visibility: hidden companion so a pre-scroll list is neither hit-testable nor
findable by find-in-page.

5. Verify

just test — tests/views.rs asserts rendered HTML only (.log-list.is-nowrap, cell text per
column) and is unaffected by scroller geometry. No test changes are expected; if any assertion
moves, that is a signal something rendered differently and should be understood rather than
re-pinned.
just check — clippy and fmt.
just lint — stylelint over the changed SCSS.
Manual, since no browser runs in CI and none of this is observable to the existing harness:
wrap mode scrolls vertically with the chrome staying put; nowrap scrolls horizontally with the
hover highlight spanning the full width at any horizontal offset; the list opens scrolled to the
newest row with no visible jump; no double scrollbar and no clipped last row.

Out of scope

The column header itself, the grid layout and the cell model (#39); column state in the query (#40);
resize and fit (#41); zebra and severity colouring (#42). content-visibility, a tail DOM cap, and
the narrow-viewport reflow are deferred entirely.

Noted, not fixed here

docs/STATUS.md is stale: it states that src/views/, templates/views/ and
templates/partials/log_row.html do not exist and that there is no VictoriaLogs integration, while
all of them are present. Pre-existing drift, unrelated to this change, and worth correcting
separately rather than silently inside it.

## Problem The log view currently scrolls the **window**. `assets/scss/main.scss:18-25` states this as a deliberate decision — "Both bars stay visible while the log scrolls; the window remains the scroller" — and implements it by pinning `.navbar` and `.view-bar` with `position: sticky` (`main.scss:21-25`, `:48-51`). The list itself sets `overflow-x: auto` (`main.scss:81-85`) so that nowrap mode can scroll sideways. That arrangement blocks the next step, and it carries a latent rendering bug. ### It cannot host a sticky column header Planned work (#39) adds a column header row, which is the prerequisite for the resize handles in #41. Two things are required of it: it must sit **inside** the horizontal scroller, or it desyncs from the columns as soon as nowrap mode scrolls sideways; and it must stay visible while the log scrolls. Both cannot hold today. Per the CSS overflow rules, when one axis is not `visible` the other computes from `visible` to `auto` — so `overflow-x: auto` makes `.log-list` a scroll container on **both** axes. A `position: sticky` header inside it therefore sticks relative to the list's own scrollport rather than the viewport, and because the list's height equals its content height there is no vertical scrolling inside it for the header to stick against. The header simply rides along and scrolls off the top of the page. The three requirements — header inside the scroller, header sticky, window as scroller — are mutually unsatisfiable. Giving up window-as-scroller is the one that costs least: the sticky positioning on the navbar and view-bar exists only to compensate for the window scrolling, so removing it is a net simplification rather than a trade. ### Row backgrounds do not span the scroll width In nowrap mode each row is sized `width: max-content; min-width: 100%` (`main.scss:176-179`), so rows are only as wide as their own content. Once the list is scrolled right, a row shorter than the scroll width has no background under the scrolled-to region — the hover highlight stops at ragged horizontal positions. This is invisible enough to miss today, because hover affects one row at a time. It stops being subtle the moment #42 adds alternating row backgrounds, where every row is painted and the ragged edges become a visible staircase. Fixing it here keeps #42 from inheriting it. ### Why now This change alters existing behaviour and reverses a documented decision, but adds no feature surface. Separating it from #39 keeps that issue to the layout and model work, and means the scroller behaviour can be judged on its own. ## Plan ### 1. Make `.log-list` the scroll container In `assets/scss/main.scss`: - Give `.log-list` a bounded height so it scrolls internally: the viewport height minus the chrome above it. Use `dvh` rather than `vh` — mobile browser chrome resizes, and a wrong value here produces either a double scrollbar or a clipped final row. - Keep `overflow-x: auto`; the computed `overflow-y: auto` that previously had nothing to do now becomes the vertical scroller, which is the point of the change. ### 2. Unpin the chrome Still in `main.scss`: - Remove `position: sticky` / `top` / `border-bottom` positioning concerns from `.navbar` (`:21-25`) and `.view-bar` (`:48-51`), keeping the hairline rules that bound the chrome visually. With a bounded-height list below them, both are simply in normal flow and stay put. - Re-examine the zero-padding rules at `:36-45`. Their stated justification is that no sticky layer may stop short of the viewport edge, because Firefox flashes any strip an opaque background does not cover while it scrolls. With nothing sticky, that reason is gone; the edge-to-edge density argument in the same comment still stands, so the rules likely remain but the comments must stop citing a hazard that no longer applies. - Remove the `z-index` on `.view-bar` if nothing else needs it. Update the comment block at `:18-20`: it currently documents "the window remains the scroller" as the governing decision. It must state the new arrangement and why — the sticky header a column header row needs cannot work inside a horizontally scrolling list while the window owns vertical scrolling. ### 3. Fix nowrap row width Move the `max-content` sizing from the row up to the list (`main.scss:171-184`), so the list is as wide as its widest row and rows fill it. Every row then paints across the full scroll width and backgrounds no longer stop at ragged positions. Re-examine the `padding-bottom: 0.75rem` at `:172-174`. It compensates for Firefox's overlay horizontal scrollbar covering the newest row when the window scrolled the list; with the list scrolling itself, verify whether it is still needed rather than carrying it over unexamined. ### 4. Scroll to the newest row against the new scroller In `assets/js/view-all.js:4-8`, `window.scrollTo(0, document.body.scrollHeight)` no longer targets the right element. Replace it with `scrollIntoView` on the last row rather than assigning `list.scrollTop = list.scrollHeight`: the two are equivalent today, but `scrollIntoView` forces the target to render and so stays correct if `content-visibility` is adopted later and makes `scrollHeight` an estimate. The `.is-ready` opacity gate (`main.scss:87-103`) stays as is — it still hides the initial jump, and still needs its `visibility: hidden` companion so a pre-scroll list is neither hit-testable nor findable by find-in-page. ### 5. Verify - `just test` — `tests/views.rs` asserts rendered HTML only (`.log-list.is-nowrap`, cell text per column) and is unaffected by scroller geometry. No test changes are expected; if any assertion moves, that is a signal something rendered differently and should be understood rather than re-pinned. - `just check` — clippy and fmt. - `just lint` — stylelint over the changed SCSS. - Manual, since no browser runs in CI and none of this is observable to the existing harness: wrap mode scrolls vertically with the chrome staying put; nowrap scrolls horizontally with the hover highlight spanning the full width at any horizontal offset; the list opens scrolled to the newest row with no visible jump; no double scrollbar and no clipped last row. ### Out of scope The column header itself, the grid layout and the cell model (#39); column state in the query (#40); resize and fit (#41); zebra and severity colouring (#42). `content-visibility`, a tail DOM cap, and the narrow-viewport reflow are deferred entirely. ### Noted, not fixed here `docs/STATUS.md` is stale: it states that `src/views/`, `templates/views/` and `templates/partials/log_row.html` do not exist and that there is no VictoriaLogs integration, while all of them are present. Pre-existing drift, unrelated to this change, and worth correcting separately rather than silently inside it.