Skip to content
This repository was archived by the owner on Jun 16, 2026. It is now read-only.

feat(ui): adaptive UI phase 1 — quick wins foundation (spec 002) - #3

Merged
Adam Poulemanos (bashandbone) merged 5 commits into
mainfrom
claude/adaptive-ui-phase-1-quick-wins
Apr 15, 2026
Merged

feat(ui): adaptive UI phase 1 — quick wins foundation (spec 002)#3
Adam Poulemanos (bashandbone) merged 5 commits into
mainfrom
claude/adaptive-ui-phase-1-quick-wins

Conversation

@bashandbone

Copy link
Copy Markdown
Contributor

Summary

Foundation phase for spec 002 — Adaptive UI. Five small, decoupled improvements that no later phase contradicts:

  • P1-A — Active session indicator on session rows (left-border + 6 px dot + 70 % dim on non-active). Uses var(--project-color, var(--color-accent)) so Phase 2 can thread project colours without touching row CSS again.
  • P1-B — AlertBar "Go to" now scrolls the session list to the row and flashes the pane border (1.5 s, prefers-reduced-motion aware). Re-clicking the same session restarts the flash via a monotonic highlightToken.
  • P1-C/ focuses the session filter globally (guarded against <input>, <textarea>, [contenteditable], and xterm's .xterm-helper-textarea).
  • P1-DSpinnerIcon.svelte + uniform loading feedback on CrossProjectOverview Refresh and LayoutSwitcher.
  • P1-E — Dark-theme-compatible warning palette (--color-warning-bg: #3d2e00 / --color-warning: #fbbf24, contrast ≈ 9:1, AA pass).

This PR is the first of five implementing spec 002 (Phase 2/3 parallel, Phase 4, Phase 5). See specs/002-adaptive-ui/plan.md.

Commits

  • 5950ff7 spec(002-adaptive-ui): design spec, research, and implementation plan
  • 755cd08 feat(ui): phase 1 adaptive-ui quick wins (A–E)
  • 01cdc20 fix(ui): address phase 1 review feedback

The first commit adds the spec docs under specs/002-adaptive-ui/; reviewers wanting to merge the spec independently can cherry-pick 5950ff7.

Review fixes applied (commit 01cdc20)

Changes from independent review of the first implementation commit:

  1. Re-flash on same-session click — replaced boolean highlighted with monotonic highlightToken; SplitView's $effect toggles flashing off/on via requestAnimationFrame so CSS animation restarts.
  2. No permanent border on SplitView — replaced border: 2 px solid transparent with a ::before overlay (inset: 0; pointer-events: none). xterm now gets full pane dimensions.
  3. Active dot placement per spec §8.1 — moved from .session-label-row::before to .session-row::before.
  4. Deeper test coverage — added component-level tests (SessionRow active/dim, AlertBar "Go to" dispatches tend:session-scroll-to, SessionList scrollIntoView spy).
  5. Phase 4 TODO comment — listener scoping concern noted in SessionList.svelte for the upcoming second list instance.

Nits also addressed: AlertBar prop typing consistency, LayoutSwitcher no-double-spinner, stale #713f12 fallback literals updated.

Test plan

  • pnpm check — 0 errors, 0 warnings (355 files)
  • pnpm test — 25 passed / 25 total (5 files, previously 7/1)
  • cargo check -p tend-protocol -p tend-cli — passes
  • cargo check -p tend-workbench — blocked by missing pango system libs in sandbox; Phase 1 doesn't touch Rust
  • Manual: flash animation with prefers-reduced-motion toggled in real browser
  • Manual: xterm sizing unchanged after border → overlay swap
  • Manual: / shortcut doesn't hijack xterm keystrokes

What this does NOT do (deferred by design)

  • No project colour values assigned yet — that's Phase 2. P1-A's CSS falls back to --color-accent cleanly.
  • No bits-ui / paneforge / vanilla-colorful / svelte-dnd-action installed. Those arrive in Phase 2–4.
  • No backend / Rust changes. First Rust touch is Phase 2 (ProjectSettings colour field).

Files changed

20 files, +916 / -59 (includes 3 new spec docs).

https://claude.ai/code/session_01Tynwwyo146dMu2McVRjgc8

Adds specs/002-adaptive-ui/ with three documents:

research.md — captures findings from two parallel research tracks:
  (A) Svelte 5 UI library landscape: paneforge (resizable panes),
  svelte-dnd-action (sortable DnD), vanilla-colorful (colour picker),
  bits-ui (headless tabs/collapsible), @neodrag/svelte (future free-drag).
  (B) UX patterns from VS Code, tmux, Slack, Grafana, Datadog, and NNGroup
  research covering session overflow, active/inactive indicators, pane
  identification, alert→pane navigation, and focus mode.

spec.md — full feature specification for:
  §1 Project colour coding (auto-palette, settings_json, vanilla-colorful picker)
  §2 Multi-pane session workspace (paneforge, svelte-dnd-action, 520 px min,
     overflow indicator, quick-switch palette Ctrl+K, project auto-fill)
  §3 Focus mode (single/split-two, AlertBar persists, breadcrumb chip, × exit)
  §4 Refresh button spinners (SpinnerIcon.svelte, uniform pattern)
  §5 Multi-view tabs (Sessions/Workspace/Overview via bits-ui Tabs)
  §6 State persistence + ghost session restore (PaneSlot schema extension,
     Restart button re-runs metadata.command[], no schema migration needed)
  §7 Collapsible sidebar (bits-ui Collapsible, hover-peek overlay, 200 ms slide)
  §8 Session identification (active indicator, hover-to-highlight, click-to-flash,
     AlertBar scroll + flash improvements)
  §9 CSS architecture for project colours (--project-color custom property)

plan.md — 5-phase implementation plan with file-change table, dependency
  installation steps, risk register, and testing notes.

No code changes in this commit — spec only.

https://claude.ai/code/session_01Tynwwyo146dMu2McVRjgc8
Implements Phase 1 of specs/002-adaptive-ui/plan.md — five small, additive
quick wins that lay groundwork for later phases without pulling in any new
dependencies or backend changes. Project colour values are NOT populated yet
(Phase 2's job); every new `--project-color` reference falls back cleanly to
`--color-accent` via `var(--project-color, var(--color-accent))`.

P1-A — Active Session Indicator
- SessionRow: `active` + `anyActive` props. Active row paints a 2 px
  --project-color left border, an 8 % colour-mix background tint, and a
  6 px dot `::before` on the label row. `data-session-id` attribute
  added to support P1-B scroll targeting.
- SessionList: `activeSessionIds: Set<number>` prop, passed per row.
- +page: derives `activeSessionIds = activeSessionId ? new Set([id]) : new Set()`.
  Phase 4 expands this to the full visible-slot set.
- Non-active rows' `.session-main` dim to 70 % opacity when any row is
  active; badges stay fully opaque (they live in `.session-meta`).

P1-B — AlertBar "Go to" flash + scroll
- SplitView: `highlighted: boolean` prop triggers a 1.5 s
  `flash-border` CSS animation on the component's outer border (also
  --project-color). Respects `prefers-reduced-motion` (animation
  suppressed, border left transparent).
- +page: `highlightSessionId` state set in `handleActivateSession`,
  cleared after 1500 ms via `setTimeout`. Timer is cleaned up on
  unmount. Passed to SplitView as
  `highlighted={highlightSessionId === activeSessionId}`.
- AlertBar: "Go to" now calls `onActivateSession` and dispatches a
  `tend:session-scroll-to` CustomEvent on `window`.
- SessionList: `$effect` registers a `tend:session-scroll-to` listener
  that looks up `[data-session-id="…"]` and calls `scrollIntoView`.
  Listener is removed on teardown.

P1-C — Filter Focus Shortcut (`/`)
- SessionList: exports `focusFilter()` instance method (Svelte 5
  `export function` in instance `<script>`). Parent uses `bind:this`.
- New util `$lib/util/isEditableTarget.ts`: guards the shortcut so it
  never fires when the keystroke originates from an `<input>`,
  `<textarea>`, `[contenteditable]` element, or anything inside xterm's
  hidden `.xterm-helper-textarea` (critical — otherwise `/` would steal
  keystrokes from the embedded terminal).
- +page: `<svelte:window onkeydown>` dispatches `/` to `focusFilter()`.

P1-D — Refresh button spinners
- New component `SpinnerIcon.svelte`: 14 px CSS-only rotating arc
  (border + border-top-color + @Keyframes). `size` and `label` props.
  Under `prefers-reduced-motion` it collapses to a static filled dot
  so the indicator still communicates "busy" without motion.
- CrossProjectOverview: Refresh button swaps its label for a
  SpinnerIcon while `overviewStore.loading`. Button is disabled and
  has a fixed `min-width` so the swap doesn't shift layout.
- LayoutSwitcher: new `refreshing` local state wraps `refresh()`
  (true before `await`, false in `finally`). Dropdown shows a
  SpinnerIcon in a new header strip while refreshing. When the list is
  empty and loading, the empty-state message reads "Loading…" with the
  spinner; the "No saved layouts" copy is preserved for the real empty
  case.

P1-E — Dark-theme warning colour
- app.css: `--color-warning-bg: #3d2e00` and `--color-warning: #fbbf24`
  under `:root` — dark-compatible, keeps salience on the dark surface.
- AlertBar: replaced hardcoded light-mode fallbacks (#fef3c7, #f59e0b,
  #6b7280, #1d4ed8, #92400e, #fde68a) with the dark-compatible CSS
  variables. Hover states use `color-mix(... transparent)` so they
  tint the warning strip without washing it out.

Tests
- `src/lib/util/isEditableTarget.test.ts`: unit tests for the guard —
  null, non-Element, `<input>`, `<textarea>`, `[contenteditable]`,
  non-editable elements, and the xterm `.xterm-helper-textarea`
  subtree check (critical for not hijacking terminal keystrokes).
- `src/lib/components/SpinnerIcon.test.ts`: renders the component with
  `svelte`'s `mount()` into jsdom, asserts `role="status"`,
  `aria-label`, and the `--spinner-size` custom property for default
  + custom size + custom label.
- `vitest.config.ts`: adds `resolve.conditions: ["browser"]` so
  vite-plugin-svelte resolves the client entry (Svelte 5's `mount()`
  is not available from the server build — tests would otherwise fail
  with `lifecycle_function_unavailable`).

Acceptance
- `pnpm check`: 0 errors, 0 warnings (353 files).
- `pnpm test`: 18 passed (3 files) — includes the two new suites.
- `cargo check -p tend-protocol -p tend-cli`: both green. (The
  `tend-workbench` crate requires `pango` system libs not present in
  the local sandbox; Phase 1 does not touch any Rust code.)

https://claude.ai/code/session_01Tynwwyo146dMu2McVRjgc8
- Click-same-session re-flashes: replace `highlighted: boolean` with a
  monotonic `highlightToken: number`. +page.svelte increments on every
  activation; SplitView's $effect toggles its `flashing` class off then
  on via requestAnimationFrame so the CSS animation restarts even when
  the same session id is re-clicked.
- Drop the always-on 2 px transparent border on `.split-view` that was
  permanently shrinking the xterm container. Moved to a
  `::before` overlay (`position: absolute; inset: 0; pointer-events:
  none`) so xterm now gets the full pane in every dimension.
- Move the active-session dot from `.session-label-row::before` to
  `.session-row::before` per spec §8.1 so the dot sits in the row's
  outer gutter, not nested inside the label flex row.
- New integration tests (7 added; total 18 → 25):
  - SessionRow.test.ts: active/dimmed classes + data-session-id.
  - AlertBar.test.ts: Go-to button dispatches tend:session-scroll-to
    CustomEvent with the right detail and calls onActivateSession.
  - SessionList.test.ts: dispatching tend:session-scroll-to on the
    window calls Element.prototype.scrollIntoView with the expected
    options; no-op for unknown session ids.
- Phase-4 TODO comment on the scroll-to listener in SessionList noting
  that a second SessionList instance will need listener scoping.
- Nits also addressed:
  - AlertBar now uses `interface Props; let {…}: Props = $props()` to
    match the rest of the codebase.
  - LayoutSwitcher suppresses the header spinner when the list is
    empty (the empty-state spinner already covers that case).
  - Update stale `#713f12` fallback literals in SessionRow and
    +page.svelte to `#3d2e00` to match the new `--color-warning-bg`
    root value. AgentPane.svelte intentionally left alone — reviewer
    did not flag it and it's not a Phase 1 file.

Verified: pnpm check 0/0, pnpm test 25/25.

https://claude.ai/code/session_01Tynwwyo146dMu2McVRjgc8
Copilot AI review requested due to automatic review settings April 15, 2026 17:24

Copilot AI 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.

Pull request overview

Implements Phase 1 “quick wins” for spec 002 (Adaptive UI) in the Svelte frontend: active-session affordances, alert navigation feedback, a global filter shortcut, consistent loading spinners, and a dark-theme warning palette, plus accompanying unit/component tests and spec docs.

Changes:

  • Add active-session styling to session rows (border + dot + dimming) and thread active session IDs from the page through SessionListSessionRow.
  • Add AlertBar “Go to” behavior to activate the session, scroll the list row into view, and flash the active pane border via a monotonic highlightToken.
  • Add global / shortcut to focus the session filter; introduce SpinnerIcon and apply it to refresh/loading UI; update warning CSS variables.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
vitest.config.ts Adjust Vitest/Vite resolution to allow Svelte 5 mount() in tests.
src/routes/+page.svelte Adds global / shortcut, active-session Set derivation, and SplitView flash token plumbing.
src/lib/util/isEditableTarget.ts New helper to detect editable targets (inputs/contenteditable/xterm helper textarea).
src/lib/util/isEditableTarget.test.ts Unit tests for editable-target detection.
src/lib/components/SplitView.svelte Adds border-flash overlay and flash restart logic keyed by highlightToken.
src/lib/components/SpinnerIcon.svelte New lightweight CSS spinner component with reduced-motion fallback.
src/lib/components/SpinnerIcon.test.ts Component test for SpinnerIcon DOM shape/props.
src/lib/components/SessionRow.svelte Adds active/anyActive props, active indicator CSS, dimming, and data-session-id.
src/lib/components/SessionRow.test.ts Component tests for active indicator, dimming, and data-session-id.
src/lib/components/SessionList.svelte Adds activeSessionIds prop, exported focusFilter(), and scroll-to event listener.
src/lib/components/SessionList.test.ts Extends tests to cover scroll-to event wiring (scrollIntoView spy).
src/lib/components/LayoutSwitcher.svelte Adds refresh spinner state and uses SpinnerIcon in dropdown UI.
src/lib/components/CrossProjectOverview.svelte Disables refresh during loading and shows SpinnerIcon to indicate activity.
src/lib/components/AlertBar.svelte “Go to” now dispatches tend:session-scroll-to and adopts warning palette vars.
src/lib/components/AlertBar.test.ts Component test ensuring “Go to” activates session + dispatches scroll event.
src/app.css Introduces dark-theme warning CSS variables.
specs/002-adaptive-ui/spec.md New Adaptive UI specification document.
specs/002-adaptive-ui/research.md Research writeup backing UX/library decisions.
specs/002-adaptive-ui/plan.md Phased implementation plan for spec 002.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread vitest.config.ts Outdated
Comment on lines +6 to +9
// Without the `browser` condition, vite-plugin-svelte resolves the
// server-side entry point and tests fail with lifecycle_function_unavailable.
resolve: {
conditions: ["browser"],

Copilot AI Apr 15, 2026

Copy link

Choose a reason for hiding this comment

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

resolve.conditions is being set to only ["browser"], which overrides Vite/Vitest’s default export conditions. That can unintentionally change how other dependencies resolve (e.g., dropping default module/development|production conditions). Consider extending the defaults instead of replacing them (e.g., include the default conditions plus browser, or derive from the merged Vite config and append browser).

Suggested change
// Without the `browser` condition, vite-plugin-svelte resolves the
// server-side entry point and tests fail with lifecycle_function_unavailable.
resolve: {
conditions: ["browser"],
// Keep Vite/Vitest's default client conditions and ensure `browser`
// remains present so vite-plugin-svelte resolves the browser entry.
resolve: {
conditions: ["module", "browser", "development|production"],

Copilot uses AI. Check for mistakes.
Comment thread src/lib/components/SessionList.svelte Outdated
const detail = (event as CustomEvent<{ sessionId: number }>).detail;
if (!detail) return;
const el = document.querySelector(`[data-session-id="${detail.sessionId}"]`);
el?.scrollIntoView({ behavior: 'smooth', block: 'nearest' });

Copilot AI Apr 15, 2026

Copy link

Choose a reason for hiding this comment

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

The scroll-to handler always uses scrollIntoView({ behavior: 'smooth' ... }), but the PR description calls out prefers-reduced-motion awareness. Consider switching to behavior: 'auto' when matchMedia('(prefers-reduced-motion: reduce)') is true so the scroll doesn’t animate for users who request reduced motion.

Suggested change
el?.scrollIntoView({ behavior: 'smooth', block: 'nearest' });
const behavior = window.matchMedia('(prefers-reduced-motion: reduce)').matches
? 'auto'
: 'smooth';
el?.scrollIntoView({ behavior, block: 'nearest' });

Copilot uses AI. Check for mistakes.
Comment on lines +34 to +43

async function refresh(): Promise<void> {
refreshing = true;
try {
const result = await layoutList();
layouts = result.layouts;
} catch (err) {
error = err instanceof Error ? err.message : String(err);
} finally {
refreshing = false;

Copilot AI Apr 15, 2026

Copy link

Choose a reason for hiding this comment

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

refreshing can become incorrect if refresh() is triggered concurrently (e.g., user toggles the dropdown quickly, or a save/delete triggers a refresh while a prior refresh is still in-flight). The first request to finish will set refreshing = false even if a later refresh is still running. Consider tracking a monotonic token/counter per refresh (or storing the current promise) and only clearing refreshing for the latest call.

Suggested change
async function refresh(): Promise<void> {
refreshing = true;
try {
const result = await layoutList();
layouts = result.layouts;
} catch (err) {
error = err instanceof Error ? err.message : String(err);
} finally {
refreshing = false;
/** Monotonic token used to ignore stale refresh completions. */
let refreshToken = 0;
async function refresh(): Promise<void> {
const token = ++refreshToken;
refreshing = true;
try {
const result = await layoutList();
if (token === refreshToken) {
layouts = result.layouts;
}
} catch (err) {
if (token === refreshToken) {
error = err instanceof Error ? err.message : String(err);
}
} finally {
if (token === refreshToken) {
refreshing = false;
}

Copilot uses AI. Check for mistakes.
Comment thread src/lib/components/SessionList.test.ts Outdated
Comment on lines +136 to +140
if (typeof Element.prototype.scrollIntoView !== "function") {
// eslint-disable-next-line @typescript-eslint/no-empty-function
(Element.prototype as unknown as { scrollIntoView: () => void }).scrollIntoView = () => {};
}
const spy = vi.spyOn(Element.prototype, "scrollIntoView");

Copilot AI Apr 15, 2026

Copy link

Choose a reason for hiding this comment

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

This test conditionally defines Element.prototype.scrollIntoView when jsdom doesn’t provide it, but never restores/deletes it afterward. That can leak state into other tests and make failures order-dependent. Consider saving the original value (possibly undefined) and restoring it in afterEach.

Copilot uses AI. Check for mistakes.
Comment thread src/lib/components/SplitView.svelte Outdated
Comment on lines +43 to +44
// Step 2: force a reflow + re-add the class on the next animation frame
// so the animation plays from the start.

Copilot AI Apr 15, 2026

Copy link

Choose a reason for hiding this comment

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

The comment says “force a reflow + re-add the class”, but the implementation only toggles flashing and waits for the next animation frame (no layout read to actually force reflow). Consider adjusting the comment to match the behavior (toggle off, then on in the next frame) to avoid misleading future maintainers.

Suggested change
// Step 2: force a reflow + re-add the class on the next animation frame
// so the animation plays from the start.
// Step 2: re-add the class on the next animation frame so the CSS
// animation plays from the start.

Copilot uses AI. Check for mistakes.
@bashandbone

Copy link
Copy Markdown
Contributor Author

@copilot apply changes based on the comments in this thread

Copilot AI and others added 2 commits April 15, 2026 18:09
…sh token, test cleanup, comment)

Agent-Logs-Url: https://github.com/knitli/tend/sessions/0227cb50-fdb4-4272-baf4-6b353304582a

Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
… boolean flag

Agent-Logs-Url: https://github.com/knitli/tend/sessions/0227cb50-fdb4-4272-baf4-6b353304582a

Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>

Copilot AI commented Apr 15, 2026

Copy link
Copy Markdown
Contributor

@copilot apply changes based on the comments in this thread

Applied all five changes from the review thread in commits bc8afd1 and a8a8e7b:

  1. vitest.config.ts — Extended default resolve conditions (["module", "browser", "development|production"]) instead of replacing them with only ["browser"].
  2. SessionList.sveltescrollIntoView now respects prefers-reduced-motion: uses behavior: 'auto' when reduced motion is preferred, 'smooth' otherwise (via optional chaining so it's safe in environments without matchMedia).
  3. LayoutSwitcher.svelte — Added a monotonic refreshToken counter; each refresh() call captures its own token and only writes to state if the token still matches when the request completes, preventing stale concurrent-refresh overwrites.
  4. SessionList.test.ts — Moved scrollIntoView setup to beforeEach, saving the original prototype value; afterEach restores it precisely (re-assigns if it was a function, deletes the property if it was absent), eliminating cross-test state leakage.
  5. SplitView.svelte — Removed the inaccurate "force a reflow" wording; comment now reads "re-add the class on the next animation frame so the CSS animation plays from the start".

All 25 tests pass, svelte-check reports 0 errors/warnings, and CodeQL found 0 alerts.

@bashandbone
Adam Poulemanos (bashandbone) merged commit 0215777 into main Apr 15, 2026
6 checks passed
@bashandbone
Adam Poulemanos (bashandbone) deleted the claude/adaptive-ui-phase-1-quick-wins branch April 15, 2026 18:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants