Fix verified UI defects (dialogs, keyboard access, upload errors, duplicate toasts) #593

New issue

Closed

opened 2026-10-15 20:18:38 +02:00 by simon · 0 comments

simon commented

2026-10-15 20:18:38 +02:00

Owner

Motivation

The UI audit for #402 found 13 defects plus 4 incidental ones and verified them in a headless browser against main @ d9093a1f. They hurt users today (a LiveView crash, Escape closing the wrong dialog, uploads failing silently, controls that only work with a mouse) and must not wait for the UI standard. Each gets a minimal, tested fix here. Where a later pattern issue (dialogs, combobox, sortable list) replaces the code structurally, the minimal fix still lands now so users do not wait.

Visible improvement

Dialogs close with Escape and return focus; upload errors are shown; no duplicate toasts; every control on the affected pages works with the keyboard and shows focus.

Scope (ordered steps, `just ci-dev` green after each)

Each item: browser finding · minimal fix · regression test.

B1 — Data field delete dialog crashes the LiveView. The dialog has phx-keydown="dialog_keydown" without phx-target; DatafieldsLive has no matching clause. Browser: confirmed with a precondition: the crash (FunctionClauseError in DatafieldsLive.handle_event/3, remount, dialog gone) happens only when the <dialog> element itself has focus (e.g. after clicking its heading); typing in the slug input is harmless because LiveView only fires phx-keydown when e.target carries the attribute. Fix: add phx-target={@myself} and the handler in the component. Test: render_keydown on the dialog with Escape closes it and the LiveView stays alive.
B2 — Escape in the four fee dialogs closes the member's (hidden) delete dialog instead. No phx-target on the fee dialogs; the event reaches MemberLive.Show.dialog_keydown. Browser: confirmed: Escape never closes a fee dialog; with the dialog focused, focus jumps to #delete-member-trigger. Fix: target the fees component and handle Escape there. Test: Escape on each fee dialog closes that dialog and leaves the delete dialog closed.
B3 / N4 — No focus return after the fee dialogs and the deactivate dialog. Browser: confirmed: focus lands on BODY (4 fee dialogs + deactivate, there also after Escape). Fix: stable trigger ids + FocusRestore push on close (legacy pattern, CODE_GUIDELINES §8.11). Test: closing pushes the focus_restore event with the trigger id.
B6 — “Add field” options in global settings are not keyboard-operable. div[role=option] tabindex=0 without key handling (WCAG 2.1.1). Browser: confirmed: Enter/Space/ArrowDown do nothing. Fix: render the options as <button type="button"> items (the menu-button pattern comes in U7). Test: the options are buttons with accessible names and trigger the add event.
B7 — The amount in the fees tab is a clickable <span>. Browser: confirmed: not reachable with Tab. Fix: render it as a <button> with an accessible name (“Edit amount for ‹cycle›”). Test: the element is a button and opens the edit dialog.
B8 — Users table sort button has no visible focus. focus:outline-none without replacement (WCAG 2.4.7). Browser: confirmed: :focus-visible applies but outline and box-shadow are none. Fix: use the shared button_focus_classes/0. Test: the rendered button carries the shared focus classes and no outline-none.
B4 / N3 — CSV upload errors are never shown; the label points nowhere. upload_errors/2 is never rendered; <label for="csv_file"> points at an id that live_file_input overrides. Browser: confirmed: a .txt file and an 11 MB .csv (limit 10 MB) give no message, only a disabled submit and a reset file selection. Fix: render upload errors below the field with aria-invalid; bind the label to the real input id. Test: uploading a wrong type and an oversized file renders the translated error; the label's for matches the input id.
B10 / N1 / N2 — User form member picker listens to every key on the page and reloads members on every change. phx-window-keydown without phx-key; validate reloads members on every change because Map.has_key?(user_params, "email") is always true; debounce also swallows fast arrow keys. Browser: confirmed: member_dropdown_keydown fires from the email field and from body; every validate reloads members; fast arrow presses get lost. Fix: bind keydown on the picker input only, without debounce on arrow keys; reload members only when the search text changes. Test: a validate without a search change performs no member read; keydown outside the picker sends no event.
B5 — Palette-colored boxes in the user form. Browser: partially confirmed: not unreadable (gray/orange are patched), but the blue/green/yellow boxes stay light with dark text in dark mode: glaring and inconsistent. Fix: replace the palette boxes with the interim callout markup (DESIGN_GUIDELINES §17.2). Test: the form renders no Tailwind palette classes.
B9 — Sidebar avatar initial is off-center. v4 class avatar placeholder. Browser: confirmed: the initial sits 4 px too high. Fix: avatar-placeholder. Test: the rendered avatar uses avatar-placeholder.
B12 — Link on the OIDC account-linking page is unstyled. text-brand does not exist. Browser: the page is unreachable without SSO; text-brand occurs 0× in the compiled CSS. Fix: link link-primary. Test: the link renders the link classes.
B11 — The email row of the join-form field order can be moved; announcements are English. locked_ids is missing; live-region texts are hard-coded English. Browser: confirmed: the email row moves by keyboard and drag, and the new order is saved. Fix: lock the email row (client and server) and translate the announcements via gettext. Test: a move request for the email row is rejected; announcements come from gettext.
B13 — Duplicate toasts on a full page load. Two flash groups (root layout and app layout). Browser: confirmed on full page loads: two identical toasts (flash-group-root + flash-group) and two live regions; a LiveView save shows one. Fix: render exactly one flash group (DESIGN_GUIDELINES §9.4). Test: a controller-rendered page with a flash contains exactly one flash group.

Out of scope

New components (<.modal>, <.combobox>, <.file_input>, menu button): U4, U7.
Redesign of the user form or the fees tab: U5, U4.

Acceptance criteria

Each defect above is fixed and covered by the named regression test.
DESIGN_GUIDELINES §5.6 (interaction states), §9.4 (one flash container), §10.3 interim rule (legacy dialogs), §18.3 (upload errors) are met on the touched pages.
Before/after evidence for B2/B3 (focus), B4 (error message), B5 (dark mode) and B13 (single toast) attached.
Keyboard check: every fixed control is reachable with Tab and operable with Enter/Space; focus is visible.
just ci-dev green after every step.

Dependencies

B2, B3 and B7 touch membership_fees_component.ex, which #536 (merged) changed: re-check their anchors before these steps.
No dependency on the UI pattern issues.

Anchors (verified against `main` @ `d9093a1f`, re-check before filing)

B1: lib/mv_web/live/custom_field_live/index_component.ex:112, lib/mv_web/live/datafields_live.ex:27
B2: lib/mv_web/live/member_live/show/membership_fees_component.ex:306,353,395,458, lib/mv_web/live/member_live/show.ex:524
B3/N4: the same fee dialogs; lib/mv_web/live/member_live/show/deactivate_component.ex:77
B4/N3: lib/mv_web/live/import_live/components.ex:79,90
B5: lib/mv_web/live/user_live/form.ex:113,166,174,192,213
B6: lib/mv_web/live/global_settings_live.ex:226
B7: lib/mv_web/live/member_live/show/membership_fees_component.ex:209
B8: lib/mv_web/components/table_components.ex:24
B9: lib/mv_web/components/layouts/sidebar.ex:318
B10/N1/N2: lib/mv_web/live/user_live/form.ex:232,482,816
B11: lib/mv_web/live/global_settings_live.ex:270, assets/js/app.js (sortable hook)
B12: lib/mv_web/live/auth/link_oidc_account_live.ex:293
B13: lib/mv_web/components/layouts/root.html.heex:72, lib/mv_web/components/layouts.ex:263

Caution

B1 only reproduces with the dialog element focused; a test that types into the slug input will not catch it.
B13: make sure the remaining flash group still covers controller-rendered pages (sign-in, join confirm).

## Motivation The UI audit for #402 found 13 defects plus 4 incidental ones and verified them in a headless browser against `main` @ `d9093a1f`. They hurt users today (a LiveView crash, Escape closing the wrong dialog, uploads failing silently, controls that only work with a mouse) and must not wait for the UI standard. Each gets a minimal, tested fix here. Where a later pattern issue (dialogs, combobox, sortable list) replaces the code structurally, the minimal fix still lands now so users do not wait. ## Visible improvement Dialogs close with Escape and return focus; upload errors are shown; no duplicate toasts; every control on the affected pages works with the keyboard and shows focus. ## Scope (ordered steps, `just ci-dev` green after each) Each item: browser finding · minimal fix · regression test. 1. **B1 — Data field delete dialog crashes the LiveView.** The dialog has `phx-keydown="dialog_keydown"` without `phx-target`; `DatafieldsLive` has no matching clause. *Browser:* confirmed **with a precondition**: the crash (`FunctionClauseError` in `DatafieldsLive.handle_event/3`, remount, dialog gone) happens only when the `<dialog>` element itself has focus (e.g. after clicking its heading); typing in the slug input is harmless because LiveView only fires `phx-keydown` when `e.target` carries the attribute. *Fix:* add `phx-target={@myself}` and the handler in the component. *Test:* `render_keydown` on the dialog with Escape closes it and the LiveView stays alive. 2. **B2 — Escape in the four fee dialogs closes the member's (hidden) delete dialog instead.** No `phx-target` on the fee dialogs; the event reaches `MemberLive.Show.dialog_keydown`. *Browser:* confirmed: Escape never closes a fee dialog; with the dialog focused, focus jumps to `#delete-member-trigger`. *Fix:* target the fees component and handle Escape there. *Test:* Escape on each fee dialog closes that dialog and leaves the delete dialog closed. 3. **B3 / N4 — No focus return after the fee dialogs and the deactivate dialog.** *Browser:* confirmed: focus lands on `BODY` (4 fee dialogs + deactivate, there also after Escape). *Fix:* stable trigger ids + `FocusRestore` push on close (legacy pattern, CODE_GUIDELINES §8.11). *Test:* closing pushes the `focus_restore` event with the trigger id. 4. **B6 — “Add field” options in global settings are not keyboard-operable.** `div[role=option] tabindex=0` without key handling (WCAG 2.1.1). *Browser:* confirmed: Enter/Space/ArrowDown do nothing. *Fix:* render the options as `<button type="button">` items (the menu-button pattern comes in U7). *Test:* the options are buttons with accessible names and trigger the add event. 5. **B7 — The amount in the fees tab is a clickable `<span>`.** *Browser:* confirmed: not reachable with Tab. *Fix:* render it as a `<button>` with an accessible name (“Edit amount for ‹cycle›”). *Test:* the element is a button and opens the edit dialog. 6. **B8 — Users table sort button has no visible focus.** `focus:outline-none` without replacement (WCAG 2.4.7). *Browser:* confirmed: `:focus-visible` applies but outline and box-shadow are none. *Fix:* use the shared `button_focus_classes/0`. *Test:* the rendered button carries the shared focus classes and no `outline-none`. 7. **B4 / N3 — CSV upload errors are never shown; the label points nowhere.** `upload_errors/2` is never rendered; `<label for="csv_file">` points at an id that `live_file_input` overrides. *Browser:* confirmed: a `.txt` file and an 11 MB `.csv` (limit 10 MB) give no message, only a disabled submit and a reset file selection. *Fix:* render upload errors below the field with `aria-invalid`; bind the label to the real input id. *Test:* uploading a wrong type and an oversized file renders the translated error; the label's `for` matches the input id. 8. **B10 / N1 / N2 — User form member picker listens to every key on the page and reloads members on every change.** `phx-window-keydown` without `phx-key`; `validate` reloads members on every change because `Map.has_key?(user_params, "email")` is always true; debounce also swallows fast arrow keys. *Browser:* confirmed: `member_dropdown_keydown` fires from the email field and from `body`; every `validate` reloads members; fast arrow presses get lost. *Fix:* bind keydown on the picker input only, without debounce on arrow keys; reload members only when the search text changes. *Test:* a `validate` without a search change performs no member read; keydown outside the picker sends no event. 9. **B5 — Palette-colored boxes in the user form.** *Browser:* **partially** confirmed: not unreadable (gray/orange are patched), but the blue/green/yellow boxes stay light with dark text in dark mode: glaring and inconsistent. *Fix:* replace the palette boxes with the interim callout markup (DESIGN_GUIDELINES §17.2). *Test:* the form renders no Tailwind palette classes. 10. **B9 — Sidebar avatar initial is off-center.** v4 class `avatar placeholder`. *Browser:* confirmed: the initial sits 4 px too high. *Fix:* `avatar-placeholder`. *Test:* the rendered avatar uses `avatar-placeholder`. 11. **B12 — Link on the OIDC account-linking page is unstyled.** `text-brand` does not exist. *Browser:* the page is unreachable without SSO; `text-brand` occurs 0× in the compiled CSS. *Fix:* `link link-primary`. *Test:* the link renders the link classes. 12. **B11 — The email row of the join-form field order can be moved; announcements are English.** `locked_ids` is missing; live-region texts are hard-coded English. *Browser:* confirmed: the email row moves by keyboard and drag, and the new order is saved. *Fix:* lock the email row (client and server) and translate the announcements via gettext. *Test:* a move request for the email row is rejected; announcements come from gettext. 13. **B13 — Duplicate toasts on a full page load.** Two flash groups (root layout and app layout). *Browser:* confirmed on full page loads: two identical toasts (`flash-group-root` + `flash-group`) and two live regions; a LiveView save shows one. *Fix:* render exactly one flash group (DESIGN_GUIDELINES §9.4). *Test:* a controller-rendered page with a flash contains exactly one flash group. ## Out of scope - New components (`<.modal>`, `<.combobox>`, `<.file_input>`, menu button): U4, U7. - Redesign of the user form or the fees tab: U5, U4. ## Acceptance criteria - Each defect above is fixed and covered by the named regression test. - DESIGN_GUIDELINES §5.6 (interaction states), §9.4 (one flash container), §10.3 interim rule (legacy dialogs), §18.3 (upload errors) are met on the touched pages. - Before/after evidence for B2/B3 (focus), B4 (error message), B5 (dark mode) and B13 (single toast) attached. - Keyboard check: every fixed control is reachable with Tab and operable with Enter/Space; focus is visible. - `just ci-dev` green after every step. ## Dependencies - B2, B3 and B7 touch `membership_fees_component.ex`, which #536 (merged) changed: re-check their anchors before these steps. - No dependency on the UI pattern issues. ## Anchors (verified against `main` @ `d9093a1f`, re-check before filing) - B1: `lib/mv_web/live/custom_field_live/index_component.ex:112`, `lib/mv_web/live/datafields_live.ex:27` - B2: `lib/mv_web/live/member_live/show/membership_fees_component.ex:306,353,395,458`, `lib/mv_web/live/member_live/show.ex:524` - B3/N4: the same fee dialogs; `lib/mv_web/live/member_live/show/deactivate_component.ex:77` - B4/N3: `lib/mv_web/live/import_live/components.ex:79,90` - B5: `lib/mv_web/live/user_live/form.ex:113,166,174,192,213` - B6: `lib/mv_web/live/global_settings_live.ex:226` - B7: `lib/mv_web/live/member_live/show/membership_fees_component.ex:209` - B8: `lib/mv_web/components/table_components.ex:24` - B9: `lib/mv_web/components/layouts/sidebar.ex:318` - B10/N1/N2: `lib/mv_web/live/user_live/form.ex:232,482,816` - B11: `lib/mv_web/live/global_settings_live.ex:270`, `assets/js/app.js` (sortable hook) - B12: `lib/mv_web/live/auth/link_oidc_account_live.ex:293` - B13: `lib/mv_web/components/layouts/root.html.heex:72`, `lib/mv_web/components/layouts.ex:263` ## Caution - B1 only reproduces with the dialog element focused; a test that types into the slug input will not catch it. - B13: make sure the remaining flash group still covers controller-rendered pages (sign-in, join confirm).

simon added this to the Minor UI and Feature Polishing | TI I milestone

2026-10-15 20:18:38 +02:00

simon added the

bug

UX Improvement

labels

2026-10-15 20:18:38 +02:00

simon added this to the Sprint 18: Oktober 2026 project

2026-10-15 20:18:38 +02:00

simon referenced this issue from a pull request that will close it,

2026-10-16 16:40:24 +02:00

Fix verified UI defects closes #593 #595