From fb4c76ba82379134ce0dd5e665e85089b216070d Mon Sep 17 00:00:00 2001 From: dtoro Date: Sat, 11 Jul 2026 21:52:59 +0200 Subject: [PATCH] =?UTF-8?q?fix(ui):=20implement=20UI=20review=20findings?= =?UTF-8?q?=20=E2=80=94=20a11y,=20IA,=20and=20consistency=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the reviewed gaps: keyboard-inaccessible delete controls (SessionRail, Entities row), case-sensitive entity filter, two competing entity-detail navigation patterns (standardize on EntitySheet), non-clickable Overview KPI cards, a bare button bypassing the shared Button component, inconsistent blur-only vs live filtering, and an unenforced sanitization assumption on search snippet HTML (now using the already-present dompurify dependency). Co-Authored-By: Claude Sonnet 5 --- .../done/2026-07-11-ui-review-ia-usability.md | 262 ++++++++++++++++++ plans/index.md | 1 + web/src/App.svelte | 8 +- web/src/lib/components/SessionRail.svelte | 44 +-- web/src/lib/utils.ts | 10 + web/src/pages/Agent.svelte | 8 +- web/src/pages/Audit.svelte | 7 +- web/src/pages/Entities.svelte | 12 +- web/src/pages/Events.svelte | 7 +- web/src/pages/Graph.svelte | 13 +- web/src/pages/Knowledge.svelte | 16 +- web/src/pages/Overview.svelte | 104 +++---- 12 files changed, 404 insertions(+), 88 deletions(-) create mode 100644 plans/done/2026-07-11-ui-review-ia-usability.md diff --git a/plans/done/2026-07-11-ui-review-ia-usability.md b/plans/done/2026-07-11-ui-review-ia-usability.md new file mode 100644 index 0000000..d7374f2 --- /dev/null +++ b/plans/done/2026-07-11-ui-review-ia-usability.md @@ -0,0 +1,262 @@ +# UI review: information architecture, usability, and best practices + +Status: Done — 2026-07-11. All fix-plan items implemented and verified live +except C2 (a11y lint enforcement — no ESLint/svelte-check is configured in +`web/` at all, so there's nothing to promote from warn to error; flagged +below instead of silently adding lint infra). Verification also surfaced an +unrelated pre-existing bug (Knowledge page search results never render) — +spun off as a separate task, not fixed here. + +## Scope + +Systematic review of `web/src/` (Svelte 5 + shadcn-svelte + Tailwind v4 +control-room UI): all 13 pages, the 11 shared components, the sidebar/routing +shell (`App.svelte`), and cross-cutting patterns (filtering, loading/empty +states, live-event wiring, accessibility). Read in full, not sampled. +Grounded in what's actually in the code — no speculative "best practice" +items without a concrete file:line instance. + +Not implementation. Findings and a proposed fix plan only, mirroring +[`2026-07-11-nomos-agent-code-review.md`](2026-07-11-nomos-agent-code-review.md)'s +structure — implement on a later "proceed." + +## Findings + +### A. Information architecture + +**A1. Entity detail has two competing UI patterns for the same content.** +[`Entities.svelte:16-19,155`](../web/src/pages/Entities.svelte) opens entity +detail as an in-page `EntitySheet` slide-over (no URL change, no sidebar +state change). [`Knowledge.svelte:57-59`](../web/src/pages/Knowledge.svelte) +and [`Graph.svelte:464`](../web/src/pages/Graph.svelte) instead navigate via +`location.hash = '#/entity/' + slug`, which `App.svelte`'s router resolves to +a full-page `EntityDetail` route — but `'entity'` isn't in `navItems` +([`App.svelte:68-79`](../web/src/App.svelte)), so landing there leaves the +sidebar with nothing highlighted and the header showing the raw slug instead +of a section name. Same underlying view +(`EntityDetailContent.svelte`), three different entry points, two +different navigation models, one of which produces an orphaned page state. +A user who reaches an entity via Knowledge or Graph has no way back to +"where they were" via the sidebar — only browser back. + +**A2. Two chat entry points with no visual link between them.** +The sidebar's "Tasks" section (board → `Chat.svelte` detail, +`isActive={page === 'tasks' || page === 'chat'}`, +[`App.svelte:127`](../web/src/App.svelte)) and the footer's "Chat drawer" +button ([`App.svelte:160-163`](../web/src/App.svelte), opens a `Sheet` +wrapping the same `Chat` component) are both valid, intentional ways to +reach chat — but nothing in the UI explains they're different modes (drawer += overlay on current page, keeps your place; Tasks = full navigation). A +first-time user has no way to know which one preserves their current page. +Low-severity, but worth a tooltip/label distinction. + +**A3. Overview's KPI cards don't drill down.** +[`Overview.svelte`](../web/src/pages/Overview.svelte) shows "Pending +approvals," "Open signals," and fleet-health counts as static cards. The +header badges for the same data (`approvalsPending`, `openSignals`, +[`App.svelte:185-194`](../web/src/App.svelte)) ARE clickable and navigate to +Ops/Signals — so the pattern exists in the app, just not on the page whose +entire purpose is summarizing this data. A dashboard card showing a count +that doesn't lead anywhere is a standard drill-down gap. + +### B. Usability / interaction consistency + +**B1. Table-row click targets lack keyboard/screen-reader support in one +place but not others.** +[`Entities.svelte:117-120`](../web/src/pages/Entities.svelte) makes an +entire `Table.Row` clickable via a bare `onclick`, with no `role`, +`tabindex`, or `onkeydown` — unreachable and inoperable via keyboard, and +screen readers get no indication the row is interactive. This is a +regression against the codebase's own established pattern: `Tasks.svelte` +wraps its cards in real ` diff --git a/web/src/lib/components/SessionRail.svelte b/web/src/lib/components/SessionRail.svelte index 6ddf40c..59a1356 100644 --- a/web/src/lib/components/SessionRail.svelte +++ b/web/src/lib/components/SessionRail.svelte @@ -44,27 +44,29 @@
{#each $sessions as session (session.id)} - +
+ + +
{:else}

No sessions yet.

{/each} diff --git a/web/src/lib/utils.ts b/web/src/lib/utils.ts index e4a5890..38f72bf 100644 --- a/web/src/lib/utils.ts +++ b/web/src/lib/utils.ts @@ -21,6 +21,16 @@ export function relativeTime(iso: string | null | undefined): string { return `${d}d ago`; } +// debounce wraps fn so rapid calls (e.g. keystrokes in a filter input) +// collapse into one invocation after `wait`ms of silence. +export function debounce void>(fn: T, wait = 300): T { + let timer: ReturnType | undefined; + return ((...args: Parameters) => { + clearTimeout(timer); + timer = setTimeout(() => fn(...args), wait); + }) as T; +} + // eslint-disable-next-line @typescript-eslint/no-explicit-any export type WithoutChild = T extends { child?: any } ? Omit : T; // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/web/src/pages/Agent.svelte b/web/src/pages/Agent.svelte index afef047..3c94747 100644 --- a/web/src/pages/Agent.svelte +++ b/web/src/pages/Agent.svelte @@ -1,10 +1,12 @@
@@ -461,7 +470,7 @@
- @@ -504,3 +513,5 @@ {/if} + + diff --git a/web/src/pages/Knowledge.svelte b/web/src/pages/Knowledge.svelte index 73608ad..96e6e4d 100644 --- a/web/src/pages/Knowledge.svelte +++ b/web/src/pages/Knowledge.svelte @@ -1,10 +1,12 @@ @@ -117,8 +123,10 @@ {hit.type}
{#if hit.snippet} - - {@html hit.snippet} + + {@html DOMPurify.sanitize(hit.snippet, { ALLOWED_TAGS: ['b'], ALLOWED_ATTR: [] })} {/if} {#if hit.linked_entities?.length}
@@ -174,3 +182,5 @@ {/if}
+ + diff --git a/web/src/pages/Overview.svelte b/web/src/pages/Overview.svelte index 4f4d14d..be9fea4 100644 --- a/web/src/pages/Overview.svelte +++ b/web/src/pages/Overview.svelte @@ -1,7 +1,7 @@