fix(ui): let the keyboard row action follow the highlight, not the index - #296
Merged
Merged
Conversation
The keyboard navigation kept the "current row" as an index (`kbd.i`) into the `<tbody>`'s row list, and `kbdEnter()` clicked whatever row that index landed on — including a row the user had never highlighted, and one that belongs to a view they had already left. Two faces: 1. `switchView()` changed views without clearing the armed row, so a stale `kbd.i` survived the switch and Enter still acted on the old table's row. 2. `kbdEnter()` trusted the index alone: after any repaint replaced the tbody's `innerHTML`, the index pointed at a different row than the one carrying the highlight. Make the highlight itself the identity: `kbdEnter()` acts only on a row that carries `.row-active` itself (a repaint drops the old element and with it the highlight, so any repaint invalidates the arming), and `switchView()` calls `kbdClear()` after the role/guest guard accepts the destination (a rejected switch must not clear). Gate `state_gate::the_keyboard_row_action_follows_the_highlight_not_the_index` verifies the two halves independently; `ui/README.md` states the contract and the gate's scope. Cache-bust bumped for `ui/js/app.js`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The keyboard navigation stored its "current row" as an index (
kbd.i) into the<tbody>'s row list, andkbdEnter()clicked whatever row that index landed on.That is an identity claim the index cannot support, with two faces:
switchView()changed the visible view withoutclearing the armed row, so a stale
kbd.isurvived the switch and Enter stillacted on the previous table's row.
kbdEnter()trusted the index alone;after any repaint replaced the tbody's
innerHTML, the index pointed at a differentrow than the one the user had highlighted — Enter clicked a row nobody had selected.
The highlight is what the user sees, so make it the identity:
kbdEnter()acts onlyon a row that carries
.row-activeitself (a repaint drops the old element and withit the highlight, so any repaint invalidates the arming for free), and
switchView()calls
kbdClear()after the role/guest guard accepts the destination (a rejectedswitch must not clear).
Related Issue
No linked issue — the repository has no open issues; this is a self-contained
correctness fix (same shape as the previous UI-contract fixes in this series).
Changes
ui/js/app.js—switchView()clears the keyboard arming on an accepted destination;kbdEnter()requires the target row to carry the highlight itselfui/README.md— the keyboard-navigation section states the contract and the gate's scopeui/index.html— cache-bust bump forui/js/app.jssrc/state_gate.rs— new gatethe_keyboard_row_action_follows_the_highlight_not_the_index(plus its roster and teeth self-tests)Tests
cargo test— 381 passed / 0 failed (baselinecc51f82was 378; +3 new tests)cargo fmt --check— cleancargo clippy --all-targets -- -D warnings— cleanr173_variant_drop_clear,r173_variant_drop_highlight_testeach fail on their own), a positive control, a roster check and a teeth checkInstrumentation (both instruments, each declaring its legs):
b0f1ef72…fails exactly on the axis rule (r1=false r2=false r3=true), and is green on the fixed tree.ALL LEGS AS DECLARED:basereproduces the defect,fixis accepted, and the two competing fixes (m_classonly,m_switchonly) are each rejected by their own leg.ui/js/app.jsmd52390d877a2859b7e49a7c02acf21e205is byte-identical to the tree committed here.Checklist
fix/)fix(ui): …)