Skip to content

feat(ui): table keyboard navigation — arrow row highlight, Enter primary action, Esc clear (v1.20 F) - #69

Merged
argszero merged 1 commit into
mainfrom
feat/ui-ue-round5-keynav
Aug 17, 2026
Merged

argszero merged 1 commit into
mainfrom
feat/ui-ue-round5-keynav

Conversation

@argszero

Copy link
Copy Markdown
Owner

Summary

Item F of rant 2026-08-17T20:46:57 (UI/UE 第五轮多角度深化, v1.20): 数据表格键盘导航 — keyboard navigation for all data tables.

  • Tables covered (KBD_TABLE_IDS): 模型市场 #mk-body, 共享 #share-body, API Key #api-keys, 员工 #emp-body, 部门 #dept-body, 运营者 #ops-body (tbodies) + 交易记录 #tx-table (div wrapping a table)
  • Activate: click a row (container click delegation, skips .mk-detail expand rows), or just press ↑/↓ — kbdContainerFrom(t) resolves the current table via closest("tbody") (id match) or closest("table").parentNode (#tx-table), falling back to the last active table
  • Keys:
    • ArrowDown/ArrowUpkbdMove() moves the .row-active highlight (accent left bar inset 3px 0 0 + --accent-soft background), scrollIntoView({block:"nearest"}); when nothing is active, ↓ starts at the first row, ↑ at the last
    • EnterkbdEnter() clicks the row's primary action — the first enabled button.btn:not(.row-expand) (marketplace=使用/消费, API keys=复制, departments=编辑, operator=充值…); disabled buttons not triggered
    • EsckbdClear() clears the highlight (falls through to existing Esc logic — help panel, inline forms — when nothing is highlighted)
  • Guards: typing (INPUT/TEXTAREA/SELECT/contentEditable) and meta/ctrl/alt combos are not intercepted; existing priorities (? help, 1-7 view switch, Esc: tour > help > inline-new-key > table highlight) are preserved

Rant reference (verbatim)

F. 数据表格键盘导航 — ↑/↓ 行高亮 .row-active,Enter 主操作,Esc 清除

Verification

  • DOM-stub smoke test: 16/16 assertions (click activates → ArrowDown skips mk-detail row → ArrowUp back → Enter clicks primary → disabled button not triggered → Esc clears → no-active no-op → #tx-table container resolution both via arrows and click)
  • Real bug found & fixed during smoke: kbdMove used stale kbd.c instead of the resolved container — arrows now work in a table before any row click
  • node --check ui/js/app.js ✓, CSS braces balanced ✓
  • cargo test ✓ (1 passed), cargo fmt --check
  • Docs: docs/user-stories.md v1.20 entry (F added), ui/README.md「数据表格键盘导航约定」section

Progress on rant 20:46:57: A (#63) + B (#65) + C (#66) + D (#67) + E (#68) + F (#69) merged; remaining G.

@argszero
argszero merged commit a0cd005 into main Aug 17, 2026
1 check passed
argszero added a commit that referenced this pull request Sep 13, 2026
## Summary

Two hand-written rosters in `ui/js/app.js` enumerated the "live" containers by id, and **both omitted the admin models table `#model-body`** — which was added later (PR #97) while the rosters date from PR #69/#79:

1. **`bindLiveRetry("<id>", fn)`** (13 entries, bound once at `DOMContentLoaded`) attached the load-error 重试 button of each container. `#model-body` was never registered, so `renderAdminModels()` rendered a 重试 button that **did nothing when clicked**. The smoking gun is the renderer itself: `loadErrorHtml(emptyLabel, retryFn, retryLabel)` declared a `retryFn` parameter and **never used it** — the renderer was always meant to hand its loader over, and the roster was the only thing wiring it up.
2. **`KBD_TABLE_IDS`** listed the tables reachable by ↑/↓/Enter. `#model-body` was absent, so its rows could not be activated (no `.row-active`) while all seven sibling tables could.

The fix **removes both rosters instead of extending them** — a roster that must be maintained by hand is the defect, not the one missing entry. Both containers are now derived from the DOM, so no future table can be forgotten:

| | Before | After |
|---|---|---|
| retry wiring | `bindLiveRetry(id, fn)` ×13 + per-container `addEventListener` | `setLiveError(container, html, loader)` — the renderer hands over its loader; one-time container delegation (`WeakMap`/`WeakSet`) dispatches `[data-live-retry]` |
| kbd container | `KBD_TABLE_IDS.indexOf(tb.id) >= 0` | `kbdTbodyOf(t) = t.closest("tbody")`, via a single **document-level** click delegation |

Runtime evidence (a jsdom probe booting the real `index.html` + the four scripts, only `fetch` stubbed so the admin endpoints 500):

| check | pre-change | post-change |
|---|---|---|
| `#dept-body` retry re-requests (control) | 5 | 5 |
| `#model-body` retry re-requests (test) | **0** | 5 |
| `#model-body` second retry click | **0** | 5 |
| `#dept-body` row click activates (control) | true | true |
| `#model-body` row click activates (test) | **false** | true |
| raise-requests rows activate (declared side effect) | **false** | true |
| a run-time-injected table's rows activate (no roster can cover it) | **false** | true |
| **verdict** | **DEFECT — 5/7 failed** | **OK — 7/7** |

## Related Issue

None — the repository carries no open issues. Found by reconciling the two enumerated rosters against the tables that actually exist.

## Changes

- [x] `ui/js/app.js`
  - `setLiveError(container, html, loader)` replaces the deleted `bindLiveRetry`; the dead `retryFn` parameter is dropped from `loadErrorHtml`. All **12** `loadErrorHtml`/`loadErrorRow` call sites now pass their loader through it.
  - `KBD_TABLE_IDS` deleted; the keyboard container is derived from the event target (`kbdTbodyOf` = `closest("tbody")`) and the row/table click delegations are merged into a single document-level listener.
  - `kbdRows`/`kbdContainerFrom` treat a container that has been rebuilt (`isConnected === false`) as "no active table" instead of highlighting rows that are gone.
- [x] `ui/index.html` — cache-bust `js/app.js?v=20260913-2` → `-3`.
- [x] `ui/README.md` — the keyboard-nav and degradation-pattern sections named both deleted rosters; they now describe the DOM-derived contract.
- [ ] 涉及配置/数据结构的改动已同步示例文件 — **N/A**: no config or data-structure change.

**Declared side effect** (a consequence of "every data table is navigable"): the JS-built raise-requests table now has keyboard row navigation. It was outside the old roster *and* passed `null` for the retry callback, so its degraded state had no 重试 button at all; it now reloads through `loadAdmin()`.

## Tests

- [x] `cargo test` 全部通过 — **190 passed / 0 failed**, i.e. **unchanged** from `main` (this change is JS + docs only; the count staying put is the evidence that no Rust behaviour moved).
- [x] `cargo fmt --check` 通过
- [x] `cargo clippy --all-targets -- -D warnings` 通过
- [ ] 新增/更新了单元测试 — **N/A**: `ui/` carries no test harness in-repo (the probes live outside it), and the change's assertion is the jsdom A/B above: with the pre-change `app.js` the probe is **red (5/7)**, with this branch it is **green (7/7)**, and the `#dept-body` control passes in both legs.

## Checklist

- [x] 分支命名符合约定 — `fix/ui-live-container-rosters`
- [x] Commit message 使用 Conventional Commits 格式 — `fix(ui): derive the live containers from the DOM, not from rosters`
- [x] 单一职责,改动最小化 — one defect class (enumerated live containers), no opportunistic refactor; `node --check ui/js/app.js` clean.
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