fix(ui): derive the live containers from the DOM, not from rosters - #204
Merged
Merged
Conversation
Two hand-written rosters enumerated the "live" containers by id, and both
omitted the admin models table `#model-body`:
1. `bindLiveRetry("<id>", fn)` bound the load-error 重试 button of 13
containers. `#model-body` was never registered, so `renderAdminModels()`
rendered a 重试 button that did nothing when clicked. The smoking gun is
`loadErrorHtml(emptyLabel, retryFn, retryLabel)`: it declared `retryFn` and
never used it — the renderer was always meant to hand its loader over.
2. `KBD_TABLE_IDS` listed the tables reachable by ↑/↓/Enter. `#model-body`
was absent, so its rows could not be activated (no `.row-active`).
Both rosters are removed rather than extended — a roster that must be
maintained by hand is the defect, not the missing entry:
- `setLiveError(container, html, loader)` replaces `bindLiveRetry` and the
unused `retryFn` parameter. Whoever renders the degraded state hands over
its loader; a one-time, container-level click delegation (WeakMap/WeakSet)
dispatches `[data-live-retry]` clicks, so a rebuilt container keeps working.
All 12 `loadErrorHtml`/`loadErrorRow` call sites were converted.
- The keyboard-nav container is derived from the event target
(`kbdTbodyOf` = `closest("tbody")`) instead of matched against a list of
ids, via a single document-level click delegation. Any data table is now
navigable without being registered, and dynamically rebuilt tables keep
their handler.
Declared side effect: the JS-built raise-requests table gains keyboard row
navigation (it was outside the old roster and its degraded state passed
`null` for the retry callback, so it had no retry button either; it now
reloads via `loadAdmin()`).
Docs: `ui/README.md` described both deleted rosters by name; the two sections
are updated to the DOM-derived contract.
Verified with a jsdom probe booting the real `index.html` + the four scripts
(fetch stubbed, admin endpoints 500): before the change `#model-body` renders
but re-requests 0 times on retry and never activates on click (5/7 checks
red); after, all 7 pass, control `#dept-body` passes in both legs, and a
run-time-injected table's rows activate (no roster can cover it).
`cargo fmt --check`, `cargo clippy --all-targets -- -D warnings` and
`cargo test` (190 passed) all clean; cache-bust bumped to `app.js?v=20260913-3`.
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
Two hand-written rosters in
ui/js/app.jsenumerated 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:bindLiveRetry("<id>", fn)(13 entries, bound once atDOMContentLoaded) attached the load-error 重试 button of each container.#model-bodywas never registered, sorenderAdminModels()rendered a 重试 button that did nothing when clicked. The smoking gun is the renderer itself:loadErrorHtml(emptyLabel, retryFn, retryLabel)declared aretryFnparameter and never used it — the renderer was always meant to hand its loader over, and the roster was the only thing wiring it up.KBD_TABLE_IDSlisted the tables reachable by ↑/↓/Enter.#model-bodywas 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:
bindLiveRetry(id, fn)×13 + per-containeraddEventListenersetLiveError(container, html, loader)— the renderer hands over its loader; one-time container delegation (WeakMap/WeakSet) dispatches[data-live-retry]KBD_TABLE_IDS.indexOf(tb.id) >= 0kbdTbodyOf(t) = t.closest("tbody"), via a single document-level click delegationRuntime evidence (a jsdom probe booting the real
index.html+ the four scripts, onlyfetchstubbed so the admin endpoints 500):#dept-bodyretry re-requests (control)#model-bodyretry re-requests (test)#model-bodysecond retry click#dept-bodyrow click activates (control)#model-bodyrow click activates (test)Related Issue
None — the repository carries no open issues. Found by reconciling the two enumerated rosters against the tables that actually exist.
Changes
ui/js/app.jssetLiveError(container, html, loader)replaces the deletedbindLiveRetry; the deadretryFnparameter is dropped fromloadErrorHtml. All 12loadErrorHtml/loadErrorRowcall sites now pass their loader through it.KBD_TABLE_IDSdeleted; 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/kbdContainerFromtreat a container that has been rebuilt (isConnected === false) as "no active table" instead of highlighting rows that are gone.ui/index.html— cache-bustjs/app.js?v=20260913-2→-3.ui/README.md— the keyboard-nav and degradation-pattern sections named both deleted rosters; they now describe the DOM-derived contract.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
nullfor the retry callback, so its degraded state had no 重试 button at all; it now reloads throughloadAdmin().Tests
cargo test全部通过 — 190 passed / 0 failed, i.e. unchanged frommain(this change is JS + docs only; the count staying put is the evidence that no Rust behaviour moved).cargo fmt --check通过cargo clippy --all-targets -- -D warnings通过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-changeapp.jsthe probe is red (5/7), with this branch it is green (7/7), and the#dept-bodycontrol passes in both legs.Checklist
fix/ui-live-container-rostersfix(ui): derive the live containers from the DOM, not from rostersnode --check ui/js/app.jsclean.