diff --git a/HANDOFF.md b/HANDOFF.md index c3ce223..b23e79c 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -549,8 +549,36 @@ structurally sound and in the right unit. - **The upload screen does not say what happens next.** A dropped file goes `queued → extracting → mapping → needs_review`, and only the last one is actionable. Say so on the screen rather than in status labels. -- **Keyboard path through the review queue.** Confirm/reject/skip without the - mouse is the difference between reviewing forty-four items and abandoning it. +- ~~**Keyboard path through the review queue.**~~ Done. `j`/`k` move a cursor, + `1`–`9` choose a candidate, `Enter` confirms, `x` rejects, `/` reaches the + search field and `Escape` leaves it. There is no skip key: moving the cursor + *is* skipping, and a second key for it would only be a second way to lose + your place. + + **The cursor is an index into a list that shrinks underneath it**, which is + how it advances — settle the row it points at, the row after it takes that + position, and the cursor is already on the next thing to do. Clamped rather + than corrected, so settling the last row lands on the new last row instead of + running off the end. Focus follows it for real (`tabIndex={-1}` on the row), + not as a class, so a screen reader is told the cursor moved and the row is + scrolled into view without asking — but only once a key has been pressed, + because moving focus on mount yanks the page for somebody who arrived with a + mouse. + + **`keyAction` is pure and tested; the listeners are not.** Same split as + `idleDelayMs`: two `addEventListener` calls and a cleanup that mirrors them + is what reading the file shows, while *which keystrokes this claims* is what + can be quietly wrong. The expensive failure is claiming one it should not + have — a `window` handler sees every keystroke on the screen, so typing + "flux" into a LOINC search must not reject the row on the `x`. Text fields, + modifier combinations, and Enter on an already-focused button are all + returned as "not ours", and each has a test. + + Two bugs the screen had all along surfaced while driving it: the tally + counted rejections as **rules learned**, when `/reject` answers + `alias_written: false` and means it — the one counter whose job is to make + convergence visible rather than claimed was overstating it. And a queue of + one read "1 result need you". - **`prefers-reduced-motion`.** The cascade scene animates unconditionally. - **A real 404 route.** Unknown `/app/*` paths currently render the shell empty. diff --git a/frontend/src/screens/Review.css b/frontend/src/screens/Review.css index e9e2777..35216fa 100644 --- a/frontend/src/screens/Review.css +++ b/frontend/src/screens/Review.css @@ -1,4 +1,38 @@ -.review__lede { color: var(--ink-body); max-width: 58ch; margin-bottom: var(--s6); } +.review__lede { color: var(--ink-body); max-width: 58ch; margin-bottom: var(--s3); } + +/* ---- the keyboard legend ------------------------------------------------- + Set in the caption size and faint ink: it is a reference somebody reads + once and then stops seeing, so it must not compete with the queue it sits + above. Hidden where there is no keyboard — a phone showing key caps it + cannot press is describing somebody else's screen. */ + +.review__keys { + display: flex; + flex-wrap: wrap; + align-items: center; + gap: var(--s1); + margin: 0 0 var(--s6); + font-size: var(--text-caption); + color: var(--ink-faint); +} +.review__keys-sep { margin: 0 var(--s2); color: var(--rule-strong); } + +.review__keys kbd { + font-family: var(--font-mono); + font-size: var(--text-caption); + line-height: 1; + padding: 3px 5px; + margin-right: 2px; + color: var(--ink-muted); + background: var(--paper-sunk); + border: 1px solid var(--rule); + border-bottom-width: 2px; + border-radius: var(--radius); +} + +@media (hover: none) { + .review__keys { display: none; } +} .ritems { list-style: none; margin: 0; padding: 0; display: grid; gap: var(--s5); } @@ -9,6 +43,18 @@ border-radius: var(--radius-lg); } +/* The cursor. A rule down the edge rather than a fill or a ring: several rows + are on screen at once, so it has to be findable at a glance without becoming + the loudest thing in a queue whose whole content is the evidence. It is also + the focus indicator — the row really does take focus — so the default + outline is dropped only because this replaces it. */ +.ritem--on { + border-color: var(--rule-strong); + box-shadow: inset 3px 0 0 var(--accent); +} +.ritem:focus, +.ritem:focus-visible { outline: none; } + /* The printed line, verbatim and set apart: this is the evidence. */ .ritem__printed { display: flex; @@ -31,7 +77,7 @@ .cand { display: grid; - grid-template-columns: 5.5rem 1fr auto; + grid-template-columns: 1.1rem 5.5rem 1fr auto; gap: var(--s3); width: 100%; padding: var(--s3); @@ -46,16 +92,28 @@ row and a filled state would read as already-confirmed. */ .cand--on { border-color: var(--accent); background: var(--accent-wash); } +/* The digit that picks this row. Faintest ink on the screen: it is an + accelerator, not part of the evidence, and it must not read as an ordering + the candidates have been ranked into. */ +.cand__key { font-size: var(--text-caption); color: var(--rule-strong); } .cand__code { font-size: var(--text-small); color: var(--ink); } .cand__name { font-size: var(--text-small); color: var(--ink-body); } .cand__why { font-size: var(--text-caption); color: var(--ink-faint); white-space: nowrap; } +/* No keyboard, no accelerators to advertise — same test the legend uses. */ +@media (hover: none) { + .cand { grid-template-columns: 5.5rem 1fr auto; } + .cand__key { display: none; } +} + .ritem__actions { display: flex; flex-wrap: wrap; gap: var(--s3); margin-top: var(--s5); } @media (max-width: 640px) { .ritem { padding: var(--s4); } .ritem__meta { margin-left: 0; width: 100%; } .cand { grid-template-columns: 1fr; gap: 2px; } + /* Stacked, the digit would take a line of its own for one character. */ + .cand__key { display: none; } .cand__why { white-space: normal; } } diff --git a/frontend/src/screens/Review.jsx b/frontend/src/screens/Review.jsx index dfd187a..8d74289 100644 --- a/frontend/src/screens/Review.jsx +++ b/frontend/src/screens/Review.jsx @@ -1,8 +1,9 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { Link } from "react-router-dom"; import { api, messageFor } from "../api/client"; import { usePatient } from "../patients/PatientContext"; +import { keyAction } from "./reviewKeys"; import "./Review.css"; /** @@ -12,9 +13,15 @@ import "./Review.css"; * it might be, with the reason each candidate surfaced. Confirming writes an * alias, so the same printed name resolves without asking again — the counter * at the top is there so that convergence is visible rather than claimed. + * + * It is also the one screen somebody works *down*. Forty-four rows at eight tab + * stops each is where a queue stops being reviewed and starts being dismissed, + * so the whole loop — move, choose, confirm — has keys. The queue keeps a + * cursor; the row under it owns the keys that settle a row, the list owns the + * keys that move between them. */ -function Candidate({ c, chosen, onPick }) { +function Candidate({ c, chosen, onPick, ordinal }) { return (