Settings: point a failed save at the field that caused it #272

Merged

robbertbos merged 16 commits from settings-field-errors into main 2026-08-09 12:52:07 +00:00

Conversation 0 Commits 16 Files changed 26 +862 -250

robbertbos commented 2026-08-08 22:10:50 +00:00

Owner

Copy link

Works through the actionable part of #208, after re-verifying its claims
against NLDD 0.8.79. The issue was written against 0.8.70; four of the
mechanisms it cites have since moved, so it is being rewritten separately.

The defect this starts from

POST /api/accounts answered 422 for three unrelated causes: Pydantic
validation (token shape, and the SSRF guard on base_url), an invalid
Mattermost token, and an unreachable server. The panel mapped every 422 to
one sentence, so pasting http://10.0.0.5/chat as Basis-URL reported
"Token is ongeldig" on a token that was fine.

#208 proposed routing 422 to the Token field. Done as written that would have
painted a correct token red for two of the three causes.

What changed

The statuses split. The catch-all in _verify_token is now 502: the
request was fine, the upstream was not reachable. 422 keeps its one meaning.

One pure function decides field versus banner. accountErrorTarget maps
an error onto a field and a sentence, and always returns a target: an error
carrying no status - a network failure, an abort, a thrown TypeError - is
panel-level rather than nothing. That was the objection to the mapping #208
proposed, which returned null for exactly those.

Cause	Lands on
422, loc names base_url	Basis-URL
422, otherwise	Token
409	Basis-URL
502	Basis-URL
400, or no status at all	the panel banner

The recovery-code field joins the form system. It was the last hand-written
<label> plus <input> in Instellingen, and its likeliest failure landed in
the panel-wide banner while focus jumped to a field showing nothing.

The last two dialogs move onto Modal.vue. The disconnect and delete
confirms were the only ones still on nldd-modal-dialog; the heading-size
drift #208 spotted (3 against 5) was a symptom of that, so it resolves itself.

The delete button says why it is off. Three conditions gated it and none
was announced. A mismatched address now says so, on blur rather than per
keystroke.

Thema is a radio group. The choice directly below it already was one, so
one panel offered the same kind of choice two ways. Sync-interval and both
retention fields keep dropdowns: five options with long labels.

Settled without code

• The <output aria-label> in RecoveryKeyModal. Measured in Chromium's
  accessibility tree: the label becomes the accessible name, and the key stays
  exposed as a StaticText child. The feared "the label replaces the key" does
  not happen.
• nldd-table for the import diff table. It fits mechanically, but the
  family has no <table>, <th scope> or <caption> anywhere - it is a CSS
  grid with role="table"/"columnheader"/"cell". The current markup is a
  native table with real <th scope="col">, so adopting it would trade
  semantics for roles.

Two traps worth knowing

• An NLDD error text is invisible unless the input carries both invalid
  and an error-message naming the text's id. Half of it renders nothing, with
  no warning. The base-URL and label fields moved into
  MattermostServerFields so that recipe exists once.
• A false boolean binding on an NLDD element is not nothing: on an element
  that has not upgraded, Vue writes the attribute "false", and NLDD reads it
  with hasAttribute(). A stray invalid makes a form field show its error
  text permanently. Bindings are cond || undefined. Both are now in CLAUDE.md.

Verification

• 1726 backend tests, 100% coverage; 734 frontend unit tests; vue-tsc;
  npm run build; uvx pre-commit run --all-files; the full e2e suite
  (67 passed, 3 skipped).
• Driven in a browser, because a design-system change is not detectable by
  gates alone. Confirmed live: a rejected base URL and an unreachable server
  each land on the Basis-URL field with aria-describedby and aria-invalid
  reaching the real inner <input> and the token field untouched; the delete
  dialog opens as a true top-layer modal with focus on the first checkbox and
  the mismatch error appearing only after blur; zero nldd-modal-dialog
  elements left in the app.

One e2e spec caught what the unit tests could not - it still drove Thema as a
combobox. It now addresses the radio by its accessible name.

Works through the actionable part of #208, after re-verifying its claims against NLDD 0.8.79. The issue was written against 0.8.70; four of the mechanisms it cites have since moved, so it is being rewritten separately. ## The defect this starts from `POST /api/accounts` answered 422 for three unrelated causes: Pydantic validation (token shape, and the SSRF guard on `base_url`), an invalid Mattermost token, and an unreachable server. The panel mapped every 422 to one sentence, so pasting `http://10.0.0.5/chat` as Basis-URL reported **"Token is ongeldig"** on a token that was fine. #208 proposed routing 422 to the Token field. Done as written that would have painted a correct token red for two of the three causes. ## What changed **The statuses split.** The catch-all in `_verify_token` is now 502: the request was fine, the upstream was not reachable. 422 keeps its one meaning. **One pure function decides field versus banner.** `accountErrorTarget` maps an error onto a field and a sentence, and always returns a target: an error carrying no status - a network failure, an abort, a thrown `TypeError` - is panel-level rather than nothing. That was the objection to the mapping #208 proposed, which returned `null` for exactly those. | Cause | Lands on | |---|---| | 422, `loc` names `base_url` | Basis-URL | | 422, otherwise | Token | | 409 | Basis-URL | | 502 | Basis-URL | | 400, or no status at all | the panel banner | **The recovery-code field joins the form system.** It was the last hand-written `<label>` plus `<input>` in Instellingen, and its likeliest failure landed in the panel-wide banner while focus jumped to a field showing nothing. **The last two dialogs move onto `Modal.vue`.** The disconnect and delete confirms were the only ones still on `nldd-modal-dialog`; the heading-size drift #208 spotted (3 against 5) was a symptom of that, so it resolves itself. **The delete button says why it is off.** Three conditions gated it and none was announced. A mismatched address now says so, on blur rather than per keystroke. **Thema is a radio group.** The choice directly below it already was one, so one panel offered the same kind of choice two ways. Sync-interval and both retention fields keep dropdowns: five options with long labels. ## Settled without code - **The `<output aria-label>` in RecoveryKeyModal.** Measured in Chromium's accessibility tree: the label becomes the accessible name, and the key stays exposed as a `StaticText` child. The feared "the label replaces the key" does not happen. - **`nldd-table` for the import diff table.** It fits mechanically, but the family has no `<table>`, `<th scope>` or `<caption>` anywhere - it is a CSS grid with `role="table"`/`"columnheader"`/`"cell"`. The current markup is a native table with real `<th scope="col">`, so adopting it would trade semantics for roles. ## Two traps worth knowing - An NLDD error text is invisible unless the input carries **both** `invalid` and an `error-message` naming the text's id. Half of it renders nothing, with no warning. The base-URL and label fields moved into `MattermostServerFields` so that recipe exists once. - A **`false` boolean binding on an NLDD element is not nothing**: on an element that has not upgraded, Vue writes the attribute `"false"`, and NLDD reads it with `hasAttribute()`. A stray `invalid` makes a form field show its error text permanently. Bindings are `cond || undefined`. Both are now in CLAUDE.md. ## Verification - 1726 backend tests, 100% coverage; 734 frontend unit tests; `vue-tsc`; `npm run build`; `uvx pre-commit run --all-files`; the full e2e suite (67 passed, 3 skipped). - Driven in a browser, because a design-system change is not detectable by gates alone. Confirmed live: a rejected base URL and an unreachable server each land on the Basis-URL field with `aria-describedby` and `aria-invalid` reaching the real inner `<input>` and the token field untouched; the delete dialog opens as a true top-layer modal with focus on the first checkbox and the mismatch error appearing only after blur; zero `nldd-modal-dialog` elements left in the app. One e2e spec caught what the unit tests could not - it still drove Thema as a combobox. It now addresses the radio by its accessible name.

robbertbos added 7 commits 2026-08-08 22:10:50 +00:00

Tell an unreachable server apart from an invalid token ... 39bccfd5ea

Verifying a Mattermost token returned 422 for both "this token is not
valid" and "we could not reach this server at all", so no caller could
distinguish them. Pydantic's own validation - the token shape and the
SSRF guard on base_url - is a third 422 on the same endpoints.

The catch-all becomes 502: the request was fine, the upstream was not
reachable. The detail stays sanitised to the exception class name.

Express the theme choice the way the panel already does ... 6d8d7a1491

Thema was a dropdown while the choice directly below it - browser or
desktop app - was a radio group, so one panel offered the same kind of
choice two ways. Three short options fit a radio group, and it needs no
component Waggle does not already use.

Sync-interval and both retention fields keep their dropdowns: five
options with long labels.

The load-bearing aria-label the <select> needed does not apply here.
form-field writes aria-label onto the slotted element when it has no
inputId, and for a radio group that element is the radiogroup itself -
for a dropdown it is a wrapper, with the real <select> two levels down.

Put the recovery-code error on the recovery-code field ... 9c75c9423b

The import wizard's recovery-code input was the last hand-written label
and <input> in Instellingen; everything else moved to NLDD's form system.
Its likeliest failure - a code that does not decrypt - landed in the
panel-wide banner while focus jumped to a field showing nothing at all.

It is now nldd-form-field plus nldd-password-field, and the two
recovery-code problem types render at the field. Every other import
error keeps the banner.

form-field preserves a consumer-set input-id, so the id stays on the
inner native input and the e2e selector still resolves.

Point a failed server save at the field that caused it ... 4f96f34b54

A failed save put one message in a banner under all three fields, and
that message named the token whatever had gone wrong: the endpoint
answered 422 for an invalid token, for a base URL the SSRF guard
rejected, and for a server it could not reach. Pasting an http:// URL
reported an invalid token on a token that was fine.

accountErrorTarget maps an error onto a field and a sentence. It always
returns a target: an error carrying no status - a network failure, an
abort, a thrown TypeError - is panel-level rather than nothing, which
is what a status-only mapping would have made it.

The base URL and label fields move into MattermostServerFields, so the
wiring that makes an NLDD error text visible - `invalid` on the input
plus an `error-message` naming the text's id, both or neither - exists
once. Getting half of it right renders nothing, silently.

Booleans bound into that wiring are `cond || undefined`, never false: on
an element that has not upgraded Vue writes the attribute, and NLDD
reads it with hasAttribute(), for which "false" is true.

Move the last two dialogs onto the shared shell, and say why the ... 1fd26995b3

delete button is off

The disconnect and delete confirms were the only dialogs still on
nldd-modal-dialog after the shell moved to nldd-window; both now use
Modal.vue, so the backdrop, the focus trap and focus restore come from
the browser here too. Their heading sizes differed - 3 against 5 - which
was a symptom of sitting outside the shell rather than a design choice,
so they take the shell's own.

The delete confirm gated its button on three conditions and announced
none of them: an address that does not match now says so, on blur rather
than per keystroke, since every address mismatches while it is typed.

Record what this change taught, and correct what it disproved ... 7e943feaf1

The NLDD_TAGS note claimed an unregistered tag makes Vue warn. It does
not: vitest.config.ts declares every nldd-* tag a custom element, so the
warning never fires and the list is about stubs that need behaviour.

New in its place: a false boolean binding on an NLDD element is not
nothing. It becomes the attribute "false", which hasAttribute() reads as
true - enough to make a form field show its error text permanently.

Drive the theme choice by role instead of by tag ... d2839b2804

The spec picked the theme control as a combobox and its options by
value attribute. Both were markup details; the radio and its label are
what a user of assistive technology actually addresses.

robbertbos referenced this pull request 2026-08-08 22:12:03 +00:00

Settings/NLDD: what was evaluated and not made, and when it expires #208

robbertbos added 1 commit 2026-08-08 22:18:04 +00:00

Fail the build on a boolean binding that reaches the DOM as "false" ... cfb26eb007

Vue writes setAttribute(key, "false") for a false binding on an element
that has not upgraded, and NLDD reads these with hasAttribute(), for
which "false" is true. On `invalid` that makes nldd-form-field reveal
its error text permanently, and nothing warns: typecheck cannot see it,
and a unit test reads "false" off the stub either way.

Same drawer as nlddTokens.test.ts and iconNames.test.ts. The finder is
proven on input that should match before it is trusted on input that
should not.

robbertbos added 1 commit 2026-08-08 22:36:10 +00:00

Wait for the settings save before reloading to check it stuck ... a058f79241

Both Voorkeuren specs changed a setting and reloaded straight after. The
panel has no submit button, so the PUT is still in flight when the action
resolves and the reload can beat it - green on a fast machine, red on a
loaded runner.

Reproduced by delaying the PUT by 2.5s: the theme assertion fails, and
passes again once the spec waits for the response. The interval one
survived the same delay, but has the same shape and gets the same wait.

robbertbos added 1 commit 2026-08-09 11:35:03 +00:00

Write the changelog entry in English ... 0065576e8e

WDR-0003 put CHANGELOG.md in English; only quoted UI strings stay Dutch.
This section was written entirely in Dutch, following the convention the
record superseded.

robbertbos added 1 commit 2026-08-09 11:41:14 +00:00

Keep an upstream 4xx a 422, not a gateway failure ... 7834b522fb

verify_token only translates 401 into MattermostTokenInvalidError; every
other non-2xx goes through raise_for_status(). So a 403 on a token that
lacks the rights, and a 404 on a URL that is not a Mattermost server,
landed in the catch-all and reported 502 - telling the caller the server
was unreachable when the server had answered.

Split on who is at fault: an upstream 4xx is about what we sent and
stays 422; a 5xx, or no answer at all, is the gateway.

robbertbos added 2 commits 2026-08-09 11:57:36 +00:00

Put the disconnect confirm back on NLDD's message dialog ... 04051cd8f3

A question, a sentence and two buttons is what nldd-modal-dialog is for;
it was moved to Modal.vue for consistency, which is the wrong trade when
the design system already has the shape.

ConfirmDialog wraps it with the two things NLDD cannot know about: the
overlay-stack registration - without which the single-key shortcuts stay
live behind an open dialog, because the <dialog> sits in a shadow root
that document.querySelector cannot reach - and the scroll lock.

Modal.vue keeps its hand-written header, with the nldd-page attempt and
its two costs recorded there.

Let ConfirmDialog's children carry the native slot attribute ... ea7948d12b

Its default slot renders straight into nldd-modal-dialog, so the buttons
are that component's light-DOM children and slot="actions" is the right
HTML - the same reason the rule already ignores nldd-* parents.

robbertbos added 1 commit 2026-08-09 12:10:02 +00:00

Show the sync interval as radio buttons, without the ten-minute step ... 96465127dd

Sync-interval was the last dropdown in Voorkeuren while the two choices
beside it were radio groups. Four options fit, so every one is visible
without opening anything.

The API takes any value from 60 to 3600, so a stored interval need not
be a preset - an older choice, or an imported bundle. Rather than leave
the group with nothing checked and no way back, the stored value is
rendered alongside the presets.

robbertbos added 1 commit 2026-08-09 12:32:41 +00:00

Drop the thirty-minute step from the sync interval ... ff08dc865b

Three options, the same as Thema beside it. A saved interval that is no
longer a preset still renders, so an older 10- or 30-minute choice stays
selectable.

robbertbos force-pushed settings-field-errors from ff08dc865b to 2a78b1849e 2026-08-09 12:49:22 +00:00 Compare

robbertbos merged commit 8c8e80d502 into main 2026-08-09 12:52:07 +00:00

robbertbos deleted branch settings-field-errors 2026-08-09 12:52:07 +00:00

robbertbos referenced this pull request 2026-08-09 13:58:25 +00:00

Settings/NLDD: what was evaluated and not made, and when it expires #208

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bug

Something is not working

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

help wanted

Need some help

  

invalid

Something is wrong

  

nldd ds

  

question

More information is needed

  

wontfix

This won't be fixed

No labels

bug

duplicate

enhancement

help wanted

invalid

nldd ds

question

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

robbertbos/waggle!272

No description provided.