docs(browser): the arguments for the page bar, the totals and the delegation
Self-review findings, applied.
The word `Total` is now spelled twice — `src/ui/grid.rs` and
`web/shell.js::gridPlan` — which fails design-principles §5's own test: would a
third front end write this again? Yes. It is kept local on the `indicator()`
precedent (the collapse glyph is spelled in both ends while the FACT is shared)
rather than built into a protocol string table for one word, and it is now
NAMED as debt at both the call site and in `context/code/clients.md` instead of
being left for the next reader to find.
`frameCostOf` now counts the totals strip, and that is a change to a rule
`context/code/clients.md` already states — so the section says why the strip is
reserved where the inline group rows deliberately are not: it is present
exactly once, never interleaved with data, so counting it is neither the
feedback loop nor the ratchet that argument rules out. The page bar needs no
rule at all, being outside the box `extentOf` measures.
The evidence file claimed three injections into `dispatch.test.js` that had not
been run. They have been now, and the claim is rewritten to match what actually
failed rather than what was planned — the third was `navigable: true`
unconditionally, not the `position !== ""` reading the text described.
`doctrine(an_injected_fault_must_be_the_fault_you_named)` applies to the
description as much as to the fault.
Injections run and read, all against the served copy or the source:
gridPlan takes the table row index -> both round-trip tests fail
whole() drops its empty-string guard -> both "means nothing" tests fail
navigable: true -> the Filter test alone fails
grid bound to `click` -> 4 Playwright tests fail, including
the pre-existing improvise-0x9u guard
paintPages never called -> the page bar assertion fails
the tfoot block never runs -> the totals assertion fails
page_coords deleted from render_model -> the Rust page-bar test fails
totals_strip returns Vec::new() -> the Rust totals test fails, and the
new cache-parity guard reports it
PageAxis::cycle replaced by items.is_some() -> the cycle index test fails
Two residuals named rather than discovered later: the `Total` spelling above,
and the page bar being display-only — clicking an entry does nothing, because
composing a page change from a click needs a `click_page` on the wasm surface
that does not exist.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
@@ -1296,6 +1296,13 @@ It is **measured off the painted model rather than predicted**, and defaults to
|
||||
the old one-and-one before the first paint, which is the only moment there is
|
||||
nothing to measure.
|
||||
|
||||
**The totals strip IS reserved**, unlike the inline group rows below, and the
|
||||
difference is what makes it countable: it is chrome on the same terms as a
|
||||
header level — present exactly when `totals` is non-empty, once, never
|
||||
interleaved with the data — so counting it is neither a feedback loop nor a
|
||||
ratchet. The page bar is not reserved and needs no rule, because `#pages` sits
|
||||
*outside* `#grid` and so is already absent from the box `extentOf` measures.
|
||||
|
||||
**Inline group ROWS are deliberately NOT reserved.** They take real height, but
|
||||
how many are in view depends on how many rows the window asked for, so every way
|
||||
of counting them is either a *feedback loop* (count the current ones: a smaller
|
||||
@@ -1442,6 +1449,83 @@ 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.
|
||||
|
||||
### One handler per surface, and the node carries the answer
|
||||
|
||||
improvise-b8ot, on the user's ruling: *one handler outside the area that gets
|
||||
redrawn, and dispatch events there*. The five clickable surfaces — the grid,
|
||||
the category tree, the two side panels and the tile bar — had one listener per
|
||||
**node**, created inside the render loops and reallocated on every repaint.
|
||||
|
||||
The roots were already stable and that is what made it cheap: `replaceChildren`
|
||||
replaces an element's *children*, not the element, so every `el.*` entry
|
||||
survives every repaint and needed no new wrapper.
|
||||
|
||||
**Not a performance change, and it must not be argued as one.** The cell count
|
||||
is bounded by the announced viewport rather than by the document, and nothing
|
||||
leaked — `replaceChildren` drops the nodes and their listeners together. The
|
||||
argument is design-principles §5: paint and wire, applied to the dispatch as
|
||||
well as to the paint.
|
||||
|
||||
**The operands are carried on the node, never re-derived from the DOM.**
|
||||
`gridPlan` already decides `row`, `col`, `editable` and the class map, and its
|
||||
`row` counts **data** rows because a group header is not one. A delegated
|
||||
handler that walked up the table counting `<tr>`s would be a second derivation
|
||||
of exactly that, and it would disagree the moment a group header appeared. So
|
||||
the plan writes `data-row`/`data-col`/`data-edit`, `cellAction` reads them
|
||||
back, and the handler is a lookup. The split follows the bead's own division:
|
||||
**data attributes for operands**, the **existing `editable` class** for the
|
||||
predicate, and **ids only for the surfaces**, which they already were — a row
|
||||
or cell id would have to be unique per document and these are rebuilt per frame.
|
||||
|
||||
Four behaviours a naive rewrite loses, all silent, all asserted in
|
||||
`harness/tests/contract.spec.js`: `mousedown` rather than `click` (above),
|
||||
`preventDefault()` **and only over a cell** — a header carried no listener
|
||||
before, so a blanket handler on the root is a behaviour change — the repaint
|
||||
that is **skipped** when an editor opens, and closing a live editor **first**,
|
||||
before the click is composed.
|
||||
|
||||
**The editor's own `keydown` and `blur` stay direct**, deliberately: one
|
||||
transient node, one at a time, attached at creation and gone with it. `blur`
|
||||
does not bubble, so delegating it would need a root that does not own the input.
|
||||
|
||||
### The page bar and the totals strip
|
||||
|
||||
improvise-p7lh, and the shape is worth more than the fix. `GridCache` has
|
||||
carried `page_coords` and `totals` since improvise-n10; the server projected
|
||||
both to every session, every client folded both correctly, the TUI painted
|
||||
both — and `RenderModel` had no field for either, so the browser painted
|
||||
neither. The shipped front page is `Quarter: page, Q1`, so it drew a grid
|
||||
filtered to Q1 with nothing saying so, while `[`/`]` — which resolve *locally*
|
||||
off that same `page_coords` — changed every number in silence.
|
||||
|
||||
**Why the parity guard did not see it.** `web/tests/render_surface.test.js`
|
||||
asks that every field `RenderModel` exposes is painted. Its domain is
|
||||
`RenderModel`, which is the thing under test: a fact that never reaches it is
|
||||
outside the domain, so the check was closed under the defect. The honest
|
||||
question is one level up — every field of every widget cache reaches a front
|
||||
end, or is named as debt — and that is now asserted too. It would have fired
|
||||
when the fields were added.
|
||||
|
||||
Two rules the painting itself owes. The **totals are relayed, never summed
|
||||
here**: the total runs over every row including ones a viewport scrolled past,
|
||||
which is why it is projected at all (gotcha 44), and an empty strip means "this
|
||||
grid has no totals" rather than a row of blanks (gotcha 33). And the
|
||||
**Page/Filter split is asked of `PageAxis::cycle`**, the one home of it: the
|
||||
plausible alternative, `items.is_some()`, agrees on both common cases and
|
||||
disagrees on an empty list, which is how a rule gets quietly respelled
|
||||
(`invariant(the_page_filter_split_lives_in_the_type)`).
|
||||
|
||||
**Two residuals, named rather than left to be discovered.** The word `Total` is
|
||||
now spelled twice — `src/ui/grid.rs` and `web/shell.js::gridPlan` — which fails
|
||||
§5's own test, *would a third front end write this again*. It is kept local on
|
||||
the `indicator()` precedent (the collapse glyph is spelled in both ends while
|
||||
the *fact* is shared) and because the alternative is a protocol-level string
|
||||
table for one word; `invariant(a_ui_string_is_keyed_by_a_command_never_by_a_key)`
|
||||
is where it belongs if a second such string ever appears. And the page bar is
|
||||
**display-only**: it shows the cycle position so `[`/`]` have an affordance, but
|
||||
clicking an entry does nothing, because composing a page change from a click
|
||||
needs a `click_page` on the wasm surface that does not exist yet.
|
||||
|
||||
### The keyboard
|
||||
|
||||
`Client::on_key_event` classifies the event and folds it locally where this
|
||||
|
||||
@@ -1877,14 +1877,19 @@ which this test found rather than remembered.
|
||||
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.
|
||||
and "the table row" differ. Three injections, three failures, each run and read:
|
||||
|
||||
- `gridPlan` takes the row's position in `model.rows` rather than counting data
|
||||
rows, so a group header consumes an index. This is the click-coordinate bug
|
||||
gotcha 1 is about, and it is **invisible on any ungrouped grid** — which is
|
||||
why the fixture has a header between its rows. Fails both round-trip tests.
|
||||
- `whole()` drops its empty-string guard. `Number("")` is `0`, so a header with
|
||||
no `data-row` reads as row 0 and every click on the frame moves the selection
|
||||
to the origin — which is exactly why the guard is not a truthiness test.
|
||||
Fails the two "means nothing at all" tests.
|
||||
- `pageEntries` reports `navigable: true` unconditionally. Fails the Filter
|
||||
test alone, and nothing else — the Page case cannot see it, which is the
|
||||
point of seating a Filter axis in the fixture at all.
|
||||
|
||||
**`render_surface.test.js`, the cache direction** (improvise-p7lh) — the
|
||||
existing guard is closed under *what `RenderModel` chose to expose*, so a fact
|
||||
|
||||
@@ -135,6 +135,8 @@ export function gridPlan(model) {
|
||||
? null
|
||||
: {
|
||||
span: cornerSpan,
|
||||
// The SECOND spelling of this word — `src/ui/grid.rs` has the other.
|
||||
// Named as debt rather than shared: context/code/clients.md.
|
||||
label: "Total",
|
||||
cells: model.totals.map((cell) => ({
|
||||
text: cell.text,
|
||||
|
||||
Reference in New Issue
Block a user