feat: the page bar becomes a control, and [/] reach the modes about the document
Three tickets on one subject: paging.
improvise-rw6l — clicking a page chip opens a menu of that axis's items, with
a down-arrow saying so. Almost nothing had to be derived: `PageAxis::items` was
already projected and `ModelCommand::SetPageSelection` was already on the wire.
The one gap was that `RenderPageAxis` dropped the names, keeping the position
and count improvise-p1ym needed for `[`/`]`. `items` is relayed INSIDE `cycle`
rather than beside it, so there is one predicate and not two: a Filter axis has
no cycle, therefore no arrow, no menu, no items, and composes nothing if a click
reaches it — all four off `PageAxis::cycle`, the one home of gotcha 27's split.
improvise-771e — the browser tile bar said "Quarter Pag". `short_label` is three
characters so the TUI aligns in a fixed-width terminal (gotcha 26); a browser has
no column to fit. `render.rs` asks `plain_label` instead; `short_label` is
untouched. Note what pinned the truncation was `harness/tests/drill.spec.js`,
not `tests/grid_frame_agreement.rs`, which does not mention the tile bar.
improvise-wiew — `[`/`]` move from Normal onto a new `document_keymap` between
the global and the modes whose subject IS the document (Normal, the three
panels, TileSelect; RecordsNormal through Normal). Help and the import wizard
take the global directly. Per the user's ruling: an overlay has a subject of its
own, and these are the first shared keys that MUTATE the document — under the
help overlay that is an invisible change to the grid it is covering.
Boundary crossings to flag, both outside the brief's stated write scope and
neither a wire change:
- `resolve.rs` gains `Click::PageItem` and `lib.rs` a `click_page_item`
binding. Composing the command anywhere else would put a derivation in the
page, which design-principles §5 forbids; `Command`/`ModelCommand` are
untouched.
- `improvise-command/src/command/keymap.rs` gains three tests. The two
existing guards were the mechanism that forced the wiew argument, so they
had to be answered where they live.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
@@ -821,3 +821,90 @@ Faults injected against the fixed code:
|
||||
duplication improvise-636d removes, and it is invisible to every check that
|
||||
asks what a key *does*.
|
||||
|
||||
|
||||
## `src/keymap.rs` — the layer between the global and the modes (improvise-wiew)
|
||||
|
||||
The user asked for `[`/`]` "probably in the global keymap", and then ruled on
|
||||
the question that ask left open: *"maybe we need an intermediate keymap: the
|
||||
help display (and import wizard, etc.) shouldn't respond to these keys, but the
|
||||
'normal' modes like categories/tiles/formulas/etc. should."*
|
||||
|
||||
So the distinction is **is the document the subject right now**, and it is a new
|
||||
distinction rather than the global's. Every key the global already binds — `?`,
|
||||
`:`, the three panel toggles — is a **UI** action. `page-next`/`page-prev`
|
||||
resolve to `ModelCommand::SetPageSelection`, which is
|
||||
`EffectKind::Model(LayoutGeometry)`: the first key on a shared layer that
|
||||
*changes the document*. Under the Help overlay that is a real mutation the user
|
||||
cannot see, because the overlay is covering the grid it just moved.
|
||||
|
||||
`document_keymap` is therefore a third level: `global → document → {Normal, the
|
||||
three panels, TileSelect}`, with Help, ImportWizard and the text-entry modes
|
||||
taking `global` directly. RecordsNormal reaches it **through Normal**, which it
|
||||
already parents — parenting it directly as well would be a second edge to keep
|
||||
in step, and `records_normal_inherits_the_document_layer_once_through_normal`
|
||||
asserts the chain is exactly four long.
|
||||
|
||||
Two things about the name. It is `document_keymap` and not `common`/`shared`
|
||||
deliberately: the name is the only thing that tells the next person whether a
|
||||
new mode should parent it, and a name that describes *who inherits it* invites
|
||||
everything to join. And the layer is **derived** in every test off
|
||||
`Normal.parent()` rather than exported, so a test cannot restate a membership
|
||||
the table disagrees with.
|
||||
|
||||
### the_page_keys_reach_the_document_modes_and_no_overlay
|
||||
|
||||
The six document modes answer `]`/`[` with `page-next`/`page-prev`; Help
|
||||
resolves **neither at all** (`lookup` is `None`, not "shadowed into something
|
||||
harmless"), ImportWizard answers `handle-wizard-key` through its `Any`, and
|
||||
CategoryAdd answers `append-char` — so typing a bracket into a name still types
|
||||
it, which is the fallback order (`AnyChar` before the parent) doing its job.
|
||||
|
||||
Fault injected: parenting Help to `document_keymap` instead of to `global`.
|
||||
`]` in Help then resolves to `page-next` and the test fails. Note the assertion
|
||||
that discriminates is the Help one — the six positive rows pass either way.
|
||||
|
||||
### records_normal_inherits_the_document_layer_once_through_normal
|
||||
|
||||
Fault injected: `Keymap::with_parent(document.clone())` for RecordsNormal in
|
||||
place of `with_parent(normal)`. The test above **still passes** — RecordsNormal
|
||||
answers `page-next` either way — and this one fails on the parent identity.
|
||||
That pair is the reason it is a separate test: a duplicated inheritance edge is
|
||||
invisible to every behavioural assertion on the day it is added, and only shows
|
||||
up the first time Normal and the layer disagree.
|
||||
|
||||
### the_document_layer_binds_only_unmodified_char_keys
|
||||
|
||||
The sibling of `the_global_binds_only_unmodified_char_keys`, and it is owed for
|
||||
exactly the same reason: the layer is reached by **fallthrough**, so a non-Char
|
||||
binding on it would fire while a descendant mode's `AnyChar` was typing. No
|
||||
descendant binds `AnyChar` today, which is precisely why the constraint has to
|
||||
be written down rather than relied on — an unconstrained new layer is where the
|
||||
next "just put it in the shared one" lands.
|
||||
|
||||
### resolve::tests::picking_a_page_item_moves_that_axis_and_only_that_axis
|
||||
|
||||
improvise-rw6l. `Click::PageItem { axis, item }` is the browser's "the user
|
||||
picked straight off the bar", and the property that separates it from `]` is
|
||||
that it names **one** axis: `page_cycle` is an odometer that restates every
|
||||
navigable axis and carries between them, so on a two-axis fixture `]` from
|
||||
`Year=2024, Quarter=Q4` emits both `Year=2025` and `Quarter=Q1`. The click emits
|
||||
`Quarter=Q1` alone. A fixture with one page axis cannot tell the two apart.
|
||||
|
||||
The basis it claims is `state.basis.claim([])` — the same claim `page_cycle`
|
||||
makes, because `SetPageSelection` names no record (gotcha 43). It is written as
|
||||
that call rather than as the `published()` fixture so the test says *which*
|
||||
claim it means; the fixture carries a record tier this command must not state.
|
||||
|
||||
### resolve::tests::a_filter_axis_offers_no_item_to_pick
|
||||
|
||||
Gotcha 27, from the click side. A Filter axis is on the page bar and carries no
|
||||
items, so `PageAxis::cycle` is `None` and the click composes nothing — the same
|
||||
one home the arrow and the menu follow. The two out-of-range rows beside it
|
||||
(an axis past the end of the bar, an item past the end of the list) are there
|
||||
because the page names both by index, so a stale bar under a repaint is the
|
||||
realistic way either arrives.
|
||||
|
||||
Fault injected: resolving `items` off `axis.items` directly instead of through
|
||||
`cycle()`. The Filter arm then indexes an empty `Some(vec![])` for a Page axis
|
||||
with no items — and, more to the point, the `items.is_some()` spelling is the
|
||||
one gotcha 27 says is *not* the split.
|
||||
|
||||
Reference in New Issue
Block a user