mirror of
https://github.com/remorses/gpuix.git
synced 2026-10-29 11:14:49 +08:00
Keep a virtual-list pinned to the top when rows are prepended
`gpui::ListState` anchors on a **logical item**, and `splice_focusable`
shifts that anchor by the number of rows inserted before it. A prepend
therefore keeps the rows already on screen and pushes the new one above
the viewport. Correct for a history pane, wrong for a feed.
A browser anchors the same way and suppresses it at `scrollTop: 0`.
`VirtualListEntry::sync` now copies that: it remembers a top-aligned,
non-`followTail` list whose `logical_scroll_top()` is `{0, 0}` and calls
`scroll_to(default)` after the splice. Scrolled anywhere else, the rows
under the pointer still do not move.
**How it looked**
`example-app` prepends the newest task. Adding through the composer:
add 1..10 top row = task N offset 0 looks fine
add 11 top row = task 11 offset -32 content now overflows
add 12 top row = task 11 offset -76 new row is off screen
add 20 top row = task 11 offset -428 list looks frozen
**Why nobody caught it**
While the content is shorter than the viewport, gpui's "does not fill"
branch walks back up to item 0 and re-anchors there on every layout, so
the drift is invisible. It appears on the frame the list first overflows.
The regression test grows a 160px list from 2 rows to 12 rather than
starting tall, and a second test asserts the anchor still holds below the
top.
Session: ses_fc16226ffffeOn4QTVIbw4K7Nz
This commit is contained in:
1 parent
c444c28ebf
commit
01f5788f0f
5 files changed
+151
No files matched your search
@@ -0,0 +1,40 @@
|
||||
---
|
||||
'@gpuix/native': patch
|
||||
'@gpuix/react': patch
|
||||
---
|
||||
|
||||
Keep `<virtual-list>` at the top when rows are prepended.
|
||||
|
||||
A list is anchored on a **row**, not a pixel offset, so inserting rows above the
|
||||
viewport used to slide the viewport down by exactly the height of the new rows.
|
||||
A todo list or a feed that prepends the newest item never showed it:
|
||||
|
||||
```tsx
|
||||
const add = (title: string) =>
|
||||
setTodos((current) => [{ id: nextId(), title }, ...current])
|
||||
```
|
||||
|
||||
Every add pushed the new row above the top edge, and the list looked frozen on
|
||||
the rows that were already there.
|
||||
|
||||
A browser anchors the same way, and suppresses it at `scrollTop: 0`. GPUIX now
|
||||
does the same: a top-aligned list that is scrolled to the very top stays at the
|
||||
top across a mutation, so the prepended row is visible. Scrolled anywhere else,
|
||||
the rows under the pointer still do not move.
|
||||
|
||||
```text
|
||||
scrolled down pinned to the top
|
||||
┌──────────────────┐ ┌──────────────────┐
|
||||
│ new row (above) │ ◄── inserted │ new row │ ◄── inserted, visible
|
||||
├──────────────────┤ ├──────────────────┤
|
||||
│ ░░ viewport ░░░░ │ stays put │ ░░ viewport ░░░░ │ follows the insert
|
||||
│ ░░░░░░░░░░░░░░░░ │ │ ░░░░░░░░░░░░░░░░ │
|
||||
└──────────────────┘ └──────────────────┘
|
||||
```
|
||||
|
||||
A history pane that loads older pages while the user reads should keep using
|
||||
`alignment="bottom"`, where a page load never moves the text.
|
||||
|
||||
The drift was invisible while the content was shorter than the viewport, because
|
||||
gpui re-anchors to row 0 on every layout in that case. It appeared the moment
|
||||
the list overflowed.
|
||||
@@ -295,6 +295,25 @@ default zeroes the smaller of the two deltas, so one diagonal wheel moved one
|
||||
axis. A browser moves both, and a two-axis container is exactly where a user
|
||||
expects that.
|
||||
|
||||
## A prepended row is only visible at the top
|
||||
|
||||
`gpui::ListState` anchors on a **logical item**, and `splice_focusable` shifts
|
||||
that anchor by the number of rows inserted before it. So a prepend keeps the
|
||||
rows already on screen and pushes the new one above the viewport. That is
|
||||
correct for a history pane, and wrong for a feed.
|
||||
|
||||
A browser anchors the same way and suppresses it at `scrollTop: 0`. GPUIX copies
|
||||
that: `VirtualListEntry::sync` remembers a top-aligned, non-`followTail` list
|
||||
whose `logical_scroll_top()` is `{0, 0}` and calls `scroll_to(default)` after the
|
||||
splice. Do not "simplify" that away.
|
||||
|
||||
**Do not trust a short list to prove a prepend works.** While the content is
|
||||
shorter than the viewport, gpui's "does not fill" branch re-anchors to item 0 on
|
||||
every layout, so the drift is invisible. It appears on the frame where the list
|
||||
first overflows. `example-app` looked stuck on two rows for exactly that reason,
|
||||
and the regression test in `virtual-list.test.tsx` grows a 160px list from 2 rows
|
||||
to 12 rather than starting tall.
|
||||
|
||||
## A frozen header cannot use native scroll
|
||||
|
||||
GPUI moves a scroll container on the wheel frame. The `onScroll` callback that
|
||||
|
||||
@@ -851,6 +851,24 @@ Combine `alignment="bottom"` and `followTail` for a chat thread:
|
||||
|
||||
The list follows new rows while the user is at the bottom. Scrolling upward pauses tail following. Returning to the bottom enables it again. A streaming final row is remeasured as its content grows.
|
||||
|
||||
### Scroll anchoring
|
||||
|
||||
The list is anchored on a **row**, not on a pixel offset, so inserting rows above the viewport keeps the rows already on screen exactly where they are. A browser does the same, and calls it scroll anchoring.
|
||||
|
||||
One exception, also copied from the browser: a top-aligned list that is scrolled to the **very top** stays at the top, so a prepended row is visible.
|
||||
|
||||
```text
|
||||
scrolled down pinned to the top
|
||||
┌──────────────────┐ ┌──────────────────┐
|
||||
│ new row (above) │ ◄── inserted │ new row │ ◄── inserted, visible
|
||||
├──────────────────┤ ├──────────────────┤
|
||||
│ ░░ viewport ░░░░ │ stays put │ ░░ viewport ░░░░ │ follows the insert
|
||||
│ ░░░░░░░░░░░░░░░░ │ │ ░░░░░░░░░░░░░░░░ │
|
||||
└──────────────────┘ └──────────────────┘
|
||||
```
|
||||
|
||||
That is what a todo list or a feed wants: `setItems((current) => [fresh, ...current])` puts the new row on screen. A history pane that loads older pages while the user reads should use `alignment="bottom"` instead, so a page load never moves the text.
|
||||
|
||||
### Programmatic scrolling
|
||||
|
||||
Use a ref to call the same renderer scroll methods as a plain scroll container:
|
||||
|
||||
@@ -3132,6 +3132,21 @@ impl VirtualListEntry {
|
||||
return;
|
||||
}
|
||||
|
||||
// gpui anchors a list on a logical item, so splicing rows in at the
|
||||
// front keeps the rows already on screen and pushes the new ones above
|
||||
// the viewport. A browser anchors too, but suppresses it at scrollTop 0,
|
||||
// so a prepend is visible. Match the browser: remember a list pinned to
|
||||
// the top and put it back after the splice.
|
||||
//
|
||||
// While the content is shorter than the viewport gpui re-anchors to
|
||||
// item 0 every layout, so the drift only appears once the list
|
||||
// overflows. That is why `example-app` looked stuck at two rows.
|
||||
let top = self.state.logical_scroll_top();
|
||||
let was_pinned_to_top = matches!(config.alignment, gpui::ListAlignment::Top)
|
||||
&& !config.follow_tail
|
||||
&& top.item_ix == 0
|
||||
&& top.offset_in_item <= gpui::px(0.0);
|
||||
|
||||
// A windowed list's children are a sliding viewport. Splicing by
|
||||
// child position would treat a scroll as a rewrite of items 0..N.
|
||||
if config.item_count.is_none() && self.child_ids != child_ids {
|
||||
@@ -3192,6 +3207,9 @@ impl VirtualListEntry {
|
||||
.remeasure_items(start..window_start + child_ids.len());
|
||||
}
|
||||
self.remeasure_unknown_rows(window_start, &child_ids, &old_rows);
|
||||
if was_pinned_to_top {
|
||||
self.state.scroll_to(gpui::ListOffset::default());
|
||||
}
|
||||
|
||||
self.window_start = window_start;
|
||||
self.child_ids = child_ids;
|
||||
|
||||
@@ -358,6 +358,62 @@ describe("<virtual-list>", () => {
|
||||
expect(painted.length).toBeLessThan(5)
|
||||
})
|
||||
|
||||
// gpui anchors a list on a logical item, so a prepend keeps the rows that are
|
||||
// already on screen and pushes the new ones above the viewport. A browser does
|
||||
// the same, except that it suppresses scroll anchoring at scrollTop 0. A list
|
||||
// pinned to the top must match the browser, or a prepend is never seen.
|
||||
//
|
||||
// This only bites once the content is taller than the viewport. While it is
|
||||
// shorter, gpui re-anchors to item 0 on every layout and hides the drift.
|
||||
const grown = (count: number) => (
|
||||
<virtual-list
|
||||
overdraw={0}
|
||||
estimatedItemHeight={40}
|
||||
style={{ width: 400, height: 160 }}
|
||||
>
|
||||
{Array.from({ length: count }, (_, index) => (
|
||||
<div
|
||||
key={count - index}
|
||||
style={{
|
||||
display: "flex",
|
||||
height: 40,
|
||||
flexShrink: 0,
|
||||
alignItems: "center",
|
||||
}}
|
||||
>
|
||||
<text>{`row-${count - index}`}</text>
|
||||
</div>
|
||||
))}
|
||||
</virtual-list>
|
||||
)
|
||||
|
||||
it("stays at the top when rows are prepended past the viewport", () => {
|
||||
const { render, renderer } = createTestRoot()
|
||||
|
||||
render(grown(2))
|
||||
expect(renderer.getPaintedText()[0]).toBe("row-2")
|
||||
|
||||
for (let count = 3; count <= 12; count += 1) {
|
||||
render(grown(count))
|
||||
expect(renderer.getPaintedText()[0], `after ${count} rows`).toBe(
|
||||
`row-${count}`,
|
||||
)
|
||||
}
|
||||
})
|
||||
|
||||
it("keeps the scroll anchor when rows are prepended below the top", () => {
|
||||
const { render, renderer } = createTestRoot()
|
||||
|
||||
render(grown(12))
|
||||
const list = renderer.findByType("virtual-list")[0]
|
||||
renderer.scrollToItem(list.id, 5)
|
||||
expect(renderer.getPaintedText()[0]).toBe("row-7")
|
||||
|
||||
// Away from the top, a prepend must not move the rows under the pointer.
|
||||
render(grown(13))
|
||||
expect(renderer.getPaintedText()[0]).toBe("row-7")
|
||||
})
|
||||
|
||||
it("lets overflow-x inside a row pan without moving the list", () => {
|
||||
const { render, renderer } = createTestRoot()
|
||||
render(
|
||||
|
||||
Reference in new issue
Block a user