Skip to content

fix(desktop): polish list pages — Usage, Memory, and Web Search - #1479

Merged
Astro-Han merged 2 commits into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:fix/1364-list-pages-polish
Jul 26, 2026
Merged

fix(desktop): polish list pages — Usage, Memory, and Web Search#1479
Astro-Han merged 2 commits into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:fix/1364-list-pages-polish

Conversation

@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
Contributor

Sub-issue of #1303 — the visual/layout audit of the list/long-content pages (Usage / Memory / Web Search), same shape as the #1361 metric-pages pass. Based on #1478 (the DataTable scroller fix this pass traced to the shared primitive); the first commit here is that branch. Closes #1364.

Two of three pages had no usable baseline

page before
Memory bridge had no memory / workspaceInstructions / app.openPath channels — the story booted into two error toasts and an empty page; entry list, backup candidates and prompt preview never rendered
Usage logs: [] + requests tab defaults to its summary Alert — no story ever rendered a DataTable, so the issue's headline theme ("DataTable overflow") had no baseline; that is also why the primitive defect in #1478 went unseen
Web search the result list only renders after a live query — never covered

New variants per the issue's expected deliverables (empty vs populated vs long-content):

  • UsageRequestsPopulated (hostile-width rows: dated preview model id, namespaced MCP tool name, full-UUID session) / UsageEmpty / existing Usage (summary-only Alert)
  • MemoryPopulated (long title/content/tag sets, archived group, backup-candidate rows) / Memory (now a clean empty state)
  • WebSearchResults — a play step drives the real query path against a configured-key bridge, with a long title, a bare-URL title, and a long snippet. One wrinkle worth noting: the bridge settings are built by hand because mergeSettings treats the masked apiKey sentinel as "keep current key" and silently drops it.

What the audit found (all measured, 480–1280px)

Usage — the page grid's implicit auto track let every block's min-content propagate: the five-tab bar (~453px intrinsic) and the requests table dragged the page into horizontal scroll even though both should scroll within themselves. Pinned the single column to minmax(0, 1fr) (page + tab panel), gave the tab bar its own overflow-x: auto, and replaced two hard-track grids:

Web search — three unbreakable tokens (TAVILY_API_KEY hint, bare-URL result titles, long result hostnames) each widened the page at the floor → overflow-wrap: anywhere; min-width: 0 on the query/key inputs (a text input never shrinks below its intrinsic size width on its own); the mono nowrap timestamp may wrap in the narrow layout.

Memory — the prompt-preview header's trailing cluster (injection Chip + copy button) cannot shrink → wraps under the title.

Routed to #1360, not patched here

Memory still overflows at the 480px floor through the shared settings-rows primitive: a .settingsFormRow's min-content (~306px) exceeds the ~190px content column. That is exactly #1362's declared "row wrapping" scope, so it is documented on the routing issue rather than hacked per-page — and the e2e contract deliberately asserts Memory per-surface (prompt-preview header) instead of whole-page containment, with a comment explaining why.

Verification

  • Two e2e contract tests (containment + tab-bar/summary geometry at the 480px floor; full-width guard so auto-fit cannot cost the wide layout). Confirmed they fail against origin/main's CSS (toHaveCSS('overflow-x', 'auto') on the tab bar).
  • Full settings e2e suite 13/13; desktop unit suite 2845/2845; format / typecheck / test:checks / check-dead-css clean.
  • Probed 7 stories × 480–1280px in Storybook: zero horizontal overflow beyond the documented settings-rows remainder.

Live-app screenshots

Usage and Web search at the 480px floor (summary folds, tab bar scrolls within itself, tokens wrap):

Image Image

The new populated baselines — requests DataTable scrolling in its own container at 1280px, and the Memory entry surfaces that had never rendered in a story:

Image

Image

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. The production fix is in the right places: DataTable owns its scroll boundary, and the page CSS only handles the grid and text constraints that belong to those pages. The remaining settings-row overflow is correctly left to the shared follow-up instead of being patched here.

I left two inline comments about states that the Story and E2E do not actually render. Those are coverage gaps, not objections to the implementation.

Comment thread apps/desktop/e2e/settings.spec.ts
Comment thread apps/desktop/stories/settings/settings-pages.stories.tsx
UncertaintyDeterminesYou4ndMe added a commit to UncertaintyDeterminesYou4ndMe/maka-agent that referenced this pull request Jul 25, 2026
…e2e and stories

Review follow-up for apache#1479 (both inline comments):

- The floor containment test never rendered the two long-content
  branches: the default fixture has no request logs (requests DataTable
  never mounts) and no Tavily key (the page stops at the no-key
  message). Two new tests lock the actual fixes — the request table
  scrolls inside its own container while the page stays contained
  (settings-usage fixture, refresh-until-seeded to dodge the boot
  race), and the hostile-width results (bare-URL title, long snippet)
  wrap inside their cards.
- Web search results come from a typed offline fixture
  (web-search-e2e-fixture.ts, mirrors the story rows) answered AFTER
  the real validation/enablement gates; the settings-search scenario
  now seeds a configured Tavily key so the query controls are enabled.
- UsageEmpty story now opens on the actual EmptyState: it reuses the
  details-on requests-tab settings instead of default settings whose
  showDetails:false rendered another summary baseline.

Settings e2e 15/15 locally; desktop unit tests 2845/2845.
@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
Contributor Author

@Astro-Han The follow-up commit (c6804b7) seems to be waiting for a maintainer to approve the workflow run — no check suite has been created on it (fork PRs appear to be gated now; base-repo PRs are still triggering fine). Local verification: settings e2e 15/15, desktop unit 2845/2845, format/typecheck clean. Could you hit "Approve and run" when you get a chance?

Sub-issue of apache#1303, same shape as the apache#1361 metric-pages pass. Based on the

Story fixtures first — two of three pages had no usable baseline
----------------------------------------------------------------
  - Memory: the story bridge had no `memory` / `workspaceInstructions` /
    `app.openPath` channels at all, so the story booted into two error
    toasts and an empty page — no entry list, backup candidates, or prompt
    preview ever rendered. The bridge now serves a clean empty state by
    default; `MemoryPopulated` covers the list surfaces with deliberately
    long titles, contents, and tag sets.
  - Usage: `logs` was `[]` and the requests tab defaults to its summary-only
    Alert, so **no story ever rendered a DataTable** — the issue's headline
    theme had no baseline. `UsageRequestsPopulated` pins hostile-width rows
    (dated preview model id, namespaced MCP tool name, full UUID session);
    `UsageEmpty` covers the all-empty tabs.
  - Web search: the result list only renders after a live query.
    `WebSearchResults` drives the real query path via a play step against a
    configured-key bridge (built by hand — `mergeSettings` treats the masked
    apiKey sentinel as "keep current" and would drop it), with a long title,
    a bare-URL title, and a long snippet.

Layout fixes (all found by measuring the new baselines)
-------------------------------------------------------
  - Usage page + tab panel: pin the implicit single grid column to
    `minmax(0, 1fr)`. With the implicit `auto` track, every block's
    min-content propagated into the track — the five-tab bar (~453px
    intrinsic) and the requests table dragged the page into horizontal
    scroll even though both scroll within themselves.
  - Usage tab bar: five nowrap tabs are wider than the whole content column
    at the 480px window floor; the bar scrolls within itself (tabs stay one
    line by design).
  - Usage filters: was a five-track grid (~724px min-content) behind a 900px
    *viewport* breakpoint; between the breakpoint and ~1100px the grid
    overflowed the page because the settings column is narrower than the
    viewport. Same lesson as apache#1361: wrap structurally instead of guessing a
    threshold — the filter input grows from its old 260px track, everything
    else falls to the next line when it no longer fits.
  - Usage summary: `repeat(4, minmax(0, 1fr))` → `auto-fit`, the same
    hard-track division apache#1361 fixed on the Permissions/Health summaries
    (tiles collapsed to ~32px at the floor). Full-width layout unchanged.
  - Memory prompt-preview header: the trailing cluster (injection Chip +
    copy button) cannot shrink; it wraps under the title at the floor.
  - Web search: three unbreakable-token fixes (`overflow-wrap: anywhere` on
    result titles that are bare URLs, result source hostnames, and the
    TAVILY_API_KEY env-var hint), `min-width: 0` on the query/key inputs
    (a text input never shrinks below its intrinsic size width on its own),
    and the mono nowrap timestamp may wrap in the narrow layout.

Routed to apache#1360, not patched here
---------------------------------
The Memory page still overflows at the 480px floor through the shared
settings-rows primitive: a `.settingsFormRow`'s min-content (~306px — label +
status Chip + switch + paddings) exceeds the ~190px content column. That is
the shared-primitive row-wrapping problem apache#1362 names for the form-row pages,
so it is documented on apache#1360 rather than hacked per-page; the e2e contract
asserts Memory per-surface (prompt-preview header) instead of whole-page
containment, with a comment explaining why.

Verification
------------
  - Two e2e contract tests: containment + tab-bar/summary geometry at the
    480px floor for Usage and Web search, per-surface for Memory; plus a
    full-width guard so `auto-fit` cannot silently cost the wide layout.
    Confirmed they fail against origin/main's CSS.
  - Full settings e2e suite 13/13; desktop unit suite 2845/2845.
  - Probed 7 stories x 480-1280px in Storybook: zero horizontal overflow
    beyond the documented settings-rows remainder.
  - Verified in the live app at 1280px and 480px on all three pages.

Closes apache#1364.
…e2e and stories

Review follow-up for apache#1479 (both inline comments):

- The floor containment test never rendered the two long-content
  branches: the default fixture has no request logs (requests DataTable
  never mounts) and no Tavily key (the page stops at the no-key
  message). Two new tests lock the actual fixes — the request table
  scrolls inside its own container while the page stays contained
  (settings-usage fixture, refresh-until-seeded to dodge the boot
  race), and the hostile-width results (bare-URL title, long snippet)
  wrap inside their cards.
- Web search results come from a typed offline fixture
  (web-search-e2e-fixture.ts, mirrors the story rows) answered AFTER
  the real validation/enablement gates; the settings-search scenario
  now seeds a configured Tavily key so the query controls are enabled.
- UsageEmpty story now opens on the actual EmptyState: it reuses the
  details-on requests-tab settings instead of default settings whose
  showDetails:false rendered another summary baseline.

Settings e2e 15/15 locally; desktop unit tests 2845/2845.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving 89681fb. The E2E now reaches the request table and long Web Search results, and UsageEmpty opens on the actual empty state. All required checks pass.

One follow-up: settings.spec.ts calls .toPass() without a timeout. With the locked Playwright 1.61.1, this defaults to 0 and does not inherit expect.timeout, so the refresh block runs once. Please set a timeout on the call or configure expect.toPass.timeout. This is not blocking.

@Astro-Han
Astro-Han merged commit a0f6e8e into apache:main Jul 26, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(desktop): polish list pages — Usage, Memory, and Web Search

2 participants