Skip to content

feat: mobile responsive layout#211

Merged
wesm merged 7 commits intowesm:mainfrom
0xjjjjjj:fix/mobile-responsive
Mar 23, 2026
Merged

feat: mobile responsive layout#211
wesm merged 7 commits intowesm:mainfrom
0xjjjjjj:fix/mobile-responsive

Conversation

@0xjjjjjj
Copy link
Copy Markdown
Contributor

Summary

  • Collapsible sidebar: sidebar collapses to a fixed overlay on viewports below 768px, toggled via hamburger button or b keyboard shortcut. On desktop, pressing b hides the sidebar and shows the hamburger to reopen it.
  • Responsive header: nav button labels hidden below 1024px (icons only), nav buttons and project typeahead hidden below 768px, export/publish buttons hidden on mobile.
  • Touch targets: header buttons enlarged to 44px minimum on pointer: coarse devices per Apple HIG.
  • Content overflow: code blocks constrained to viewport width on mobile, breadcrumb metadata (time, tokens, session ID) and status bar stats hidden on narrow screens.
  • Reactive viewport tracking: matchMedia listener in UIStore tracks the 768px breakpoint, auto-closing sidebar on mobile and auto-opening on desktop resize. Includes proper cleanup.
  • Accessibility: sidebar backdrop uses a <button> with aria-label instead of a plain div.

No new dependencies. No changes to the Go backend. Desktop layout (>= 1024px) is unchanged.

Test plan

  • cd frontend && npx vitest run -- 740/740 tests pass (10 new tests added)
  • cd frontend && npx vite build -- builds cleanly
  • go vet -tags fts5 ./... -- no issues
  • Browser devtools responsive mode at 375px (phone), 768px (tablet), 1024px, 1440px (desktop)
  • Press b to toggle sidebar at all widths; hamburger appears when sidebar is closed
  • Resize browser across 768px boundary without reload (simulates window manager retiling)
  • Select a session on mobile; sidebar auto-closes
  • All existing keyboard shortcuts (j/k/o/l/r/e/p/s/c/?) still work
  • No visual changes at >= 1024px (regression check)

@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Mar 23, 2026

roborev: Combined Review (5dcf56d)

Summary: The PR introduces responsive sidebar layout and mobile behavior, but requires fixes for mobile navigation accessibility and touch-target layout offsets.

Medium

  • Location: frontend/src/lib/components /layout/AppHeader.svelte (@media (max-width: 767px) block)

    • Problem: The mobile breakpoint hides all .nav-btn elements and the export/publish buttons, but this diff does not add any replacement navigation or overflow menu. On phone-
      sized screens, that makes views like Pinned, Insights, and Trash unreachable, and removes session export/publish actions entirely.
    • Fix: Keep those actions accessible on mobile, for example by moving them into the sidebar/drawer or a dedicated overflow menu instead of display: none.
  • Location: frontend/src/lib/components/layout/ThreeColumnLayout.svelte (height: calc(100vh - 40px - 24px), mobile top: 40px) and frontend/src/lib/components/layout/AppHeader .svelte (@media (pointer: coarse))

    • Problem: The layout still hard-codes a 40px header offset, but the coarse-pointer rules raise header controls to a 44px minimum touch target. On touch devices, that mismatch can cause the fixed sidebar/content
      area to sit under the actual header or produce clipped/overflowing header controls.
    • Fix: Define shared header/footer size variables and update the layout/sidebar offsets together with the larger touch-target sizing, instead of mixing hard-coded 40px offsets with 44px controls
      .

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Mar 23, 2026

roborev: Combined Review (8c5908a)

Summary: One medium severity issue identified regarding CSS variable scoping for touch device layouts; no security issues found.

Medium

  • **
    Location:** frontend/src/lib/components/layout/AppHeader.svelte (@media (pointer: coarse) block), frontend/src/lib/components/layout/ThreeColumnLayout.svelte (var(--header-height) layout/sidebar offsets)
  • Problem: The coarse
    -pointer override sets --header-height on .header only, but the layout and mobile sidebar offsets read --header-height from their own scope. On touch devices, the header becomes 44px tall while the main layout and fixed sidebar still calculate against the root 40px value
    , which can leave the content area too tall and make the sidebar overlap the header by 4px.
  • Fix: Move the coarse-pointer --header-height override to a shared ancestor such as :root/html/body, or stop using a scoped CSS variable for shared layout measurements and
    update all dependent components from the same source.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Mar 23, 2026

roborev: Combined Review (0f9938e)

Summary Verdict: The PR successfully implements responsive sidebar toggling and mobile navigation, but contains a medium-severity usability issue where the sidebar inappropriately auto-closes on list
-based routes.

Medium

  • Location: frontend/src/lib/components/layout/ThreeColumnLayout.svelte (in mobileNav function)
  • Problem: mobileNav unconditionally calls ui.closeSidebar(). When users navigate to list-based routes via
    the mobile drawer tabs (like "Sessions", "Pinned", or "Trash"), this hides the sidebar overlay immediately. This leaves the user looking at an empty content screen and forces them to manually reopen the sidebar to actually select a session.
  • Fix: Remove ui.closeSidebar() from mobileNav for routes that require the user to interact with the sidebar list. The existing $effect in App.svelte already correctly handles closing the sidebar once a specific session is selected.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Mar 23, 2026

roborev: Combined Review (73d6f29)

The PR successfully implements responsive mobile layout and navigation, but introduces a couple of medium-severity issues related to accessibility and Svelte reactivity that should be addressed.

Medium

  • Location: frontend/src/lib/components/layout/AppHeader.svelte (around Sessions / Pinned /
    Insights / Trash buttons and @media (max-width: 1023px))

    • Problem: On widths <= 1023px, the text labels are removed with display: none while the buttons remain visible, and the SVGs are marked
      aria-hidden="true". That leaves these navigation buttons without an accessible name for screen readers, turning primary navigation into unlabeled icon buttons on tablet-sized viewports.
    • Fix: Keep a visually hidden text label instead of display: none, or add explicit aria-label attributes to
      each button.
  • Location: frontend/src/App.svelte:75

    • Problem: Reading ui.isMobileViewport (which is a reactive $state) inside the reactive block/$effect that watches for session id changes creates an unintended
      reactive dependency. Resizing the browser window across the 768px breakpoint will cause ui.isMobileViewport to change, which will trigger the effect to re-run and cause unnecessary duplicate messages.loadSession(id) and sync.watchSession(id) calls.

Fix: Use Svelte 5's untrack to read the viewport state without establishing a dependency: if (untrack(() => ui.isMobileViewport)) { ui.closeSidebar(); }


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Mar 23, 2026

roborev: Combined Review (cd29a13)

Summary Verdict: The changes implement a responsive mobile layout but introduce high-severity routing logic flaws and medium-severity keyboard shortcut inconsistencies for non-default tabs.

High

  • Location: frontend/src/lib/components/ layout/ThreeColumnLayout.svelte (lines 14-22) and frontend/src/lib/components/layout/AppHeader.svelte (lines 74-85)
  • Problem: The mobile navigation logic incorrectly assumes "sessions" is the only list-based route
    . mobileNav fails to deselect the active session for "pinned"/"trash" and erroneously closes the sidebar for them, making their lists inaccessible on mobile. Furthermore, clicking the hamburger while on "pinned" or "trash" forcefully navigates back to "sessions" instead of toggling the sidebar
    , resetting the user's context.
  • Fix: In ThreeColumnLayout.svelte, call sessions.deselectSession() unconditionally and only call ui.closeSidebar() if route === "insights". In AppHeader.svelte, group "pinned" and "trash" with "sessions " (e.g., check route !== "insights" instead of route !== "sessions") for the hamburger's onclick logic, class:visible directive, and ARIA labels.

Medium

  • Location: frontend/src/lib/utils/keyboard.ts :207
  • Problem: The new b shortcut always calls ui.toggleSidebar(), but the new header control is route-aware: outside the sessions route it navigates back to sessions and opens the sidebar instead of just toggling it. This leaves keyboard behavior
    out of sync with the UI and can expose the drawer on pinned/insights/trash in a state the header explicitly avoids.
  • Fix: Make the shortcut mirror the hamburger handler: toggle only on sessions, otherwise deselect the session, navigate to sessions, and open the sidebar.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Mar 23, 2026

roborev: Combined Review (090ed60)

Summary Verdict: The PR successfully introduces responsive layout and sidebar changes, but contains a few medium-severity accessibility and user-experience issues related to navigation controls and viewport states.

Medium

  • Location: [frontend/src/lib/components/layout/AppHeader.svelte#L71](/home/
    roborev/.roborev/clones/wesm/agentsview/frontend/src/lib/components/layout/AppHeader.svelte#L71)
    Problem: The hamburger is hidden on the sessions route whenever ui.sidebarOpen is true. On widths
    above 1023px, there is no media-query override to keep it visible, so pointer-only users can open the sidebar but cannot close it again from the UI; the new toggle is effectively keyboard-only in that state.
    Fix: Keep the toggle visible on the sessions route regardless of open state, or add a separate visible close affordance when the sidebar is open.

  • Location: [frontend/src/lib/components/layout/AppHeader.svelte (lines 73-84)](/home/roborev/.roborev/clones/wesm
    /agentsview/frontend/src/lib/components/layout/AppHeader.svelte#L73) and [frontend/src/lib/utils/keyboard.ts (lines 207-215)](/home/roborev/.roborev/clones/wesm/
    agentsview/frontend/src/lib/utils/keyboard.ts#L207)
    Problem: The hamburger button and 'b' shortcut unconditionally act as a "Back to sessions" navigation trigger on non-sessions routes. On desktop and tablet viewports (where the sidebar remains open across all
    routes), this prevents users from toggling the sidebar on routes like insights or pinned, and unexpectedly navigates them away. Additionally, the class:visible={!ui.sidebarOpen || router.route !== "sessions"} logic forces the hamburger to redundantly appear on desktop non-sessions routes right
    next to the existing Home button.
    Fix: Gate the "back to sessions" navigation logic with ui.isMobileViewport (e.g., if (ui.isMobileViewport && router.route !== "sessions")). On desktop/tablet, both actions should always fall back to
    ui.toggleSidebar(). Simplify the desktop visibility to class:visible={!ui.sidebarOpen} so the hamburger only appears when the sidebar is actually closed.

  • Location: [frontend/src/lib/components/layout/AppHeader.svelte#L105](/home/robore
    v/.roborev/clones/wesm/agentsview/frontend/src/lib/components/layout/AppHeader.svelte#L105)
    Problem: The route buttons now hide their only text label behind .nav-label { display: none; } at widths up
    to 1023px, but no aria-label is added. On those widths the buttons become icon-only and lose an accessible name, which is a regression for screen-reader users.
    Fix: Add explicit aria-label/title values to the route buttons,
    or replace display: none with a visually-hidden text treatment that preserves the accessible name.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

0xjjjjjj and others added 5 commits March 23, 2026 09:41
Add sidebarOpen and isMobileViewport reactive state driven by
matchMedia('min-width: 768px'). Includes toggleSidebar() and
closeSidebar() methods with proper listener cleanup.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- ThreeColumnLayout: sidebar becomes fixed overlay below 768px with
  backdrop, mobile nav drawer for route switching, accessible button
  backdrop with aria-label
- app.css: extract --header-height and --status-bar-height variables,
  bump header to 44px on pointer:coarse devices
- StatusBar: use CSS variable for height, hide stats on mobile

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add hamburger button: toggles sidebar on sessions route, navigates
  back to sessions on mobile non-session routes, toggles sidebar on
  desktop non-session routes
- Wrap nav labels in spans for CSS targeting, hide below 1024px
- Add aria-label to all nav buttons for screen reader accessibility
  when labels are visually hidden
- Hide ProjectTypeahead on mobile to prevent overflow
- Touch targets enlarged to 44px on pointer:coarse devices

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add route+viewport-aware 'b' shortcut matching hamburger behavior
- Auto-close sidebar on mobile session select via isMobileViewport
- CodeBlock: constrain max-width on mobile for horizontal scroll
- SessionBreadcrumb: hide metadata (time, tokens, ID) on mobile

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- UIStore: toggle, close, idempotency, isMobileViewport default,
  matchMedia initialization on narrow and wide viewports
- Keyboard: route-aware 'b' shortcut (sessions toggle, mobile
  nav-back, desktop toggle), modal/input guards

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@0xjjjjjj 0xjjjjjj force-pushed the fix/mobile-responsive branch from 090ed60 to cdd092d Compare March 23, 2026 16:48
@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Mar 23, 2026

roborev: Combined Review (cdd092d)

Summary Verdict: The PR successfully implements responsive layout and sidebar features, but introduces a couple of medium-severity issues regarding legacy browser compatibility and desktop navigation.

Medium Severity

  • Location: frontend/src/lib/stores/ui.svelte.ts:173
    • Problem: The viewport listener uses mq.addEventListener("change", ...) unconditionally. MediaQueryList only exposes addListener/removeListener in older Safari and some
      WebKit-based desktop webviews, which can throw an error during store initialization and break the UI on those platforms.
    • Fix: Feature-detect the modern API and fall back to addListener/removeListener when addEventListener is unavailable.
  • Location: frontend/ src/lib/components/layout/AppHeader.svelte (lines 72-73)
    • Problem: The class:visible={router.route === "sessions" || ui.isMobileViewport} condition hides the hamburger button on desktop widths (>1023px)
      during non-session routes (e.g., 'insights' or 'pinned'). A user can close the sidebar on sessions, navigate to another route, and end up with the sidebar hidden but no visible control to reopen it without using a keyboard shortcut.
    • Fix: Remove the route restriction (e
      .g., use class:visible={true} or adjust CSS) to ensure the sidebar toggle remains consistently accessible for desktop users across all routes.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

- Hamburger always visible (no conditional class:visible). On
  non-session routes it's a no-op toggle on desktop, navigates
  back to sessions on mobile.
- Fall back to addListener/removeListener for older Safari/WebKit
  that lack addEventListener on MediaQueryList.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Mar 23, 2026

roborev: Combined Review (b6f0376)

Summary Verdict: The PR successfully implements responsive sidebar and mobile layout behaviors, but contains a high-severity bug that will crash the application on startup due to improper Svelte 5
$effect usage.

High

  • Location: frontend/src/lib/stores/ui.svelte.ts (lines ~183-191)
  • Problem: The Svelte 5 $effect rune cannot be used at the module level or inside a singleton class
    constructor outside of a component initialization. Instantiating UIStore globally will crash the application on startup with Svelte's ERR_SVELTE_ORPHAN_EFFECT error.
  • Fix: Remove the $effect wrapper entirely (as global singleton stores live for the lifetime of the
    application, removing the event listener is not strictly necessary), or wrap the setup logic inside $effect.root(() => { ... }) if you strictly need a cleanup lifecycle.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@roborev-ci
Copy link
Copy Markdown

roborev-ci bot commented Mar 23, 2026

roborev: Combined Review (e029e3d)

Verdict: All reviewers agree the code is clean and no issues were found.

The changes successfully implement responsive sidebar behavior and mobile navigation state with no identified security or functional concerns.


Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@wesm
Copy link
Copy Markdown
Owner

wesm commented Mar 23, 2026

Thank you! I will review this

@0xjjjjjj
Copy link
Copy Markdown
Contributor Author

0xjjjjjj commented Mar 23, 2026

Sure thing, had a fun time going at it with roborev. Here it is on an iPhone 15 Pro/Safari running from my tailnet synced to my Claude sessions, filtered to 7 days. image

@wesm
Copy link
Copy Markdown
Owner

wesm commented Mar 23, 2026

Cool, looks good here! Merging this

@wesm wesm merged commit a349413 into wesm:main Mar 23, 2026
10 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.

2 participants