fix(automations): make the 计划提醒 inspector two tab stops from any row - #2185
Conversation
|
Ran an external review over the two commits (Pi, Accepted — the suite could pass while the fix was broken. Giving the roving layout effect a dependency array is the obvious cleanup (its own comment warns against it), and every assertion still passed: the click path and the arrow path both move focus imperatively, so nothing observed the effect re-running. The new assertions check the tabindex after an arrow press and that Shift+Tab off the resize handle returns to the row the user left. Verified discriminating — with a dependency array added, this fails and nothing else does. The review proposed pinning it by toggling the 状态 filter instead. That does not work here: Accepted — comment was wrong. The test said the four fixture reminders are always created in the same millisecond. They are not; whether they tie is a matter of run timing, and each of the two resulting orderings is deterministic. Corrected. Accepted — announcement named a placement it does not own. It said 面板, but below 1024px the same content is a sheet that announces itself. The wording no longer names a placement. Also folded in: the effect read Deferred, with reason: clicking a row's non-button whitespace selects without focusing, so the tab stop can briefly lag the selection — vendor Full suite still green: 4/4 in |
98f3624 to
f0702ac
Compare
Every per-item action moved into the inspector, and the inspector renders after the whole list — so reaching it from row k of N cost N−k tab presses through rows that do nothing. A roving tabindex puts one stop on the list and gives arrows, Home and End back for moving inside it; Tab now leaves the list in one press from any row. Focus follows the user, not the selection: arrows move focus without opening the inspector, so browsing the list stays browsing.
Selecting a row moves no focus — a mouse user did not ask to leave the list — so a screen reader got no signal that a panel had opened beside it. A polite status line names the task whose details are now showing, in both placements: the side panel and the narrow-window sheet.
The suite could go green on the most likely regression. Both the click path and the arrow path move focus imperatively, so giving the layout effect a dependency array — the obvious cleanup, and the one its comment warns against — left every assertion passing while rows mounted after the first render kept their default tab stop. Assert the attribute after an arrow press, and that Shift+Tab off the resize handle returns to the row the user left. Verified discriminating: with a dependency array added, this fails and nothing else does. Also drop the ref the effect read through — with no dependency array the render closure already holds the current index — and stop naming a placement in the announcement, which is a sheet below the breakpoint.
ecd3ac2 to
3518569
Compare
1 participant
Summary
#2155 made every 计划提醒 row an inert selector and moved the per-item actions (启用/暂停、立即触发、延后、编辑、复制、清空记录、删除) into the right-side inspector, per Astryx's rule that interactive elements must not sit inside an interactive list item. That is right, but it left the keyboard path worse than before: each row is its own tab stop, and Astryx
Layoutrenders header → content → end, so the inspector comes after the whole list. Selecting row k of N and reaching the inspector's first control costs N−k presses through rows that do nothing, plus the resize handle'sseparatorstop. Before #2155 the path was direct — Tab landed on the row's own switch or menu.Two changes:
use-roving-row-focus.ts) keeps exactly one row in the tab order and gives ArrowUp/ArrowDown/Home/End back for moving inside the list. Tab now leaves the list in one press from any row, so the inspector is two stops away regardless of list length or position. The tabindex is written onto the design system's own invisible row buttons in a layout effect — the same DOM seam refactor(automations): rebuild 定时任务 on Astryx's tracker archetype #2155 already reaches through to restore focus after a delete — becauseListItemtakes notabIndexfor that button.The resize handle keeps its tab stop: it is a real keyboard control (arrows resize the panel), not a dead one.
Verification
npm run format:check/npm run lint— cleannpm --workspace @maka/ui run typecheck,npm --workspace @maka/desktop run typecheck(four tsconfigs) — passnpm --workspace @maka/ui run test— 323 passnpm --workspace @maka/desktop run test— 1685 passnode scripts/check-dead-css.mjs --check,node scripts/check-story-annotations.mjs— passnpm --workspace @maka/desktop run smoke:storybook— 92 manifest checks + 84 catalog renders passplan-reminders.spec.ts— all eight pass, including the focus contracts refactor(automations): rebuild 定时任务 on Astryx's tracker archetype #2155 pinned (post-delete focus, edit-dialog Escape restore, narrow-window sheet) and the seeded-order contract from fix(desktop): seed plan reminders with distinct createdAt #2186.The new assertion counts the tab stops from the first row, which still has seven rows after it: Tab → the resize handle, Tab → the inspector's first control, and the rows below hold
tabindex="-1". It also presses ArrowDown and re-checks, so the stop has to move with the arrows rather than be written once on mount. Confirmed load-bearing — with the roving props removed from the list container it fails on the first Tab, which lands on the next row instead of the handle.