fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(browser): paint the page bar and the totals strip, and delegate the pointer

Browse Source 
improvise-p7lh: `GridCache::page_coords` and `GridCache::totals` were projected
to every session and folded correctly by every client, and NO front end read
either. The shipped front page — `web/demo.improv` is `Quarter: page, Q1` —
drew a grid filtered to Q1 with nothing saying so, and `[`/`]` resolve locally
off that same `page_coords`, so a user could page through quarters watching
every number change with no way to tell which one they were looking at.

`RenderModel` gains `page_coords` and `totals`, fed straight off `state.grid`.
The Page/Filter split is asked of `PageAxis::cycle` rather than re-derived from
`items.is_some()`, which disagrees with it on an empty item list (gotcha 27).
The totals are RELAYED, never summed in the client: the sum runs over every row
including ones a viewport scrolled past (gotcha 44).

`web/tests/render_surface.test.js` gains the direction that would have caught
this when improvise-n10 added the fields. The existing guard is closed under
what `RenderModel` chose to expose, so a cache field that never reaches it is
invisible to it; the new one asks that every `pub` field of every widget cache
is read somewhere the browser runs, or is named as debt with a reason.
`grid.col_widths` is the one entry, pointing at improvise-pcje.

improvise-b8ot: the five clickable surfaces moved from one listener per node,
reallocated inside the render loops on every repaint, to one handler per stable
delegation root — the user's ruling. The operands are carried on the node as
data attributes and read back by a pure `cellAction`/`indexAction` in
`web/shell.js`, so the handler is a LOOKUP rather than a second derivation of
which data row was clicked: `gridPlan` counts data rows because a group header
is not one, and walking the table back up would disagree the moment one appears.
The editor's own keydown/blur stay direct — one transient node, one at a time,
and `blur` does not bubble.

All four documented behaviours a naive delegation loses are preserved and each
is now asserted in `harness/tests/contract.spec.js`: mousedown rather than click
(asserted by dispatching mousedown ALONE), `preventDefault` and only over a cell,
the conditionally skipped repaint (asserted by the editor's survival), and
closing a live editor before composing the click.

Gates: 1598 passed / 0 failed / 7 ignored over 37 binaries (baseline 1595);
node --test 47 over 6 files (baseline 36 over 5); Playwright 47 passed /
4 skipped (baseline 43 / 4); clippy, fmt and --check-evidence clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
Edward Langley
2026-09-21 19:21:57 -07:00
parent 523d57197c
commit a93d668e5e
11 changed files with 819 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/test-evidence/clients.md
+153
View File
@@ -397,6 +397,65 @@ sees a keystroke do nothing, types over it, and commits something they never
read. `None` outside an editing mode is the other half: a stale buffer must not
draw an editor that is not open.
### the_page_bar_names_every_fixed_category_and_what_it_is_fixed_to
(`render.rs`, improvise-p7lh.) `GridCache::page_coords` was projected to every
session, folded correctly by every client, and read by **no** front end — so the
shipped front page (`web/demo.improv` is `Quarter: page, Q1`) drew a grid
filtered to Q1 with nothing anywhere saying so. Worse, `[`/`]` resolve *locally*
in the browser off that same `page_coords`, so a user could press `]`, watch
every number in the grid change, and have no way to tell which quarter they were
looking at or how to get back. The numbers were right and the frame was a lie by
omission.
Proved able to fail by deleting the `page_coords:` line from `render_model`,
which is the state the bug shipped in.
Why this is the assertion and not "the field exists": the bar's whole job is to
name *which item*, so `selected` is what discriminates. A bar that drew the
category name alone would satisfy a presence check and still leave the user
unable to tell Q1 from Q2.
### only_a_navigable_page_axis_offers_a_cycle
(`render.rs`, gotcha 27.) Page is navigable; Filter constrains without being
navigable, and a client must compose no cycle for one. The one home of that
split is `PageAxis::cycle`, and the plausible wrong answer is `items.is_some()`
— which agrees with it on the two common cases and disagrees on the third.
Three axes in one fixture, because two would not separate the readings:
- a Page axis with three items, selected at index 2. **Injected** `items.is_some()`
with `at: 0` and watched it fail on the index — a cycle that always reports
position 1 reads as correct on a freshly-opened document and is wrong the
moment anyone pages.
- a Filter axis (`items: None`). Both readings agree here, which is why it
cannot be the only case.
- an axis with `items: Some(vec![])`. `PageAxis::cycle` filters the empty list
and says `None`; `items.is_some()` says there is a cycle of length zero. This
is the case that makes the test discriminating rather than decorative.
### the_totals_strip_is_relayed_and_an_empty_one_stays_empty
(`render.rs`, improvise-p7lh.) The same shape as the page bar: `GridCache::totals`
projected, folded, and painted by nothing. The TUI draws it (`src/ui/grid.rs`);
the browser had no field for it.
Proved able to fail by replacing `totals_strip`'s body with `Vec::new()` — the
state the bug shipped in — and watching it fail on the relayed text.
Two halves, and the second is the one gotcha 33 is about: an **empty** strip
means "this grid has no totals" (records mode, or an empty grid) and must stay
empty rather than becoming a row of blanks. A client that padded it would draw a
blank total row under a records grid that has none.
What is deliberately **not** asserted anywhere: that the numbers are right. They
are the host's, summed over every row *including ones this viewport scrolled
past*, which is exactly why they are projected. A client-side check of the
arithmetic would be the second derivation the projection exists to prevent — so
the arithmetic is pinned end to end instead, in `contract.spec.js` below, against
a real document where the sum is a fact.
---
## `improvise-wasm-client/tests/browser_round_trip.rs`
@@ -1788,6 +1847,13 @@ clearing the timer, so the watchdog *pre-empts* rather than being disarmed —
the bug PR #13 fixed and PR #21 half-lost; `extentOf` stops subtracting the
frame; `frameCostOf` stops counting the column-group line.
**`grid_plan.test.js`, the totals strip** (improvise-p7lh) — `plan.totals` is
`null` where the strip is empty and never an empty row, because an empty strip
means "this grid has no totals" rather than "not caught up yet" (gotcha 33). The
label spans every row-header level, and the cells carry no coordinates at all —
a total is not a cell a click can name, which is what keeps the delegated
`td[data-row]` handler off the `tfoot` without a second rule saying so.
**`grid_plan.test.js`** — the data-row index is taken from the row's position
in `rows`, so a group header consumes one (the click coordinate bug gotcha 1
is about); the group label is tested for truthiness, so a group legitimately
@@ -1807,6 +1873,43 @@ first is the shape of the bug the four widgets shared, and the list it checks
against currently names `doc_name`/`doc_dirty`/`doc_stored_as` — improvise-5jna,
which this test found rather than remembered.
**`dispatch.test.js`** (improvise-b8ot, improvise-p7lh) — the delegated
handler's operands are read back off the node, so the round trip
`cellAction(cellAttrs(cell), …)` is the property, and the fixture puts a group
header between its data rows because that is the one input where "the data row"
and "the table row" differ. Injections: `cellAttrs` writes the row's position in
`rows` rather than `gridPlan`'s counted index, which is the click-coordinate bug
gotcha 1 is about and is invisible on any ungrouped grid; `whole()` drops its
empty-string guard, so a header with no `data-row` reads as row 0 and every
click on the frame moves the selection to the origin (`Number("")` is `0`, which
is why the guard is not a truthiness test); `pageEntries` reports `navigable`
off `position !== ""`, which is the same answer by accident and stops being one
the moment a Page axis's cycle is asked for.
**`render_surface.test.js`, the cache direction** (improvise-p7lh) — the
existing guard is closed under *what `RenderModel` chose to expose*, so a fact
that never reaches it is invisible to it. That is precisely how `page_coords`
and `totals` sat projected and unpainted from improvise-n10 onward with a green
suite. The new assertion is one level up: every `pub` field of every widget
cache on `ClientState` must be read somewhere the browser actually runs, or be
named as debt.
Two things it got wrong first, both found by injection rather than by reading:
- The whole-cache escape hatch was a substring test for `&state.grid`, and
`&state.grid.row_frame` **contains** that string — so every `GridCache` field
read as handed-over and the check was vacuous for the one cache the bead is
about. It is now anchored with `(?![.\w])`.
- rustfmt breaks `state.grid.totals` across three lines, so a substring search
reported a field that *is* read as unread. The sources are whitespace-stripped
before the search.
The residual hole, stated because it is real: where a whole cache is handed to a
shared derivation (`flatten_cat_tree(&state.categories, …)`), every field of that
cache counts as reached. A second field added to `CategoryTreeCache` and ignored
by the derivation would pass. Closing that needs the *consumer* checked, not the
call site, and is not attempted here.
**`bundle/wasm_surface.test.js`** — `client.render_model()` renamed to
`client.render_modal()` fails both its tests. It reads the generated `.d.ts`, so
a missing bundle is an assertion failure and never a skip.
@@ -2121,6 +2224,56 @@ The other two losses are deliberate: `pointer.spec.js`'s sidebar test was
superseded by the open-at-launch ruling, and the widgets file was merged into
this one.
## `harness/tests/contract.spec.js` — the page bar, and the pointer's delegation
**"the page bar names the quarter, and both it and the totals follow `]`"**
(improvise-p7lh) is the end-to-end half the Rust tests deliberately do not
attempt. It runs against the shipped `web/demo.improv`, where `Quarter` really
is on `Axis::Page` with four items, and asserts the *arithmetic*: Q1's Revenue
total is `63,700` and Q2's is `66,400`, each a sum over all four products. That
is why the totals must be **projected** — the number is a fact about every row,
and a client that summed what it had been sent would be right only by accident
and wrong the moment a viewport scrolled.
Pressing `]` is what makes it a test of the pair rather than of two fields: a
bar that showed a stale quarter over fresh numbers is the precise lie the bead
names, and it passes any assertion that reads the two independently.
**The three delegation tests** (improvise-b8ot) each pin a property a naive
rewrite loses *silently* — the page still paints, and only the pointer stops
working, which no unit test and no render-model contract can see:
- **mousedown, not click.** Asserted by dispatching `mousedown` **alone**, with
no click behind it. A handler bound to `click` does nothing at all here. This
is improvise-0x9u's mechanism from the other side: from an editing state the
input's `blur` repaints first and the `<td>` is detached before `click` would
fire, so the event never arrives. `locator.click()` — which every other
pointer test uses — dispatches both and therefore cannot separate them.
- **preventDefault, and only over a cell.** Read off `window`, which is last in
the bubble path and so sees what `#grid`'s handler did. The second half is
what makes it discriminating: a header carried no listener before delegation,
so a blanket handler on the root that prevents *everything* is a behaviour
change, and asserting only the cell would pass for it.
- **The coordinates come off the node.** The test rewrites a `<td>`'s
`data-row` in the DOM and requires the click to land where the *attribute*
says. A per-node closure over `gridPlan`'s answer ignores the rewrite and
fails. This is the assertion that says the handler is a **lookup** rather than
a second derivation of which data row was clicked — the thing the bead is
actually about, and the thing a "does clicking still work" test cannot tell
apart from the old code.
**The fourth property — closing a live editor first — is not a new test.** It is
`"a click moves the selection from an editing state too"`, already in this file,
which clicks a second cell with no Escape between and asserts the first cell's
`<input>` is gone and the second's is open. Adding a fifth spec asserting the
same ordering would have been a second copy, not a second guard.
**And the repaint-skipping branch is asserted by survival**: after a mousedown
on an editable cell the `<input>` must still be there three hundred
milliseconds later, because a repaint would call `replaceChildren` and take it
away. That is the one of the four whose failure is *most* invisible — the editor
opens and vanishes within a frame, which reads as a click that did nothing.
### the_keys_that_compose_a_switch_view_are_these
improvise-lltt is deferred on a **premise**, not a judgement: its own note says it
crates/improvise-wasm-client/src/render.rs
+153 -2
View File
@@ -32,6 +32,12 @@ pub struct RenderModel {
/// How many row-header levels every [`RenderRow::Data`] carries.
pub row_levels: usize,
pub rows: Vec<RenderRow>,
/// The column totals strip, **empty where this grid has none** — the same
/// fact `GridCache::totals` carries, never a sum taken here (gotcha 44).
pub totals: Vec<RenderCell>,
/// The page bar: one entry per category fixing which records are visible.
/// Page **and** Filter; `cycle` is what separates them (gotcha 27).
pub page_coords: Vec<RenderPageAxis>,
/// The category panel, flattened by the one flattening.
pub categories: Vec<RenderCatRow>,
/// Whether the panel is open, so the page does not decide.
@@ -145,6 +151,25 @@ impl RenderCell {
}
}
/// One category fixing what the grid shows, and what it is fixed to. The page
/// spells neither the split nor the cycle — `context/code/clients.md`.
#[derive(Debug, Clone, Default, PartialEq, Serialize)]
pub struct RenderPageAxis {
pub cat: String,
pub selected: String,
/// Where `selected` sits in what `[`/`]` cycle. **`None` for a Filter
/// axis**, which constrains without being navigable (gotcha 27).
pub cycle: Option<RenderPageCycle>,
}
/// A position in a navigable page axis, as a named pair: two bare counts read
/// as "how many page axes are there".
#[derive(Debug, Clone, Copy, Default, PartialEq, Serialize)]
pub struct RenderPageCycle {
pub at: usize,
pub count: usize,
}
/// One row of the category panel. `item_count` and `expanded` are a category
/// header's own facts; an item row carries neither, hence `Option`.
#[derive(Debug, Clone, Default, PartialEq, Serialize)]
@@ -251,6 +276,8 @@ pub fn render_model(state: &ClientState) -> RenderModel {
})
.flatten()
.collect(),
totals: totals_strip(state),
page_coords: page_axes(state),
categories: entries
.iter()
.enumerate()
@@ -277,6 +304,37 @@ pub fn render_model(state: &ClientState) -> RenderModel {
}
}
/// The column totals, relayed. **Never summed here**: the total runs over
/// every row, including the ones this viewport scrolled past (gotcha 44).
fn totals_strip(state: &ClientState) -> Vec<RenderCell> {
// `RenderCell::of` is the one conversion, and its answers are honest for a
// total: `id` is `None`, so nothing offers it as editable.
state
.grid
.totals
.iter()
.map(|cell| RenderCell::of(cell, false))
.collect()
}
/// The page bar. `PageAxis::cycle` is the one home of the Page/Filter split,
/// so this asks it rather than testing `items` itself (gotcha 27).
fn page_axes(state: &ClientState) -> Vec<RenderPageAxis> {
state
.grid
.page_coords
.iter()
.map(|axis| RenderPageAxis {
cat: axis.cat.clone(),
selected: axis.selected.clone(),
cycle: axis.cycle().map(|(items, at)| RenderPageCycle {
at,
count: items.len(),
}),
})
.collect()
}
/// The view panel's rows while it is open. The axis line is
/// `viewmodel::view_axis_summary`, which the TUI's own panel calls too.
fn view_rows(state: &ClientState) -> Vec<RenderViewRow> {
@@ -480,8 +538,8 @@ mod tests {
use crate::init;
use improvise_core::view::Axis;
use improvise_protocol::{
CategoryRow, CategoryTreeUpdate, Command, GridUpdate, Projection, RecordCell, Snapshot,
TileEntry, ViewState, ViewSummary,
CategoryRow, CategoryTreeUpdate, Command, GridUpdate, PageAxis, Projection, RecordCell,
Snapshot, TileEntry, ViewState, ViewSummary,
};
fn surrogate() -> improvise_protocol::RecordHash {
@@ -889,4 +947,97 @@ mod tests {
"an editor seeds from the stored value, never from the drawn one"
);
}
fn page_axis(cat: &str, selected: &str, items: Option<Vec<&str>>) -> PageAxis {
PageAxis {
cat: cat.into(),
selected: selected.into(),
items: items.map(|i| i.into_iter().map(String::from).collect()),
}
}
/// improvise-p7lh: the page bar reached the client's cache and stopped
/// there, so a grid filtered to Q1 said nothing at all. Evidence file.
#[test]
fn the_page_bar_names_every_fixed_category_and_what_it_is_fixed_to() {
let mut client = init();
client
.state_mut()
.fold(&Command::Project(Projection::Grid(GridUpdate::PageCoords(
vec![page_axis("Quarter", "Q2", Some(vec!["Q1", "Q2", "Q3"]))],
))));
let bar = client.model().page_coords;
assert_eq!(bar.len(), 1, "one fixed category, one entry");
assert_eq!(bar[0].cat, "Quarter");
assert_eq!(
bar[0].selected, "Q2",
"WHICH item the grid is filtered to is the whole point of the bar"
);
}
/// Gotcha 27: Page is navigable and Filter is not, and `PageAxis::cycle`
/// is the one home of that split — `items.is_some()` is not it.
#[test]
fn only_a_navigable_page_axis_offers_a_cycle() {
let mut client = init();
client
.state_mut()
.fold(&Command::Project(Projection::Grid(GridUpdate::PageCoords(
vec![
page_axis("Quarter", "Q3", Some(vec!["Q1", "Q2", "Q3"])),
page_axis("Region", "North", None),
page_axis("Empty", "x", Some(vec![])),
],
))));
let bar = client.model().page_coords;
assert_eq!(
bar[0].cycle,
Some(RenderPageCycle { at: 2, count: 3 }),
"a Page axis says where `[`/`]` currently sit"
);
assert_eq!(
bar[1].cycle, None,
"a Filter axis constrains without being navigable: nothing to cycle"
);
assert_eq!(
bar[2].cycle, None,
"and an EMPTY item list is no cycle either — `items.is_some()` says it is"
);
}
/// The totals strip is relayed, never summed here: the total runs over
/// rows this viewport scrolled past (gotcha 44). improvise-p7lh.
#[test]
fn the_totals_strip_is_relayed_and_an_empty_one_stays_empty() {
let bare = client_with_grid(ViewState::default()).model();
assert!(
bare.totals.is_empty(),
"an empty strip means this grid has NO totals, not a row of blanks"
);
let mut client = client_with_grid(ViewState::default());
client
.state_mut()
.fold(&Command::Project(Projection::Grid(GridUpdate::Totals(
vec![
CellDisplay::number("9,001"),
CellDisplay::number("2"),
CellDisplay::number("3"),
],
))));
let totals = client.model().totals;
assert_eq!(totals.len(), 3, "one total per projected column");
assert_eq!(
totals[0].text, "9,001",
"the host's formatted sum, not a number this client added up"
);
assert!(
totals[0].numeric,
"a total right-aligns like the cells above it"
);
assert!(
!totals[0].editable && !totals[0].selected,
"a total is not a cell anyone may type into or select"
);
}
}
harness/tests/contract.spec.js
+101
View File
@@ -134,6 +134,39 @@ test.describe("every field the render model projects reaches the screen", () =>
).toBeVisible();
await expect(rowHeaders).not.toHaveText(["Espresso", "Latte", "Cold Brew", "Pastry"]);
});
// improvise-p7lh. `page_coords` and `totals` were projected to every session
// and folded correctly, and NO front end read either: the shipped front page
// showed a grid filtered to Q1 with nothing on screen saying so, and `]`
// changed every number in silence.
test("the page bar names the quarter, and both it and the totals follow `]`", async ({
page,
}) => {
await openApp(page);
// `web/demo.improv` is `Quarter: page, Q1` — the fact this bead is about.
await expect(page.locator("#pages")).toBeVisible();
await expect(page.locator("#pages .page-axis")).toHaveText("Quarter = Q1 (1/4)");
// Page is navigable where Filter is not (gotcha 27), and that is what
// says `[`/`]` will move it.
await expect(page.locator("#pages .page-axis")).toHaveClass(/navigable/);
// The totals strip. Q1 Revenue is 18400+24600+9200+11500 — a sum over
// EVERY row, which is why it is projected and not added up here.
await expect(page.locator("#grid tfoot th")).toHaveText("Total");
await expect(page.locator("#grid tfoot td").first()).toHaveText("63,700");
// A total is not a cell anyone may type into.
await expect(page.locator("#grid tfoot td.editable")).toHaveCount(0);
await page.keyboard.press("]");
// BOTH move, and the numbers are Q2's: a bar that showed a stale quarter
// over fresh numbers is the exact lie this bead names.
await expect(page.locator("#pages .page-axis")).toHaveText("Quarter = Q2 (2/4)", {
timeout: 10_000,
});
await expect(page.locator("#grid tfoot td").first()).toHaveText("66,400");
});
});
test.describe("a pointer event reaches the client and its answer is painted", () => {
@@ -248,6 +281,74 @@ test.describe("a pointer event reaches the client and its answer is painted", ()
await expect(cells.nth(4)).toHaveClass(/selected/);
await expect(page.locator("#grid tbody td.selected")).toHaveCount(1);
});
// improvise-b8ot moved these five surfaces onto one handler per stable root.
// The three tests below are the properties a naive delegation loses, each of
// which is silent: the page still paints, and only the pointer stops working.
// The fourth — closing a live editor FIRST — is the test above, which asserts
// the old editor is gone and the new one is open with no Escape between.
test("a mousedown alone reaches the grid, because click comes too late", async ({
page,
}) => {
await openApp(page);
const cells = page.locator("#grid tbody td");
// MOUSEDOWN ONLY, dispatched with no click behind it. A handler bound to
// `click` does nothing here, which is the whole of improvise-0x9u: from an
// editing state the blur repaints and the <td> is gone before click fires.
await cells.nth(4).dispatchEvent("mousedown");
await expect(cells.nth(4).locator("input")).toHaveCount(1);
// And the repaint is SKIPPED on that branch: a repaint would take the
// <input> away, so its survival is the assertion.
await page.waitForTimeout(300);
await expect(page.locator("#grid input")).toHaveCount(1);
});
test("the grid prevents its own default, and only over a cell", async ({ page }) => {
await openApp(page);
// `window` is last in the bubble path, so it sees what #grid's handler did.
await page.evaluate(() => {
window.__defaultPrevented = null;
window.addEventListener("mousedown", (e) => {
window.__defaultPrevented = e.defaultPrevented;
});
});
await page.locator("#grid tbody td").nth(1).dispatchEvent("mousedown");
expect(await page.evaluate(() => window.__defaultPrevented)).toBe(true);
// A header carried no listener before delegation, so preventing ITS
// default would be new behaviour — the check must be on the cell, not the
// root. Without this the first assertion passes for a blanket handler.
await page.evaluate(() => {
window.__defaultPrevented = null;
});
await page.locator("#grid thead th").last().dispatchEvent("mousedown");
expect(await page.evaluate(() => window.__defaultPrevented)).toBe(false);
});
test("a click's coordinates are read off the node, not closed over", async ({
page,
}) => {
await openApp(page);
const cells = page.locator("#grid tbody td");
await expect(cells.nth(0)).toHaveClass(/selected/);
// Rewriting the attribute must change where the click lands. A per-node
// closure over `gridPlan`'s answer ignores it and this fails — which is
// what makes the delegated handler a LOOKUP rather than a second
// derivation of which data row was clicked (improvise-b8ot).
await cells.nth(0).evaluate((td) => {
td.dataset.row = "2";
});
await cells.nth(0).dispatchEvent("mousedown");
await page.keyboard.press("Escape");
const thirdRow = page.locator("#grid tbody tr").nth(2).locator("td").first();
await expect(thirdRow).toHaveClass(/selected/);
await expect(page.locator("#grid tbody td.selected")).toHaveCount(1);
});
});
test.describe("the selection is on screen", () => {
web/app.js
+100 -22
View File
@@ -6,15 +6,19 @@ import {
COL_PX,
HANDSHAKE_TIMEOUT_MS,
ROW_PX,
cellAction,
cellAttrs,
classifyWorkerMessage,
createShell,
documentTitle,
extentOf,
frameCostOf,
gridPlan,
indexAction,
indicator,
isFatal,
noticeText,
pageEntries,
socketUrl,
} from "./shell.js";
@@ -33,6 +37,7 @@ const el = {
formulas: document.getElementById("formulas"),
views: document.getElementById("views"),
tiles: document.getElementById("tiles"),
pages: document.getElementById("pages"),
mode: document.getElementById("mode"),
minibuffer: document.getElementById("minibuffer"),
hint: document.getElementById("hint"),
@@ -211,16 +216,15 @@ function repaint() {
model.doc_dirty,
model.doc_stored_as,
);
paintPages(model);
el.grid.replaceChildren(gridTable(model));
// The flag is the client's answer, not the page's: `C` toggles it and the
// TUI draws the panel on exactly the same condition (improvise-wsaw).
el.categories.hidden = !model.category_panel_open;
el.categories.replaceChildren(categoryList(model));
paintTiles(model);
paintPanel(el.views, model.views, viewRow, (i) => client.click_view(i));
paintPanel(el.formulas, model.formulas, formulaRow, (i) =>
client.click_formula(i),
);
paintPanel(el.views, model.views, viewRow);
paintPanel(el.formulas, model.formulas, formulaRow);
// The selection must be on screen. `#grid` scrolls on its own — ROW_PX and
// COL_PX under-estimate and group rows are uncharged — so a selection the
@@ -286,6 +290,27 @@ function paintHelp(model) {
el.help.replaceChildren(tabs, body);
}
/** The page bar: which item each fixed category is fixed to. Without it a
* grid filtered to Q1 says nothing at all, and `[`/`]` move it in silence
* (improvise-p7lh). An empty list is what says there are none. */
function paintPages(model) {
const entries = pageEntries(model);
el.pages.hidden = entries.length === 0;
el.pages.replaceChildren(
...entries.map((entry) => {
const span = document.createElement("span");
span.className = "page-axis";
// Only a Page axis is navigable; a Filter one constrains without being
// something `[`/`]` can move (gotcha 27).
span.classList.toggle("navigable", entry.navigable);
span.textContent = entry.position
? `${entry.text} (${entry.position})`
: entry.text;
return span;
}),
);
}
/** The grid, painted from a plan in which every decision is already made
* (improvise-anr9, and `shell.js::gridPlan`). Nothing below decides anything:
* which labels repeat and are therefore blank, how many header levels there
@@ -352,20 +377,29 @@ function gridTable(model) {
for (const [name, on] of Object.entries(cell.classes)) {
td.classList.toggle(name, on);
}
// Every cell moves the selection; an editable one also opens its editor,
// which is the pointer's `i`. Not repainted in that branch: `edit` has
// set `editing`, and a repaint would take the <input> away.
// On MOUSEDOWN, and defaulted away: from an editing state the input's
// blur repainted first and this <td> was gone before `click` could fire
// (improvise-0x9u). See context/code/clients.md.
td.addEventListener("mousedown", (event) => {
event.preventDefault();
if (editing) editing.close(false);
const frame = client.click_cell(cell.row, cell.col);
if (frame) transport.send(frame);
if (cell.editable) edit(td, cell.row, cell.col, cell.editText);
else repaint();
});
// The plan's own answers, CARRIED rather than re-derived: `row` counts
// data rows, which walking the table back up would get wrong (b8ot).
Object.assign(td.dataset, cellAttrs(cell));
}
}
// The totals strip, PROJECTED: the sum runs over every row including the
// ones this viewport scrolled past, so nothing here adds anything up.
if (plan.totals) {
const foot = table.createTFoot();
const tr = foot.insertRow();
const label = document.createElement("th");
label.scope = "row";
label.colSpan = plan.totals.span;
label.textContent = plan.totals.label;
tr.appendChild(label);
for (const cell of plan.totals.cells) {
const td = tr.insertCell();
td.textContent = cell.text;
for (const [name, on] of Object.entries(cell.classes)) {
td.classList.toggle(name, on);
}
td.classList.add("total");
}
}
return table;
@@ -411,6 +445,8 @@ function edit(td, row, col, seed) {
};
editing = { row, col, close };
// These two stay DIRECT while the five surfaces above are delegated: one
// transient node, one at a time, and `blur` does not bubble (improvise-b8ot).
input.addEventListener("keydown", (e) => {
if (e.key === "Enter") dismiss(true);
else if (e.key === "Escape") dismiss(false);
@@ -428,12 +464,12 @@ function edit(td, row, col, seed) {
/** A side panel. An empty row list is what says it is closed — the page asks
* no question about the mode, exactly as `paintHelp` does not. */
function paintPanel(host, rows, build, clicked) {
function paintPanel(host, rows, build) {
host.hidden = rows.length === 0;
const list = document.createElement("ul");
rows.forEach((row, index) => {
const li = build(row);
li.addEventListener("click", () => send(clicked(index)));
li.dataset.index = String(index);
list.appendChild(li);
});
host.replaceChildren(list);
@@ -473,7 +509,7 @@ function paintTiles(model) {
span.className = "tile";
span.classList.toggle("selected", tile.selected);
span.textContent = `${tile.category} ${tile.axis}`;
span.addEventListener("click", () => send(client.click_tile(index)));
span.dataset.index = String(index);
return span;
});
el.tiles.replaceChildren(...tiles);
@@ -490,7 +526,7 @@ function categoryList(model) {
li.textContent = row.is_category
? `${indicator(!row.expanded)} ${row.label} (${row.item_count}) ${row.axis}`
: row.label;
li.addEventListener("click", () => send(client.click_category(index)));
li.dataset.index = String(index);
list.appendChild(li);
});
return list;
@@ -503,6 +539,48 @@ function send(frame) {
repaint();
}
// The pointer. ONE handler per surface, on a root OUTSIDE what is redrawn:
// `replaceChildren` replaces an element's children, so these outlive every
// repaint (user ruling, improvise-b8ot). See context/code/clients.md.
/** A surface whose rows are named by one index. */
function delegateIndex(root, selector, act) {
root.addEventListener("click", (event) => {
const node = event.target.closest(selector);
if (!node || !root.contains(node)) return;
const index = indexAction(node.dataset);
if (index !== null) send(act(index));
});
}
delegateIndex(el.categories, "li[data-index]", (i) => client.click_category(i));
delegateIndex(el.views, "li[data-index]", (i) => client.click_view(i));
delegateIndex(el.formulas, "li[data-index]", (i) => client.click_formula(i));
delegateIndex(el.tiles, "span[data-index]", (i) => client.click_tile(i));
// MOUSEDOWN, not click, and deliberately: from an editing state the input's
// blur repaints first and the <td> is gone before `click` could fire
// (improvise-0x9u). Delegation does not change that — the target is detached.
el.grid.addEventListener("mousedown", (event) => {
const td = event.target.closest("td[data-row]");
// Defaulted away only once this really is a cell: a header had no listener
// before, so preventing its default would be a new behaviour.
if (!td || !el.grid.contains(td)) return;
// `editable` is the class the plan already set; the coordinates are the
// attributes it already wrote. Nothing here walks the table.
const action = cellAction(td.dataset, td.classList.contains("editable"));
if (!action) return;
event.preventDefault();
// The live editor closes FIRST, before the click is composed. Order matters.
if (editing) editing.close(false);
const frame = client.click_cell(action.row, action.col);
if (frame) transport.send(frame);
// Every cell moves the selection; an editable one also opens its editor.
// NOT repainted in that branch: a repaint would take the <input> away.
if (action.editable) edit(td, action.row, action.col, action.editText);
else repaint();
});
// The keyboard. `Client::on_key_event` folds locally where it can; what it
// cannot answer is sent and does nothing, by design (context/code/clients.md).
web/index.html
+7
View File
@@ -77,6 +77,13 @@
<div id="tiles" aria-label="Tiles"></div>
<!--
The page bar: which item each Page or Filter category is fixed to. The
TUI draws it inside the grid's border; here it is a line of its own,
above #grid, so it is not charged to the grid's own frame cost.
-->
<div id="pages" aria-label="Page axes" hidden></div>
<main>
<aside id="categories" aria-label="Categories"></aside>
<aside id="formulas" aria-label="Formulas" hidden></aside>
web/shell.js
+66 -2
View File
@@ -51,11 +51,30 @@ export function extentOf(box, frameCost, metrics = { rowPx: ROW_PX, colPx: COL_P
* model, never predicted. Inline group ROWS are deliberately not counted. */
export function frameCostOf(model) {
return {
rows: model.col_levels.length + (model.col_groups.length > 0 ? 1 : 0),
// The totals strip is chrome on the same terms as a header level: always
// there when there are totals, and never a data row (improvise-p7lh).
rows:
model.col_levels.length +
(model.col_groups.length > 0 ? 1 : 0) +
(model.totals.length > 0 ? 1 : 0),
cols: model.row_levels,
};
}
/** The page bar: one entry per category fixing which records are visible. The
* Page/Filter split is `cycle`, decided in Rust — gotcha 27, clients.md. */
export function pageEntries(model) {
return model.page_coords.map((axis) => ({
// The TUI's own spelling (`src/ui/grid.rs`); only the spelling is local,
// exactly as `indicator`'s is.
text: `${axis.cat} = ${axis.selected}`,
// `cycle` is null for a Filter axis, which constrains without being
// navigable: there is no position to show and no key that would move it.
position: axis.cycle ? `${axis.cycle.at + 1}/${axis.cycle.count}` : "",
navigable: Boolean(axis.cycle),
}));
}
/** The grid, as a description: every decision made, nothing created. The page
* decides nothing here and neither does this — which labels repeat, how many
* header levels there are and which columns share a group all arrive
@@ -109,7 +128,52 @@ export function gridPlan(model) {
};
});
return { cornerSpan, groups, levels: model.col_levels, body };
// An EMPTY strip means this grid has no totals — not a row of blanks, and
// not "not caught up yet" (gotcha 33). So `null`, not an empty row.
const totals =
model.totals.length === 0
? null
: {
span: cornerSpan,
label: "Total",
cells: model.totals.map((cell) => ({
text: cell.text,
classes: { numeric: cell.numeric, error: cell.error },
})),
};
return { cornerSpan, groups, levels: model.col_levels, body, totals };
}
/** What a data cell carries on its node, so one delegated handler can read
* back the answers `gridPlan` already made (improvise-b8ot, clients.md). */
export function cellAttrs(cell) {
return { row: String(cell.row), col: String(cell.col), edit: cell.editText };
}
/** …and the inverse: what a delegated mousedown MEANS. `null` where the event
* did not land on a data cell, which is every header and the corner. */
export function cellAction(dataset, editable) {
const row = whole(dataset.row);
const col = whole(dataset.col);
// A node that carries no coordinates is not a cell: every header and the
// corner reach here, and none of them means anything.
if (row === null || col === null) return null;
return { row, col, editable, editText: dataset.edit ?? "" };
}
/** The same, for the surfaces whose rows are named by one index: the panels,
* the tile bar and the category tree. `null` where there is no row. */
export function indexAction(dataset) {
return whole(dataset.index);
}
/** A non-negative whole number off an attribute, or `null`. An ABSENT
* attribute and the string `""` are both "no index", and `Number` says 0. */
function whole(text) {
if (typeof text !== "string" || text.trim() === "") return null;
const value = Number(text);
return Number.isInteger(value) && value >= 0 ? value : null;
}
/** What the worker just posted. A protocol frame is a STRING and a shell
web/style.css
+35
View File
@@ -120,6 +120,41 @@ main {
font-size: 0.85em;
}
/* The page bar. Magenta in the TUI; the FACT that an axis is navigable is
`cycle`, decided in Rust, and only this weight is the page's. */
#pages {
display: flex;
gap: 0.5rem;
padding: 0.25rem 0.5rem;
border-bottom: 1px solid var(--line);
overflow-x: auto;
white-space: nowrap;
font-size: 0.85em;
}
#pages .page-axis {
padding: 0 0.4rem;
border: 1px solid var(--line);
border-radius: 3px;
opacity: 0.7;
}
#pages .navigable {
color: var(--accent, inherit);
opacity: 1;
}
/* The totals strip. Never summed here — it is projected, because the sum runs
over rows this viewport scrolled past. */
tfoot th,
td.total {
font-weight: 700;
border-top: 2px solid var(--line);
/* `td:not(.editable)` dims every unsettable cell; a total is unsettable and
is not incidental, so it opts back out. */
opacity: 1;
}
#categories .item {
padding-left: 1.5rem;
opacity: 0.85;
web/tests/dispatch.test.js
+108
View File
@@ -0,0 +1,108 @@
// What a node in a delegated surface MEANS, and what the page bar reads as.
// Both are decisions, so neither needs a browser — context/code/clients.md.
import { test } from "node:test";
import assert from "node:assert/strict";
import { cellAction, cellAttrs, gridPlan, indexAction, pageEntries } from "../shell.js";
const cell = (text, extra = {}) => ({
text,
edit_text: text,
numeric: true,
error: false,
selected: false,
editable: true,
...extra,
});
/// The shape the row index has to survive: a group header between data rows,
/// which is exactly what walking the table back up would get wrong.
function grouped() {
return {
row_levels: 1,
col_levels: [["Revenue", "Cost"]],
col_groups: [],
totals: [],
editing: null,
rows: [
{ kind: "group", label: "Hot", collapsed: false },
{ kind: "data", labels: ["Espresso"], cells: [cell("1"), cell("2")] },
{ kind: "group", label: "Cold", collapsed: true },
{
kind: "data",
labels: ["Cold Brew"],
cells: [cell("3", { editable: false }), cell("4", { edit_text: "4000" })],
},
],
};
}
test("a cell's attributes carry the plan's answers back to the handler", () => {
const plan = gridPlan(grouped());
const last = plan.body.at(-1).cells[1];
const action = cellAction(cellAttrs(last), last.editable);
assert.deepEqual(action, { row: 1, col: 1, editable: true, editText: "4000" });
});
// improvise-b8ot: the DATA row index is what a click sends, and `gridPlan`
// counts it because a group header is not a data row. A delegated handler
// that inferred the row from the node's position in the table would say 3.
test("the row a delegated click means is the data index, not the table row", () => {
const plan = gridPlan(grouped());
const rows = plan.body
.filter((r) => r.kind === "data")
.map((r) => cellAction(cellAttrs(r.cells[0]), r.cells[0].editable).row);
assert.deepEqual(rows, [0, 1]);
});
test("editable is the class, so an unsettable cell opens no editor", () => {
const plan = gridPlan(grouped());
const aggregate = plan.body.at(-1).cells[0];
assert.equal(aggregate.classes.editable, false);
assert.equal(cellAction(cellAttrs(aggregate), false).editable, false);
});
// Every header, the corner and the totals strip reach the delegated handler
// too. None of them is a cell, and `Number("")` is 0 rather than nothing.
test("a node carrying no coordinates means nothing at all", () => {
assert.equal(cellAction({}, false), null);
assert.equal(cellAction({ row: "0" }, false), null);
assert.equal(cellAction({ row: "", col: "" }, false), null);
assert.equal(cellAction({ row: "x", col: "0" }, false), null);
assert.equal(cellAction({ row: "1.5", col: "0" }, false), null);
});
test("an index surface reads its row back, and nothing else", () => {
assert.equal(indexAction({ index: "0" }), 0);
assert.equal(indexAction({ index: "12" }), 12);
assert.equal(indexAction({}), null);
assert.equal(indexAction({ index: "" }), null);
assert.equal(indexAction({ index: "-1" }), null);
});
// improvise-p7lh: a grid filtered to Q1 said nothing at all, and `[`/`]`
// moved it in silence. The bar is what says which quarter this is.
test("the page bar names the category and the item it is fixed to", () => {
const [entry] = pageEntries({
page_coords: [{ cat: "Quarter", selected: "Q2", cycle: { at: 1, count: 4 } }],
});
assert.equal(entry.text, "Quarter = Q2");
assert.equal(entry.position, "2/4", "one-based, as a user counts");
assert.equal(entry.navigable, true);
});
// Gotcha 27: Page is navigable and Filter is not. A Filter axis offers no
// position, because there is no key that would move it.
test("a filter axis is shown, and is not offered as something to cycle", () => {
const [entry] = pageEntries({
page_coords: [{ cat: "Region", selected: "North", cycle: null }],
});
assert.equal(entry.text, "Region = North");
assert.equal(entry.position, "");
assert.equal(entry.navigable, false);
});
test("no fixed categories is an empty bar, not a blank one", () => {
assert.deepEqual(pageEntries({ page_coords: [] }), []);
});
web/tests/grid_plan.test.js
+27
View File
@@ -23,6 +23,7 @@ function grouped() {
row_levels: 2,
col_levels: [["Q1", ""], ["Revenue", "Cost"]],
col_groups: [{ span: 2, label: "2026", collapsed: false }],
totals: [],
editing: null,
rows: [
{ kind: "group", label: "Hot", collapsed: false },
@@ -108,3 +109,29 @@ test("the editor is seeded from the stored value, not the drawn one", () => {
assert.equal(plan.body[1].cells[0].text, "20,000");
assert.equal(plan.body[1].cells[0].editText, "20000");
});
test("an empty totals strip is no strip at all, not a row of blanks", () => {
// Gotcha 33: an EMPTY strip means this grid has no totals — records mode,
// or an empty grid — so there is nothing to draw and nothing to pad.
assert.equal(gridPlan(grouped()).totals, null);
});
test("the totals strip is relayed, and its label spans the row headers", () => {
const model = grouped();
model.totals = [
{ text: "9,001", numeric: true, error: false },
{ text: "err", numeric: false, error: true },
];
const plan = gridPlan(model);
assert.equal(plan.totals.span, 2, "the label spans every row-header level");
assert.equal(plan.totals.label, "Total");
assert.deepEqual(
plan.totals.cells.map((c) => [c.text, c.classes.numeric, c.classes.error]),
[
["9,001", true, false],
["err", false, true],
],
);
// And it carries no coordinates: a total is not a cell a click can name.
assert.ok(!("row" in plan.totals.cells[0]));
});
web/tests/render_surface.test.js
+61
View File
@@ -44,6 +44,67 @@ test("every field the render model projects is painted, or named as debt", () =>
);
});
/// One level up, and the direction that would have caught improvise-p7lh:
/// the check above is closed under what `RenderModel` chose to expose, so a
/// cache field that never reaches it is invisible to it. Reason per entry.
const UNREAD_CACHE = {
"grid.col_widths":
"the browser lets the table size its own columns; improvise-pcje is the parity residue",
};
/// `ClientState`'s cache fields, and the `pub` fields of each cache type.
function cacheFields() {
const client = read("../../crates/improvise-protocol/src/client.rs");
const render = read("../../crates/improvise-protocol/src/render.rs");
const state = client.slice(
client.indexOf("pub struct ClientState {"),
client.indexOf("\n}", client.indexOf("pub struct ClientState {")),
);
assert.ok(state.length > 0, "ClientState moved — this test reads the struct itself");
const caches = [...state.matchAll(/\n {4}pub ([a-z_0-9]+): ([A-Za-z]*Cache)\b/g)];
assert.ok(caches.length >= 5, `found only ${caches.length} widget caches`);
return caches.flatMap(([, field, type]) => {
const start = render.indexOf(`pub struct ${type} {`);
if (start < 0) return []; // Defined elsewhere (BasisCache): not a drawn widget.
const body = render.slice(start, render.indexOf("\n}", start));
return [...body.matchAll(/\n {4}pub ([a-z_0-9]+):/g)].map((m) => `${field}.${m[1]}`);
});
}
// improvise-p7lh: `GridCache::totals` and `page_coords` were projected, folded
// and correctly maintained, and no front end read either for months.
test("every field of every widget cache reaches the browser, or is named as debt", () => {
// The browser's whole decision surface: what it renders, what it resolves a
// key against, and the fold. A field none of these names is drawn by nothing.
// Whitespace-stripped, because rustfmt breaks `state.grid.totals` across
// three lines and a substring search would then read it as unread.
const client = [
"../../crates/improvise-wasm-client/src/render.rs",
"../../crates/improvise-wasm-client/src/lib.rs",
"../../crates/improvise-wasm-client/src/socket.rs",
"../../crates/improvise-protocol/src/resolve.rs",
]
.map((rel) => code(rel).replace(/\s+/g, ""))
.join("\n");
const unread = cacheFields().filter((path) => {
const [cache, field] = path.split(".");
// Either the field itself is read, or the WHOLE cache is handed to a
// shared derivation — which is a real hole, named in the evidence file.
if (client.includes(`state.${cache}.${field}`)) return false;
if (field === "cells" && client.includes(`state.${cache}.cell(`)) return false;
// `&state.grid.row_frame` CONTAINS `&state.grid`, so a plain substring
// test here makes every GridCache field read as handed over. Demonstrated.
return !new RegExp(`&state\\.${cache}(?![.\\w])`).test(client);
});
assert.deepEqual(
unread.sort(),
Object.keys(UNREAD_CACHE).sort(),
"a maintained cache field reaches no front end — read it, or add it to UNREAD_CACHE with the reason",
);
});
test("every element the page looks up exists in the page", () => {
const app = read("../app.js");
const html = read("../index.html");
web/tests/shell.test.js
+8 -1
View File
@@ -147,10 +147,11 @@ test("a window smaller than its own frame still asks for one cell", () => {
});
test("the frame costs a line per column level, plus one for a group line", () => {
const model = (levels, groups, rowLevels) => ({
const model = (levels, groups, rowLevels, totals = []) => ({
col_levels: levels,
col_groups: groups,
row_levels: rowLevels,
totals,
});
assert.deepEqual(frameCostOf(model([["a"]], [], 1)), { rows: 1, cols: 1 });
// Two category levels on the column axis, and a group line above them.
@@ -158,6 +159,12 @@ test("the frame costs a line per column level, plus one for a group line", () =>
rows: 3,
cols: 2,
});
// And the totals strip, which is chrome on the same terms as a header
// level: always there when there are totals, never a data row.
assert.deepEqual(frameCostOf(model([["a"]], [], 1, [{ text: "9" }])), {
rows: 2,
cols: 1,
});
});
test("the collapse glyph points right when collapsed", () => {