From 8747074e52cea2a868c561b3f12fa7008f51b8f6 Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Wed, 23 Sep 2026 08:33:40 -0700 Subject: [PATCH] Seed the mouse editor from its target, and mark the target on the first edit (improvise-ugj4, improvise-jibp) Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- crates/improvise-wasm-client/src/lib.rs | 60 ++++++++++++++++++++++ crates/improvise-wasm-client/src/render.rs | 8 --- crates/improvise-wasm-client/src/socket.rs | 32 ++++++++++++ web/app.js | 17 +++--- web/shell.js | 5 +- web/tests/dispatch.test.js | 5 +- web/tests/grid_plan.test.js | 9 ---- 7 files changed, 105 insertions(+), 31 deletions(-) diff --git a/crates/improvise-wasm-client/src/lib.rs b/crates/improvise-wasm-client/src/lib.rs index e02f7cf9..e1cafc33 100644 --- a/crates/improvise-wasm-client/src/lib.rs +++ b/crates/improvise-wasm-client/src/lib.rs @@ -109,6 +109,24 @@ impl Client { .map_err(|e| JsError::new(&e.to_string())) } + /// What the editor opened on `target` shows: the seed the target captured, + /// never the page's paint, which can be stale (improvise-ugj4). + #[wasm_bindgen] + pub fn edit_seed(&self, target: &str) -> Result { + let target: EditTarget = + serde_json::from_str(target).map_err(|e| JsError::new(&e.to_string()))?; + Ok(target.opened.seed) + } + + /// `target` after the user's first edit of its ``, still opaque to + /// the page (improvise-jibp). + #[wasm_bindgen] + pub fn edited_target(&self, target: &str) -> Result { + let target: EditTarget = + serde_json::from_str(target).map_err(|e| JsError::new(&e.to_string()))?; + serde_json::to_string(&target.edited()).map_err(|e| JsError::new(&e.to_string())) + } + /// The frame for "the user typed `text` into the cell `target` names". By /// identity, never position: rows renumber while an editor is open. #[wasm_bindgen] @@ -267,6 +285,48 @@ mod tests { assert_eq!(client.typed_cell_frame(0, 0, "42"), None); } + /// BUG (improvise-ugj4, improvise-jibp) — see `context/test-evidence/clients.md`. + #[test] + fn the_page_opens_its_editor_on_what_the_target_captured() { + let id = improvise_protocol::RecordCell { + record: surrogate("fe4c8e5e9cbd578e0d388f4b2b5a7879"), + measure: Some("Revenue".into()), + }; + let at = |text: &str, stored: &str| { + let mut cell = improvise_protocol::CellDisplay::number(text); + cell.edit_text = Some(stored.into()); + cell.id = Some(CellId::Record(id.clone())); + Projection::Grid(GridUpdate::Cells(vec![((0, 0), cell)])) + }; + let mut client = init(); + client.bootstrap(Snapshot { + view: ViewState::default(), + seed: vec![ + Projection::Grid(GridUpdate::RowFrame(AxisFrame::flat(["North"]))), + Projection::Grid(GridUpdate::ColFrame(AxisFrame::flat(["Revenue"]))), + at("1,200.50", "1200.5"), + Projection::Basis(improvise_protocol::basis::published( + 0, + 0, + [(id.record, 0)].into_iter().collect(), + )), + ], + }); + let first = client.edit_target(0, 0).unwrap().expect("a record cell"); + assert_eq!(client.edit_seed(&first).unwrap(), "1200.5", "not the drawn text"); + client.fold_frame(ServerFrame::Commands(vec![Command::Project(at("777", "777"))])); + let later = client.edit_target(0, 0).unwrap().expect("a record cell"); + assert_eq!(client.edit_seed(&later).unwrap(), "777", "not the cache at open"); + assert_eq!(client.edit_seed(&first).unwrap(), "1200.5", "a target is one capture"); + + assert!(client.typed_cell(&later, "777").unwrap().is_none()); + let edited = client.edited_target(&later).unwrap(); + assert!( + client.typed_cell(&edited, "777").unwrap().is_some(), + "an edited editor typed back to its seed composed nothing" + ); + } + /// The end-to-end shape a JS harness drives: bootstrap, key, message. #[test] fn a_client_can_be_driven_entirely_through_json() { diff --git a/crates/improvise-wasm-client/src/render.rs b/crates/improvise-wasm-client/src/render.rs index 24ca5e93..f71d120e 100644 --- a/crates/improvise-wasm-client/src/render.rs +++ b/crates/improvise-wasm-client/src/render.rs @@ -133,9 +133,6 @@ pub struct RenderCell { /// May this client compose a write for it? `CellId::Record` alone. pub editable: bool, pub selected: bool, - /// What an editor should start with: the stored value, not the formatted - /// one. Seeding from `text` turns `1200` into the string `"1,200"`. - pub edit_text: String, } impl RenderCell { @@ -155,7 +152,6 @@ impl RenderCell { | None => false, }, selected, - edit_text: cell.edit_text.clone().unwrap_or_default(), } } } @@ -1024,10 +1020,6 @@ mod tests { "a row says which of the two it is; the page does not guess" ); assert_eq!(value["rows"][0]["labels"][0], "North"); - assert_eq!( - value["rows"][0]["cells"][0]["edit_text"], "1200", - "an editor seeds from the stored value, never from the drawn one" - ); } fn page_axis(cat: &str, selected: &str, items: Option>) -> PageAxis { diff --git a/crates/improvise-wasm-client/src/socket.rs b/crates/improvise-wasm-client/src/socket.rs index 1bb05232..2da28cd7 100644 --- a/crates/improvise-wasm-client/src/socket.rs +++ b/crates/improvise-wasm-client/src/socket.rs @@ -19,6 +19,20 @@ pub struct EditTarget { pub opened: OpenedEditor, } +impl EditTarget { + /// The same target after the user's first edit: a commit of its seed is then + /// a write (improvise-jibp). The page's `` is not a buffer `resolve` sees. + pub fn edited(self) -> Self { + Self { + opened: OpenedEditor { + touched: true, + ..self.opened + }, + ..self + } + } +} + /// What a refused batch says to the person who composed it. Worded here rather /// than on `Rejection`, and why the wording matters: `context/code/clients.md`. fn rejection_message(rejection: &Rejection) -> String { @@ -304,6 +318,24 @@ mod tests { } } + /// BUG (improvise-jibp) — see `context/test-evidence/clients.md`. + #[test] + fn a_mouse_target_marked_edited_writes_its_own_seed() { + let mut client = client_showing(Some(CellId::Record(record("Revenue")))); + let target = client.edit_target_at(0, 0).expect("a record cell"); + assert_eq!( + client.typed_target_frame(target.clone(), "1200"), + None, + "control: an untouched editor committing its seed writes nothing" + ); + let edited = target.edited(); + assert_eq!(edited.opened.seed, "1200", "marking must not move the seed"); + assert!( + client.typed_target_frame(edited, "1200").is_some(), + "an edited editor typed back to its seed was dropped" + ); + } + /// BUG (improvise-ixxx) — see `context/test-evidence/clients.md`. #[test] fn a_mouse_commit_is_judged_against_the_seed_it_opened_with() { diff --git a/web/app.js b/web/app.js index 31289ce1..3277eb55 100644 --- a/web/app.js +++ b/web/app.js @@ -546,20 +546,19 @@ function gridTable(model) { /** Open an inline editor. Committing sends a frame; the value that comes * back is the server's, never the string typed here. */ -function edit(td, row, col, seed) { +function edit(td, row, col) { if (editing) return; // WHICH cell, captured now: rows renumber while the editor is open, so the // position is not an address by commit time (improvise-2wu3). - const target = client.edit_target(row, col); + let target = client.edit_target(row, col); if (!target) return; - // `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. + // always repaints, so restoring the seed would leave `1200` where `1,200` was. const drawn = td.textContent; const input = document.createElement("input"); - input.value = original; + // The seed the target captured, never the paint, which may be stale while + // another editor is open: one capture shown and compared (improvise-ugj4). + input.value = client.edit_seed(target); td.replaceChildren(input); input.focus(); input.select(); @@ -590,6 +589,8 @@ function edit(td, row, col, seed) { // 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). + // The first edit marks the target, so retyping the seed is still a write. + input.addEventListener("input", () => (target = client.edited_target(target)), { once: true }); input.addEventListener("keydown", (e) => { if (e.key === "Enter") dismiss(true); else if (e.key === "Escape") dismiss(false); @@ -745,7 +746,7 @@ el.grid.addEventListener("mousedown", (event) => { 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 away. - if (action.editable) edit(td, action.row, action.col, action.editText); + if (action.editable) edit(td, action.row, action.col); else repaint(); }); diff --git a/web/shell.js b/web/shell.js index 169373a7..296fa1de 100644 --- a/web/shell.js +++ b/web/shell.js @@ -222,7 +222,6 @@ export function gridPlan(model) { row: dataRow, col, text: typing ? `${model.editing}▏` : cell.text, - editText: cell.edit_text, editable: cell.editable, classes: { numeric: cell.numeric && !typing, @@ -260,7 +259,7 @@ export function gridPlan(model) { /** 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 }; + return { row: String(cell.row), col: String(cell.col) }; } /** …and the inverse: what a delegated mousedown MEANS. `null` where the event @@ -271,7 +270,7 @@ export function cellAction(dataset, editable) { // 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 ?? "" }; + return { row, col, editable }; } /** The same, for the surfaces whose rows are named by one index: the panels, diff --git a/web/tests/dispatch.test.js b/web/tests/dispatch.test.js index 3246634e..045a6811 100644 --- a/web/tests/dispatch.test.js +++ b/web/tests/dispatch.test.js @@ -16,7 +16,6 @@ import { const cell = (text, extra = {}) => ({ text, - edit_text: text, numeric: true, error: false, selected: false, @@ -40,7 +39,7 @@ function grouped() { { kind: "data", labels: ["Cold Brew"], - cells: [cell("3", { editable: false }), cell("4", { edit_text: "4000" })], + cells: [cell("3", { editable: false }), cell("4")], }, ], }; @@ -50,7 +49,7 @@ 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" }); + assert.deepEqual(action, { row: 1, col: 1, editable: true }); }); // improvise-b8ot: the DATA row index is what a click sends, and `gridPlan` diff --git a/web/tests/grid_plan.test.js b/web/tests/grid_plan.test.js index c6f04420..61dbd6b1 100644 --- a/web/tests/grid_plan.test.js +++ b/web/tests/grid_plan.test.js @@ -8,7 +8,6 @@ import { gridPlan } from "../shell.js"; const cell = (text, extra = {}) => ({ text, - edit_text: text, numeric: true, error: false, selected: false, @@ -102,14 +101,6 @@ test("an empty edit buffer still means an editor is open", () => { assert.equal(plan.body[1].cells[0].text, "▏"); }); -test("the editor is seeded from the stored value, not the drawn one", () => { - const model = grouped(); - model.rows[1].cells[0] = cell("20,000", { edit_text: "20000" }); - const plan = gridPlan(model); - 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.