Skip to content

fix(ui): advertise only shortcut digits that work in the sidebar - #257

Merged
argszero merged 1 commit into
mainfrom
fix/sidebar-shortcut-advertises-a-dead-key
Sep 15, 2026
Merged

argszero merged 1 commit into
mainfrom
fix/sidebar-shortcut-advertises-a-dead-key

Conversation

@argszero

Copy link
Copy Markdown
Owner

Summary

The sidebar advertises a keyboard shortcut digit on every nav item (a .nav-key badge plus a title reading "Shortcut N · "), and one global keydown handler resolves that digit. Both sides are halves of a single contract and must read the same array:

const NAV_ORDER = NAV.flatMap((g) => g.items);   // the registry
const short = NAV_ORDER.indexOf(item) + 1;       // the keycap  (renderNav)
const item = NAV_ORDER[Number(e.key) - 1];       // the handler (global keydown)

renderNav()'s guest branch hand-built an equivalent-looking literal instead of drawing from the registry. That object is not a member of NAV_ORDER, so indexOf(item) was -1:

  • the guest sidebar showed keycap 0, and NAV_ORDER[-1] is undefined — a dead key;
  • the tooltip said "Shortcut 0 · Marketplace", so the lie is visible without pressing anything;
  • the digit that does open the marketplace is 2 — a number the guest is never told.

Reproduction (no account needed): open the instance, click "browse as guest", and press 0 / 2 while watching the single nav item.

Fingerprint of drift rather than design: ui/README.md documents the opposite — "数字 1-8 → 切换侧边栏视图(键位 = NAV_ORDER 下标 +1…)", and the handler's own comment states the invariant ("角标是 NAV_ORDER 下标 +1,这里用同一数组取项,视图增删后两者自动保持一致"). The shape dates from #44 (v1.17 D keyboard accessibility), which added the keycap machinery without noticing that the guest item predates it and belongs to no registry; it survived #86 (i18n) and all the recent nav work.

Changes

  • ui/js/app.js: the guest group draws from the registry it shares with the handler — items: NAV_ORDER.filter((it) => GUEST_VIEWS.includes(it.id)). The guest keycap becomes 2 and the tooltip agrees; the live and role paths are untouched (they already drew from NAV/NAV_ORDER). Label rendering is unchanged (T(item.label) on the registry's key).
  • src/state_gate.rs: new invariant the_sidebar_advertises_only_digits_that_work, four rules with independent teeth:
    1. renderNav renders no hand-built nav-item literal (id: "…" inside its body) — the sidebar may only render registry members;
    2. the keycap is derived exactly once and that line must not branch on the session (rejects the tempting isGuest ? 2 : NAV_ORDER.indexOf(item) + 1, where the session picks a number nobody is responsible for making work);
    3. exactly one line in the whole file indexes the registry (NAV_ORDER[…]) and it selects with the pressed digit (Number() — the two halves name the same array;
    4. the registry is derived from NAV (NAV.flatMap(…)), not a second hand-written list.
  • ui/README.md: the "键位只有一个真源" rule, including the guest branch, and the reason the static gate owns the shape while the probe owns the values.
  • ui/index.html: app.js?v=20260915-11-12.
  • src/i18n_pack.rs: recalibrated positive controls T_LITERAL_COUNT 543 → 542 and T_LITERAL_DISTINCT 434 → 433 — removing that one guest T("nav.marketplace") call site is the only change to the T() literal population (the key itself is still reachable dynamically through T(item.label); both packs keep their 809 keys).
  • The gate is comment-aware (strips // lines, multi-line /* … */ bodies and inline pairs) — the new comment and the C2141 doc section both mention NAV_ORDER[ and the old literal, so a naive contains would red-flag its own documentation (ledger pitfall #296/#309).

Related Issue

No issue exists for this; found during Recon. No linked issue is expected.

Tests

  • cargo test285 passed (283 before; +2 gate tests).
  • cargo fmt --check — clean.
  • cargo clippy --all-targets — only the pre-existing collapsible_match at src/protocol.rs:662.

jsdom probe (real index.html + the four real scripts, fetch stubbed and logged, real "browse as guest" button, real nav clicks and real keydown events; the "working digit" is measured on the same apparatus by pressing 0–9 and watching for a re-render, never copied):

leg scenario expectation baseline afc9c6f this branch
A0 guest mode entered, exactly one nav item precondition pass pass
A1 axis: pressing the keycap the sidebar shows opens that item's view red→green FAIL (advertised="0", no switch) pass (advertised="2")
A2 axis: the keycap equals the digit that actually opens it red→green FAIL (keycap="0", measured ["2"]) pass
A3 axis: the tooltip names the same digit as the keycap red→green FAIL ("Shortcut 0 · Marketplace", measured 2) pass
A4 control: the guest item's label is still the localized one pass pass pass
B0/B1/B2 control (live role=user): 6 items, every keycap opens its own view, keycaps are the registry positions 1-5,8 pass pass pass
C1/C2 control (live role=admin): the role item has a title but no badge, and the digit in that title opens the view pass pass pass
D1 control: digits resolving to no registry entry (9, 0) switch nothing — i.e. A1 fails for the right reason pass pass pass

Rejected competitor fixes (probe, both red; --base pins the expectations to the explicit baseline):

variant shape probe why rejected
m_visible renumber the keycap from the visible items (groups.flatMap(g => g.items).indexOf(item) + 1) 6/11 — {A1,A2,A3,B1,B2} guest 1 is still dead (1 is the dashboard ⇒ lock toast), and it breaks the live path: settings keycap becomes 6 while the handler still indexes NAV_ORDER (8)
m_nokey hide the guest badge instead of fixing the number 8/11 — {A1,A2,A3} the tooltip still advertises Shortcut 0 (A3), so the sidebar still lies — only about a shorter distance

Gate A/B (ui/js/app.js mutated in place, restored byte-for-byte, md5 checked before/after every leg):

leg mutation gate rule that fired
m0 untouched PASS
m1 restore the hand-built guest literal (= the shipped defect) FAIL rule 1
m2 const short = isGuest ? 2 : NAV_ORDER.indexOf(item) + 1; FAIL rule 2
m3 handler indexes a different list FAIL rule 3
m4 const NAV_ORDER = [...NAV[0].items, ...NAV[1].items]; FAIL rule 4
m5 competitor m_visible (badge from visible order) FAIL rule 2 (keycap no longer derived from the registry)

Restored ui/js/app.js md5 == pristine md5 after every leg. Both instruments agree on the fix and reject both competitors; the gate is deliberately the stricter of the two on shape (rule 1), while the probe owns the values.

Checklist

  • 分支命名符合约定(fix/<描述>
  • Commit message 使用 Conventional Commits 格式(fix(ui): …
  • 单一职责,改动最小化(一行修复 + 门禁 + 文档;src/i18n_pack.rs 仅两个阳性对照常量重新校准)

The sidebar prints a keycap digit and a "Shortcut N" tooltip on every nav item,
and one global number-key handler resolves the digit. Both sides must read the
same array:

  const NAV_ORDER = NAV.flatMap((g) => g.items);
  const short = NAV_ORDER.indexOf(item) + 1;     // the keycap
  const item = NAV_ORDER[Number(e.key) - 1];     // the handler

renderNav()'s guest branch hand-built an equivalent-looking literal that is not
a member of NAV_ORDER, so indexOf(item) was -1 and the guest sidebar advertised
keycap 0 with the tooltip "Shortcut 0 . Marketplace" -- while 0 resolves to
NAV_ORDER[-1], i.e. nothing at all. The digit that does open the marketplace is
2, which the guest is never told.

The shape dates from #44 (v1.17 D keyboard accessibility), which added the
keycap machinery without noticing that the guest item is not a registry member;
it survived #86 (i18n) and the recent nav work. ui/README.md documents the
opposite (the badge is NAV_ORDER's index + 1, and the digits are 1..N of that
same array), so this is drift, not a trade-off.

Fix: the guest group draws from the registry it shares with the handler --
NAV_ORDER.filter((it) => GUEST_VIEWS.includes(it.id)).

Gate: state_gate::the_sidebar_advertises_only_digits_that_work -- four rules
with independent teeth: (1) renderNav renders no hand-built nav-item literal;
(2) the keycap is derived once and never branches on the session; (3) exactly
one line indexes the registry and it selects with the pressed digit; (4) the
registry is derived from NAV, not a second hand-written list. A companion test
pins the scanners on synthetic input, comments included.

Also recalibrates the i18n positive controls (T_LITERAL_COUNT 543 -> 542,
T_LITERAL_DISTINCT 434 -> 433): removing that one T("nav.marketplace") call
site is the only change to the T() literal population.
@argszero

Copy link
Copy Markdown
Owner Author

Self-review (Committer, allow_self_merge: true).

Verified on the branch tip 2ca133f:

  • The defect is real and reachable without an account: guest mode shows one nav item with keycap 0 / title "Shortcut 0 · Marketplace", while 0 resolves to NAV_ORDER[-1] (nothing) and the working digit is 2. Measured by the jsdom probe on the pinned baseline afc9c6f (md5 077f2264…), legs A1/A2/A3 red.
  • The fix is one line and stays inside the existing contract: the guest group now draws from NAV_ORDER (the array the handler indexes) via GUEST_VIEWS. Live and role paths untouched; label rendering unchanged.
  • Both competitor shapes are rejected, not just "something changed": renumbering from the visible order (m_visible) breaks the live path too (settings 6 vs handler 8), and hiding the badge (m_nokey) leaves the tooltip advertising Shortcut 0.
  • The gate's four rules each have an independent tooth: m1→rule 1, m2→rule 2, m3→rule 3, m4→rule 4, m5→rule 2 (badge no longer derived from the registry); ui/js/app.js restored byte-for-byte after every leg.
  • No unintended surface: cargo test 285 passed, cargo fmt --check clean, clippy shows only the pre-existing collapsible_match at src/protocol.rs:662. The two i18n_pack.rs constants moved by exactly the one T() literal this change removes; no i18n key was added or dropped.

@argszero
argszero merged commit 9982063 into main Sep 15, 2026
1 check passed
@argszero
argszero deleted the fix/sidebar-shortcut-advertises-a-dead-key branch September 15, 2026 01:52
@argszero argszero mentioned this pull request Sep 15, 2026
10 tasks
argszero added a commit that referenced this pull request Sep 15, 2026
Ships the 18 PRs merged since v0.7.24 (#242-#259). Schema 14 -> 15 (two
covering indexes, applied at startup). No config change, so no deployment-side
config.toml edit is needed.

Two themes:

Perf on the NFS dev database
- #259: stop mapping the db (PRAGMA mmap_size 64MB -> 0) and stop a real write
  per request (dao::touch_api_key gains a 60s guard). Measured on the live dev
  db: mmap=64MB 1.7-3.1s per COUNT / 250 MiB read vs mmap=0 ~10.5ms / 80 KiB;
  mmap=0 alone still leaves ~1.2s behind any write, so the pair is required.
- #242: codify the two emergency indexes in a v15 migration and gate the
  conditional joins at the plan level.
- #243: read the sharing page's earn total from one batched aggregate.

Frontend: display must equal what it filters on, and one fact, one source
- #250 one writer for the transaction cache; #251 clear every session slot at
  the identity boundary and give the wallet view a loader; #253 one shared
  writer for the wallet/dashboard month-changes; #254 boot loads only the
  destination view; #255 a model row's identity is the model, not its index;
  #256 the marketplace source follows the session, not whether data arrived;
  #257 the sidebar advertises only digits that work; #258 the admin
  total-balance card sums the gift amount its caption names.

i18n
- #249 every backend error reaches the wordlist, and the comment stripper stops
  mangling UTF-8; #252 the backend stops inventing Chinese display labels in
  response data fields.

Forms and robustness
- #244 a non-auth boot failure no longer looks like being logged out; #245 a
  credential 401 is no longer read as a session expiry; #246 wire timestamps
  reach the renderer unsliced; #247 inline cards submit from every field; #248
  a market row's availability label comes from that row.

- Cargo.toml / Cargo.lock: 0.7.24 -> 0.7.25.
- CHANGELOG.md: v0.7.25 entry.
- ui/index.html: cache-bust left as-is; the UI PRs in this release already
  advanced it past the value deployed with v0.7.24 (app.js 20260915-13,
  i18n.js 20260915-3).

cargo test 288 passed; cargo fmt --check clean; clippy unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant