Merge gh/main (33cf4969) into collapse-fu/gw18

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
Edward Langley
2026-09-23 08:03:56 -07:00
7 changed files with 166 additions and 3 deletions
+55
View File
@@ -4781,3 +4781,58 @@ while the browser composed `SetCell`, so the documents differed. The final
assertion that South holds 1200 is the half that keeps the test from passing
when both ends decline. Rival: `PasteAt::Resolved` back through `paste_into`.
This test fails.
### a_locally_resolved_page_scroll_lands_where_the_host_would_put_it
**What was missing (improvise-5xbq).** `tests/lockstep.rs` compared only `j`
and `G` against the host. The browser's page rule (`resolve::page_rows`) and
the TUI's (`Move`'s `Page` arm, `visible_rows * 3 / 4`) were two copies, and
two faults injected into the browser's copy left the whole workspace green:
`page_rows` returning `window_rows / 2`, and `grid_move` paging by
`window_cols`. The second was invisible for a further reason: the only window
the page-rule test used was square, `(8, 8)`.
**The fixture.** 20 rows by 3 columns; the host sees 8 rows and the client
announces `(8, 3)`. The three candidate rules then give three different
steps: 6 (three quarters of the rows), 4 (half the rows) and 2 (three quarters
of the columns). The walk starts off the edge (two `j`), then presses `PgDn`,
`PgUp`, `Ctrl-d` and `Ctrl-u`, including a `PgDn` that clamps at row 19.
**Why it pins the rows as well as comparing the ends.** `page_rows` is now the
one home (the TUI's `Move` calls it), so a wrong rule moves host and client
together, and a comparison alone passes. Each press therefore also asserts the
row the TUI's rule names.
**Injected, before and after the move to one home.**
| Fault | Two copies | One home |
|---|---|---|
| `page_rows` returns `window_rows / 2` | fails at press 2 (`PgDn`), "client and host disagree" | fails at press 2, "not the row the TUI's page rule names"; `app.rs`'s `page_down_scrolls_by_three_quarters_visible` and `page_up_scrolls_backward` fail too |
| `grid_move` pages by `window_cols` | fails at press 2, "client and host disagree" | fails at press 2, "client and host disagree" |
`Ctrl-d`/`Ctrl-u` are `scroll-rows ±5`, not the page rule. They are in the walk
because the ticket named them, and they move the start point of the next page
turn to a row the fixed rules alone would not reach.
### a narrow window still shows how to page through help — `harness/tests/help.spec.js`
**What broke (improvise-1c5m).** Below the width at which the app bar wraps,
`#help` (fixed, `inset: 2rem`, `z-index: 30`) covers `#hint`, and since PR #180
Welcome carries no paging instruction of its own. So a new user on Welcome at
phone width could not see how to reach page two.
**The fix is option (b) from the ticket.** The overlay draws `model.hint` in a
footer. In Help mode that is already `hint_line(Help)`, the same string `#hint`
carries, so it adds no derivation, and it sits outside `.help-body`, so
"Welcome is prose" still holds. The footer is `position: sticky` because the
overlay itself scrolls and Welcome is longer than a phone-height overlay.
**What the case asserts.** At 600x800 on Welcome, it reads the hint string from
`#hint`, so the spec spells no keys. It then requires some element showing
exactly that string to be the topmost element at its own centre, which is the
`elementFromPoint` measurement the ticket made by hand.
**Red, then a rival.** At base it failed with "the paging hint is under the
help overlay". With the fix and `position: sticky` changed to `static`, it
fails the same way: the footer exists but sits below the fold of a scrolling
overlay, so presence alone cannot satisfy the case.
@@ -118,7 +118,7 @@ impl Cmd for Move {
}
}
MoveKind::Page(dir) => {
let delta = (self.cursor.visible_rows as i32 * 3 / 4).max(1) * dir;
let delta = improvise_protocol::resolve::page_rows(self.cursor.visible_rows) * dir;
let nr = (self.cursor.row as i32 + delta).clamp(0, row_max) as usize;
(nr, self.cursor.col)
}
+2 -1
View File
@@ -620,7 +620,8 @@ enum GridMove {
}
/// Rows one `page-scroll` step moves: three quarters of the window, at least one.
fn page_rows(window_rows: usize) -> i32 {
/// The one home: the TUI's `Move` calls it too (improvise-5xbq).
pub fn page_rows(window_rows: usize) -> i32 {
(window_rows as i32 * 3 / 4).max(1)
}
@@ -266,6 +266,70 @@ fn a_locally_resolved_cursor_move_lands_where_the_host_would_put_it() {
);
}
/// Twenty rows by three columns, the host's window eight rows by three: not
/// square, so a rule keyed off the wrong axis lands somewhere else.
fn paging_host() -> App {
let mut wb = Workbook::new("Test");
wb.add_category("Region").unwrap();
for i in 0..20 {
wb.add_item("Region", format!("R{i:02}")).unwrap();
}
for m in ["A", "B", "C"] {
wb.add_item("_Measure", m).unwrap();
}
wb.active_view_mut().set_axis("Region", Axis::Row);
wb.active_view_mut().set_axis("_Measure", Axis::Column);
let mut app = App::new(wb, None);
app.term_height = 8 + 8;
app.rebuild_layout();
app
}
/// Page and half-page scrolls land in the same place on both ends, and in the
/// place the TUI's rule names. Why the fixture and the pinned rows: the evidence file.
#[test]
fn a_locally_resolved_page_scroll_lands_where_the_host_would_put_it() {
let window = (8, 3);
let mut app = paging_host();
let mut client = client_of(&app, window);
let key = |code, mods| KeyEvent::new(code, mods);
let none = KeyModifiers::NONE;
let ctrl = KeyModifiers::CONTROL;
let presses = [
(key(KeyCode::Char('j'), none), 1),
(key(KeyCode::Char('j'), none), 2),
(key(KeyCode::PageDown, none), 8),
(key(KeyCode::PageDown, none), 14),
(key(KeyCode::PageUp, none), 8),
(key(KeyCode::Char('d'), ctrl), 13),
(key(KeyCode::PageDown, none), 19),
(key(KeyCode::Char('u'), ctrl), 14),
(key(KeyCode::PageUp, none), 8),
(key(KeyCode::PageUp, none), 2),
];
for (press, (event, want_row)) in presses.into_iter().enumerate() {
match resolve(&client, event) {
KeyOutcome::Local(cmds) => {
for cmd in &cmds {
client.fold(cmd);
}
}
other => panic!("press {press}: the client must answer {event:?}, got {other:?}"),
}
reduce_key(&mut app, event);
assert_eq!(
client.view.cursor(),
app.view_state.cursor(),
"press {press} ({event:?}): client and host disagree"
);
assert_eq!(
client.view.cursor().selected,
(want_row, 0),
"press {press} ({event:?}): not the row the TUI's page rule names"
);
}
}
/// The same for `G`, the key `GridExtent` exists for: it is *defined* as "the
/// last row", so a window-only client would have nothing to jump to.
#[test]
+26
View File
@@ -132,4 +132,30 @@ test.describe("the help overlay draws the shared content", () => {
await expect(rows_.last().locator("td.selected")).toHaveCount(1);
await expect(page.locator("#grid tbody td.selected")).toHaveCount(1);
});
// improvise-1c5m. Below the width at which the app bar wraps, `#help` covers
// `#hint`, and Welcome itself says nothing about paging. The contract: the
// mode's own hint is UNCOVERED somewhere while help is open.
test("a narrow window still shows how to page through help", async ({ page }) => {
await page.setViewportSize({ width: 600, height: 800 });
await openHelp(page);
// The string is the page's own `#hint`, i.e. `hint_line(Help)`; this spec
// spells no keys. Asserting it names paging keeps an empty hint out.
const hint = (await page.locator("#hint").textContent()).trim();
expect(hint).toContain("next-page");
// An element showing exactly that string, topmost at its own centre: the
// overlay covering `#hint` is precisely what `elementFromPoint` sees.
const uncovered = await page.evaluate((text) =>
[...document.querySelectorAll("body *")].some((node) => {
if (node.children.length > 0 || node.textContent.trim() !== text) return false;
const r = node.getBoundingClientRect();
if (r.width === 0 || r.height === 0) return false;
const top = document.elementFromPoint(r.left + r.width / 2, r.top + r.height / 2);
return top !== null && (top === node || node.contains(top));
}),
hint);
expect(uncovered, "the paging hint is under the help overlay").toBe(true);
});
});
+7 -1
View File
@@ -348,7 +348,13 @@ function paintHelp(model) {
}
}
el.help.replaceChildren(tabs, body);
// The mode's own hint again, as the TUI popup's footer: once the app bar
// wraps, the overlay covers `#hint` (improvise-1c5m).
const footer = document.createElement("p");
footer.className = "help-footer";
footer.textContent = model.hint;
el.help.replaceChildren(tabs, body, footer);
}
/** The page bar: which item each fixed category is fixed to. Without it a
+11
View File
@@ -736,6 +736,17 @@ td.total {
font-weight: 700;
}
/* Sticky, so the paging keys stay in view however long the page scrolls. */
.help-footer {
position: sticky;
bottom: -1.25rem;
margin: 0.75rem 0 0;
padding: 0.5rem 0;
border-top: 1px solid var(--line);
background: var(--bg);
color: var(--dim);
}
.help-body {
display: grid;
grid-template-columns: 16rem 1fr;