diff --git a/context/code/clients.md b/context/code/clients.md
index c4d3a2f4..1c606de6 100644
--- a/context/code/clients.md
+++ b/context/code/clients.md
@@ -1112,7 +1112,7 @@ reaching it from https means terminating TLS in front of it). From an https page
an explicit `ws://` is **upgraded** rather than passed through, since passing it
through can only produce a failure the user cannot act on.
-### `set_viewport`, not `Client.viewport`
+### `set_viewport`, and the twin that is gone
Same frame, but it also **records** the window. A client learns its window only
from what it announced — the row frame's own length is the window clipped to the
@@ -1121,6 +1121,23 @@ arithmetic matters. Without it the client cannot clamp, so every cursor key goes
upstream into `Host::handle`'s empty vec and the arrow keys silently do nothing
(improvise-bpq2).
+There used to be a frame-only twin, `Client::viewport`, exported beside it and
+called by nobody. Its doc said it was kept because `web/app.js` already spelled
+it — and `web/app.js` called `set_viewport` in the same wave, so the reason was
+false before it was read (improvise-seik). That matters more than an ordinary
+unused function, because **the two are one letter apart and the wrong one fails
+silently**: it sends an identical frame and simply forgets the window, which
+turns every cursor key into the black hole above. It is deleted, along with the
+exported `init()` free function that duplicated `new Client()`.
+
+A deletion is not the guard, though — the next export with no caller arrives the
+same way. `web/tests/bundle/wasm_surface.test.js` now reads the generated
+`.d.ts` and requires every declared method to be one `app.js` calls, or to be
+named in an `UNCALLED` table with its reason. `free` and `initSync` are
+wasm-bindgen's own; `revision` is there because `on_frame` returns it. Proved by
+injecting a spare `static viewport(...)` declaration into the built `.d.ts` and
+watching it fail by name.
+
### `frameCost`: what the grid's own frame costs, in whole cells
One header row and one header column was right until improvise-anr9: the column
@@ -1216,6 +1233,46 @@ Neither arm leaves one armed.
1200 reads `"1,200"` and committing that stores the *string* `"1,200"`. Enter
removes the focused ``, which fires `blur`, which calls `close` again:
harmless, but only by accident.
+- **The title bar is three fields and one joining.** `doc_name`, `doc_dirty` and
+ `doc_stored_as` come off the cache `src/draw.rs::draw_title` folds; the joining
+ — `name (stored_as) [+]`, in that order — is `shell.js::documentTitle`, which
+ is the TUI's own spelling minus the `improvise ·` chrome and the right-hand
+ key strip. The three were projected by improvise-z754 and painted by nothing
+ for a wave, and `render_surface.test.js` could not see it: that check is a
+ **substring search**, so a field named by a function nothing calls reads as
+ painted. What sees it is `contract.spec.js`, which loads the page and asserts
+ the text (improvise-5jna). The marker itself is a node test rather than a
+ harness one, because the standalone deployment persists on every accepted
+ batch and clears the flag in the same round trip, so `[+]` is transient there.
+
+### The pointer takes cells on `mousedown`
+
+A cell's handler is `mousedown` with `preventDefault()`, not `click`, and the
+reason is an ordering trap worth the paragraph (improvise-0x9u). From an
+**editing** state a `click` never arrived at all: the mousedown moved focus, the
+``'s `blur` ran `close`, `close` repainted, `replaceChildren` destroyed
+the `
` the mousedown had landed on, and no `click` was ever dispatched. The
+pointer was dead until the user pressed Escape or clicked outside the grid.
+
+Three things had to move together, and each is load-bearing:
+
+- **`mousedown`, so the handler runs before any focus change.**
+- **`preventDefault()`, so no focus change happens at all.** Otherwise the
+ default action focuses `#grid` *after* the handler has called `input.focus()`
+ on the new editor, and blurs it immediately. The keyboard listener is on
+ `window`, so nothing needs `#grid` focused.
+- **`close` no longer repaints, and `blur` checks `closed` first.** Removing a
+ focused `` fires `blur` **synchronously**, so a `close` that repainted
+ from inside itself detached the very `
` the handler was about to open an
+ editor on. This is the same repair one level down, and it is why the first
+ attempt at the fix still failed: the `blur` arm re-entered and rebuilt the
+ table. `close` restores the **drawn** text rather than the seed, since nothing
+ repaints behind it any more.
+
+The state the old spec could not see is the whole point: it pressed Escape
+before clicking the second cell, so it only ever clicked from a non-editing
+page — this repo's characteristic defect, a test that passes while the thing it
+names is broken in a neighbouring state.
### The keyboard
diff --git a/context/test-evidence/clients.md b/context/test-evidence/clients.md
index 8ab53306..9f32691b 100644
--- a/context/test-evidence/clients.md
+++ b/context/test-evidence/clients.md
@@ -2062,3 +2062,41 @@ done
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.
+
+#### "a click moves the selection from an editing state too"
+
+improvise-0x9u, and the fault was injected by running this assertion against the
+bundle as it stood: `cells.nth(4).locator("input")` came back **0**, with the
+selection still on cell 0, exactly as the bug report measured in Chromium.
+
+Two things about it are worth more than the assertion. First, **what it does not
+do**: it never presses Escape between the two clicks. The sibling test does, and
+that single line is why the property it names ("a click moves the selection") was
+asserted for a whole wave while the neighbouring state was broken — the click
+path from an *editing* page was never exercised.
+
+Second, **the fix's own first attempt failed this same test**, which is the part
+to remember. Moving the handler to `mousedown` is necessary and not sufficient:
+`close` still repainted, and removing a focused `` fires `blur`
+**synchronously**, so `close` rebuilt the table from inside itself and detached
+the `
` the handler was about to open an editor on. The assertion could not
+tell that apart from the original bug — same symptom, different cause — so it is
+the debug run rather than the test that separated them. `close` therefore no
+longer repaints, and the `blur` arm checks `closed` before dismissing.
+
+#### "the widgets the demo document fills are drawn, in the state that fills them"
+
+The `#doc-name` row is improvise-5jna, and the reason it belongs in a **browser**
+test rather than in `render_surface.test.js` is that the latter is a substring
+search over the page's source: `doc_name` mentioned by a function nothing calls
+reads as painted. That is how `model.minibuffer` once shipped dead, and how
+z754's three title-bar fields sat projected and unpainted for a wave while the
+static check was green. Proved by adding the assertion first: `element(s) not
+found` for `#doc-name`.
+
+The `[+]` marker is deliberately **not** asserted here. The standalone deployment
+persists on every accepted batch and `effect::Save` clears the flag in the same
+round trip (gotcha 45), so the marker is transient in a browser and any timing
+assertion over it would be flaky rather than discriminating. Its spelling — the
+order `name (stored_as) [+]`, the `null` `stored_as` writing no empty parens — is
+`web/tests/shell.test.js`, over `documentTitle` directly.
diff --git a/crates/improvise-wasm-client/src/lib.rs b/crates/improvise-wasm-client/src/lib.rs
index fe4601c8..31f654ce 100644
--- a/crates/improvise-wasm-client/src/lib.rs
+++ b/crates/improvise-wasm-client/src/lib.rs
@@ -19,9 +19,8 @@ pub struct Client {
state: ClientState,
}
-/// Allocate a client: an empty view slice and one empty cache per widget. A
-/// client that draws before its bootstrap draws an empty grid, correctly.
-#[wasm_bindgen]
+/// An empty view slice and one empty cache per widget. Not exported: the
+/// page's door is `new Client()` (improvise-seik).
pub fn init() -> Client {
Client::default()
}
@@ -55,14 +54,8 @@ impl Client {
serde_json::to_string(&message).ok()
}
- /// The whole client state as JSON: the view slice, every widget cache, and
- /// the revision it corresponds to.
- #[wasm_bindgen]
- pub fn render_state(&self) -> Result {
- self.state_json().map_err(|e| JsError::new(&e.to_string()))
- }
-
- /// The cheap half of [`Client::render_state`]: has anything moved?
+ /// Has anything moved? The cheap question, and the one `on_frame` already
+ /// answers by returning it.
#[wasm_bindgen]
pub fn revision(&self) -> u64 {
self.state.revision
@@ -71,24 +64,17 @@ impl Client {
// The transport surface: a handful of string calls and the whole harness a
// page needs. See `context/code/clients.md`.
- /// The handshake frame, associated rather than a method. **Prefer
- /// [`Client::set_viewport`]** — this one forgets the window (clients.md).
+ /// The handshake frame, *and* the record of what this client can show.
+ /// The **only** door, and why that matters: `context/code/clients.md`.
#[wasm_bindgen]
- pub fn viewport(rows: usize, cols: usize) -> String {
+ pub fn set_viewport(&mut self, rows: usize, cols: usize) -> String {
+ self.state.window = (rows, cols);
serde_json::to_string(&Self::viewport_frame(rows, cols))
// Two `usize` in a unit variant; unreachable, and an empty frame
// is skipped by the server rather than crashing the tab.
.unwrap_or_default()
}
- /// The handshake frame, *and* the record of what this client can show —
- /// which is not derivable from anything the host sends (clients.md).
- #[wasm_bindgen]
- pub fn set_viewport(&mut self, rows: usize, cols: usize) -> String {
- self.state.window = (rows, cols);
- Self::viewport(rows, cols)
- }
-
/// Fold one frame, in arrival order, one at a time; returns the new
/// revision. Never reorder or coalesce — a split batch loses cells.
#[wasm_bindgen]
@@ -316,12 +302,15 @@ mod tests {
);
}
- /// The handshake frame is the same either way; `set_viewport` also keeps
- /// the window, which is what lets a cursor key resolve at all.
+ /// The handshake frame, and the window it records — one call, because a
+ /// second door that sent the frame and forgot the window was silent.
#[test]
- fn set_viewport_sends_the_same_frame_and_keeps_the_window() {
+ fn set_viewport_sends_the_handshake_frame_and_keeps_the_window() {
let mut client = init();
- assert_eq!(client.set_viewport(24, 8), Client::viewport(24, 8));
+ assert_eq!(
+ serde_json::from_str::(&client.set_viewport(24, 8)).unwrap(),
+ Client::viewport_frame(24, 8)
+ );
assert_eq!(client.state().window, (24, 8));
}
diff --git a/crates/improvise-wasm-client/tests/browser_round_trip.rs b/crates/improvise-wasm-client/tests/browser_round_trip.rs
index 0fb65264..776f2501 100644
--- a/crates/improvise-wasm-client/tests/browser_round_trip.rs
+++ b/crates/improvise-wasm-client/tests/browser_round_trip.rs
@@ -98,10 +98,10 @@ fn a_browser_client_seeds_types_a_number_and_sees_it_come_back() {
// The handshake. The extent comes first because the snapshot seeds the
// *visible* cells: one built at extent zero is an empty grid.
- let handshake: ClientFrame = serde_json::from_str(&Client::viewport(10, 10))
+ let handshake: ClientFrame = serde_json::from_str(&client.set_viewport(10, 10))
.expect("the viewport frame must be one the server reads");
let ClientFrame::Viewport { rows, cols } = handshake else {
- panic!("Client::viewport must produce a Viewport frame");
+ panic!("set_viewport must produce a Viewport frame");
};
let (id, snapshot) = host.connect(rows, cols);
deliver(&mut client, &[snapshot]);
@@ -305,7 +305,7 @@ fn a_resize_reprojects_the_grid_at_the_new_extent() {
"a one-row viewport is seeded with one row"
);
- let frames = send(&mut host, id, &Client::viewport(10, 10));
+ let frames = send(&mut host, id, &client.set_viewport(10, 10));
deliver(&mut client, &frames);
assert_eq!(
client.model().rows.len(),
diff --git a/harness/tests/contract.spec.js b/harness/tests/contract.spec.js
index 74093787..2c3394cc 100644
--- a/harness/tests/contract.spec.js
+++ b/harness/tests/contract.spec.js
@@ -26,6 +26,15 @@ test.describe("every field the render model projects reaches the screen", () =>
await expect(page.locator("#grid tbody th").first()).toHaveText("Espresso");
await expect(page.locator("#grid tbody td").first()).not.toHaveText("");
+ // `doc_name`, `doc_stored_as`: the title bar. improvise-z754 projected
+ // these and nothing painted them for a wave, which `render_surface`'s
+ // substring search cannot see — only a loaded page can (improvise-5jna).
+ // The dirty marker's own spelling is `web/tests/shell.test.js`: this
+ // deployment persists on every accepted batch, so `[+]` is transient.
+ await expect(page.locator("#doc-name")).toHaveText(
+ "Corner Coffee — 2025 (demo.improv)",
+ );
+
// `mode`, `hint`, `view_name`, `status`: the header's four facts.
await expect(page.locator("#mode")).toHaveText("NORMAL");
await expect(page.locator("#hint")).not.toHaveText("");
@@ -212,4 +221,25 @@ test.describe("a pointer event reaches the client and its answer is painted", ()
await expect(page.locator("#mode")).toHaveText("FORMULAS");
await expect(page.locator("#formulas li.selected")).toHaveCount(1);
});
+
+ // improvise-0x9u. The neighbouring state the assertion above cannot see: it
+ // presses Escape first, so it only ever clicks from a NON-editing page.
+ test("a click moves the selection from an editing state too", async ({ page }) => {
+ await openApp(page);
+ const cells = page.locator("#grid tbody td");
+
+ await cells.nth(0).click();
+ await expect(cells.nth(0).locator("input")).toHaveCount(1);
+
+ // No Escape between the two clicks — that is the whole test. The blur used
+ // to repaint and `replaceChildren` destroyed the
the mousedown landed
+ // on, so no click event was ever dispatched and the pointer was dead.
+ await cells.nth(4).click();
+ await expect(cells.nth(4).locator("input")).toHaveCount(1);
+ await expect(cells.nth(0).locator("input")).toHaveCount(0);
+
+ await page.keyboard.press("Escape");
+ await expect(cells.nth(4)).toHaveClass(/selected/);
+ await expect(page.locator("#grid tbody td.selected")).toHaveCount(1);
+ });
});
diff --git a/web/app.js b/web/app.js
index 9c496209..b435d56d 100644
--- a/web/app.js
+++ b/web/app.js
@@ -7,6 +7,7 @@ import {
HANDSHAKE_TIMEOUT_MS,
ROW_PX,
createShell,
+ documentTitle,
extentOf,
frameCostOf,
gridPlan,
@@ -35,6 +36,7 @@ const el = {
help: document.getElementById("help"),
status: document.getElementById("status"),
viewName: document.getElementById("view-name"),
+ docName: document.getElementById("doc-name"),
};
await init();
@@ -60,8 +62,8 @@ function sendViewport() {
const box = el.grid.getBoundingClientRect();
const metrics = { rowPx: ROW_PX, colPx: COL_PX };
const { rows, cols } = extentOf(box, frameCost, metrics);
- // `client.set_viewport`, NOT the associated `Client.viewport`: the same
- // frame, but it also RECORDS the window. Why that matters: clients.md.
+ // `set_viewport` RECORDS the window as well as framing it; the frame-only
+ // twin beside it is deleted, so there is nothing to call by mistake (seik).
transport.send(client.set_viewport(rows, cols));
}
@@ -162,6 +164,13 @@ function repaint() {
paintMinibuffer(model);
shell.setModel(model.status);
el.viewName.textContent = model.view_name;
+ // The title bar, off the cache `src/draw.rs::draw_title` folds — one
+ // derivation, both front ends (improvise-z754, improvise-5jna).
+ el.docName.textContent = documentTitle(
+ model.doc_name,
+ model.doc_dirty,
+ model.doc_stored_as,
+ );
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).
@@ -299,7 +308,12 @@ function gridTable(model) {
// 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 away.
- td.addEventListener("click", () => {
+ // On MOUSEDOWN, and defaulted away: from an editing state the input's
+ // blur repainted first and this
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);
@@ -314,10 +328,12 @@ function gridTable(model) {
* back is the server's, never the string typed here. */
function edit(td, row, col, seed) {
if (editing) return;
- editing = { row, col };
// `seed` is `RenderCell::edit_text` -- the STORED value, not the drawn one:
// `td.textContent` has been through the number format.
const original = seed;
+ // What was on the screen, kept so closing can put it back: `close` no longer
+ // always repaints, so restoring `seed` would leave `1200` where `1,200` was.
+ const drawn = td.textContent;
const input = document.createElement("input");
input.value = original;
td.replaceChildren(input);
@@ -327,6 +343,8 @@ function edit(td, row, col, seed) {
// Enter removes the focused , which fires `blur`, which calls
// `close` again. Harmless before, but only by accident.
let closed = false;
+ // `close` does NOT repaint: a caller about to open another editor on a live
+ //
must not have the table rebuilt under it (improvise-0x9u).
const close = (commit) => {
if (closed) return;
closed = true;
@@ -337,19 +355,28 @@ function edit(td, row, col, seed) {
// follows restores whatever the server now says is there.
if (frame) transport.send(frame);
}
- td.replaceChildren(original);
+ td.replaceChildren(drawn);
painted = -1;
+ };
+ const dismiss = (commit) => {
+ close(commit);
repaint();
};
+ editing = { row, col, close };
input.addEventListener("keydown", (e) => {
- if (e.key === "Enter") close(true);
- else if (e.key === "Escape") close(false);
+ if (e.key === "Enter") dismiss(true);
+ else if (e.key === "Escape") dismiss(false);
// The editor owns the keyboard while it is open; the page-level handler
// below must not also see these.
e.stopPropagation();
});
- input.addEventListener("blur", () => close(false));
+ // Removing the focused fires `blur` SYNCHRONOUSLY, so a caller that
+ // already closed this editor must not have the table rebuilt from inside its
+ // own `close` — that is what detached the
in improvise-0x9u's repair.
+ input.addEventListener("blur", () => {
+ if (!closed) dismiss(false);
+ });
}
/** A side panel. An empty row list is what says it is closed — the page asks
diff --git a/web/index.html b/web/index.html
index 8e3dd905..7fd56994 100644
--- a/web/index.html
+++ b/web/index.html
@@ -34,6 +34,7 @@
-->
improvise
+
diff --git a/web/shell.js b/web/shell.js
index 3bbf7508..7100f234 100644
--- a/web/shell.js
+++ b/web/shell.js
@@ -16,6 +16,12 @@ export function composeStatus(shell, model) {
return [shell, model].filter(Boolean).join(" · ");
}
+/** The title bar's one line, in the order and spelling `src/draw.rs::draw_title`
+ * uses. The three facts are the host's; only the joining is here (clients.md). */
+export function documentTitle(name, dirty, storedAs) {
+ return `${name}${storedAs ? ` (${storedAs})` : ""}${dirty ? " [+]" : ""}`;
+}
+
/** The collapse glyph. `collapsed` is the fact, decided by the view and
* carried on the frame; only this spelling is the page's (gotcha 26). */
export function indicator(collapsed) {
diff --git a/web/style.css b/web/style.css
index 03908006..c2055238 100644
--- a/web/style.css
+++ b/web/style.css
@@ -49,6 +49,10 @@ header h1 {
color: var(--accent);
}
+#doc-name {
+ opacity: 0.85;
+}
+
#mode {
color: var(--accent);
}
diff --git a/web/tests/bundle/wasm_surface.test.js b/web/tests/bundle/wasm_surface.test.js
index bde6bdef..f4016167 100644
--- a/web/tests/bundle/wasm_surface.test.js
+++ b/web/tests/bundle/wasm_surface.test.js
@@ -86,6 +86,29 @@ test("the page still reaches the client through a handful of calls", () => {
);
});
+/// Methods on `Client` the page does not call, each with the reason it is
+/// still exported. A door with no caller and no entry here is improvise-seik:
+/// the dangerous kind, because calling the wrong one of a pair is silent.
+const UNCALLED = {
+ free: "wasm-bindgen's own destructor, not ours to remove",
+ initSync: "wasm-bindgen's own module loader, beside the default export",
+ revision: "on_frame returns it; a page that stores it can skip a repaint",
+};
+
+test("the bundle exports no client door the page has no reason to hold", () => {
+ const declared = declarations("pkg/improvise_wasm_client.d.ts");
+ const called = new Set(callsOn("app.js", "client").map((c) => c.name));
+ // `constructor` is `new Client()`, which `callsOn` cannot see as a call.
+ const spare = [...declared.keys()].filter(
+ (name) => name !== "constructor" && !called.has(name),
+ );
+ assert.deepEqual(
+ spare.sort(),
+ Object.keys(UNCALLED).sort(),
+ "an exported method nothing calls — delete it, or add it to UNCALLED with the reason",
+ );
+});
+
test("every worker call web/worker.js makes is declared, with that arity", () => {
const declared = declarations("worker-pkg/improvise_worker_server.d.ts");
const calls = callsOn("worker.js", "server");
diff --git a/web/tests/render_surface.test.js b/web/tests/render_surface.test.js
index aa0766aa..ee9b3d6d 100644
--- a/web/tests/render_surface.test.js
+++ b/web/tests/render_surface.test.js
@@ -17,14 +17,10 @@ const code = (rel) =>
/// Fields the page deliberately does not read, each with the reason. A field
/// belongs here only with an argument; the list is the debt, written down.
-// improvise-z754 put the title bar's facts in one shared cache and taught
-// `src/draw.rs::draw_title` to fold it; this page never learned to. That is
-// improvise-5jna, open, and this list is what stops it being forgotten.
-const UNPAINTED = {
- doc_name: "improvise-5jna: the title bar is drawn by the TUI and not here",
- doc_dirty: "improvise-5jna",
- doc_stored_as: "improvise-5jna",
-};
+// Empty since improvise-5jna painted the title bar. Note what this check CANNOT
+// see, and why the harness asserts the painted text too: it is a substring
+// search, so a field named by a function nothing calls reads as painted.
+const UNPAINTED = {};
/// The `pub` fields of `RenderModel`, read off the Rust that defines it.
function renderModelFields() {
diff --git a/web/tests/shell.test.js b/web/tests/shell.test.js
index 8d229d4a..8cf36b80 100644
--- a/web/tests/shell.test.js
+++ b/web/tests/shell.test.js
@@ -7,6 +7,7 @@ import assert from "node:assert/strict";
import {
composeStatus,
createShell,
+ documentTitle,
extentOf,
frameCostOf,
indicator,
@@ -160,3 +161,17 @@ test("the collapse glyph points right when collapsed", () => {
assert.equal(indicator(true), "▶");
assert.equal(indicator(false), "▼");
});
+
+test("the title bar spells the document the way the TUI's own title does", () => {
+ // `src/draw.rs::draw_title`: name, then what it is stored as in parens, then
+ // the dirty marker. The harness cannot pin `[+]` — the standalone deployment
+ // persists on every accepted batch, so the marker is transient there.
+ assert.equal(
+ documentTitle("Corner Coffee", true, "demo.improv"),
+ "Corner Coffee (demo.improv) [+]",
+ );
+ assert.equal(documentTitle("Corner Coffee", false, "demo.improv"), "Corner Coffee (demo.improv)");
+ // A host with nowhere to write it: `stored_as` is `None` over the wire, which
+ // arrives as `null`, and an empty pair of parens would be a lie about storage.
+ assert.equal(documentTitle("Untitled", true, null), "Untitled [+]");
+});