fix(ui): make the settings controls either wired or explicitly inert - #263
Merged
Merged
Conversation
The three configuration cards in Settings (account / notifications / preferences) each render a control that looks operable but has no consumer anywhere: - `#settings-nickname`: editable, filled from the real `/api/me`, but there is no write path for a nickname at all -- `UPDATE users` only touches `dept_id` / `verified` / `password_hash` -- and no listener, so an edit is silently discarded on the next `renderSettings`. Its own card sibling `#settings-email` is already `readonly` with a hint: commit 09e4121 (#157) did both for the email and missed the nickname. - `#prefs-model`: filled from the real `/api/models`, but no listener, no storage key and no reader (`openChat` is only ever called with an explicit model). Its three card siblings (lang / theme / density) all persist and take effect. - the three notification toggles: rendered *checked*, with no id, no name and no reader -- the repository has no notification subsystem at all. The fix is not "make them work" (that needs a backend route and a consumer that do not exist); it is the repository's own idiom for "capability not open" (`#withdraw-btn`: `disabled` plus a short note): declare the control inert and say why, in both language packs. Wired siblings are left untouched. - `#settings-nickname` and `#prefs-model` get `readonly` / `disabled` plus a hint; the three notification toggles get `disabled` plus a card-level hint (they also lose the misleading `checked`). - Three new i18n keys, both packs. - New static gate `state_gate::settings_controls_are_either_live_or_marked_inert`, scoped explicitly to those three cards: a control that is not consumed must be marked inert, a control that is consumed must not be, and a card holding an inert control must carry a hint whose key exists in both packs. - Companion extractor self-test: "filled" is not "consumed", and neither is "persisted" -- writing the value to `localStorage` with no reader is the half-fix this gate rejects. - `ui/README.md` convention section; `ui/index.html` cache-bust `i18n.js?v=...-4`; `src/i18n_pack.rs` positive-control constants recalibrated to the values the gates reported.
Owner
Author
Self-reviewWhat this change is: it removes a promise the UI was making and could not keep. All Evidence the direction is right, not merely the diff consistent
Honest limits of the gate
Not touched, deliberately
|
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 three configuration cards in Settings (account / notifications / preferences)
each render a control that looks operable but has no consumer anywhere:
#settings-nickname/api/me, but there is no write path for a nickname —UPDATE usersonly touchesdept_id/verified/password_hash— and no listener, so an edit is silently discarded on the nextrenderSettings.#prefs-model/api/modelsand it looks selectable, but there is no listener, no storage key and no reader (openChat()is only ever called with an explicit model).id, nonameand no reader — the repository has no notification subsystem at all.None of them is a missing feature in disguise: each family can point at the half it
lacks in its own card or elsewhere in the repo, which is what makes this drift
rather than a choice.
#settings-emailis alreadyreadonlywith a hint — commit09e4121(feat(ui): redesign settings/admin/ops views on the landed component layer #157, the same redesign) deleted the dead "save" button next to it, marked the
email inert, and missed the nickname.
#prefs-lang→atp_lang,#prefs-theme→atp-theme, density radios →atp-density) all persist andtake effect.
#withdraw-btn:disabledplus a short note.The fix is therefore not "make them work" — that would need a backend route and a
consumer that do not exist — it is: declare the control inert and say why, in both
language packs.
Changes
#settings-nickname→readonly+ a hint;#prefs-model→disabled+ a hint;the three notification toggles →
disabled+ a card-level hint (they also lose themisleading
checked). Their fill sites are untouched: the nickname still shows thereal
/api/mevalue and the dropdown is still sourced from the real catalogue.state_gate::settings_controls_are_either_live_or_marked_inert, with the scopeexplicitly registered as those three cards (the API-key card is out of scope —
its wiring already has its own gates — so the gate needs no exemption list):
inert ⟺ ¬consumed, both directions — an unwired control must be markedinert (this catches all three faces), and a wired control must not be
(this catches the "just disable everything" over-correction, which would kill
lang / theme / density).
hint, and that key must exist inboth packs (a missing pack shows the raw key name — worse than no hint).
the_settings_control_extractors_have_teeth: "filled" is not "consumed" (ref#338), and neither is "persisted" — a listener that only writes the value into
localStoragehas no reader at all, so it is exactly the half-fix this axis mustreject. It also pins the identifier-token boundary (
themeSel.valueis notsel.value, ref #333), the radio-groupnamehandle path, the button case, and"a control with no
id/namecan never be recognised, so it is never wired" —that last rule is what holds the three toggles.
ui/README.mdconvention section;ui/index.htmlcache-busti18n.js?v=…-4;src/i18n_pack.rspositive-control constants recalibrated to the values the gatesreported (not hand-computed).
Related Issue
None.
Tests
cargo test— 290 → 292 passed, 0 failedcargo fmt --checkcleancargo clippy --all-targets— only the pre-existingprotocol.rs:662warningsrc/state_gate.rs), with a self-verifying extractorA/B, gate (in-place mutation, restored byte-exactly, md5 verified each leg)
tmp/c2148_gate_ab.py— 6/6 legs as declared:settings-nicknameis unmarked and unwired#prefs-langalsodisabled)#prefs-modelre-enabled + listener that only writeslocalStorage)The unfixed tree also goes red on the axis rule only (the full
state_gate::runreports exactly one failing test), so the gate is not carried by an unrelated rule.
A/B, probe (
tmp/c2148_probe.js, jsdom booting the realindex.html+ the four real scripts, stubbing and loggingfetch, driving the real controls)4/4 legs as declared:
index.html) — exactly the 7 axis legs red, all 7 precondition /control legs green.
control leg
D1.localStorageonly) — rejected by axisleg
A2.The probe pins the screen-side facts (the fields still carry the real values, the
dropdown is still sourced from the real catalogue, the hints are localized — switched
via the real language control, no CJK left in the en pack). CI has no JS runner, so the
gate above is what keeps the convention.