Skip to content

Give pull requests a mobile tab and a phone-first review layout - #965

Open
yongkangc wants to merge 3 commits into
mainfrom
mobile-pr-review
Open

yongkangc wants to merge 3 commits into
mainfrom
mobile-pr-review

Conversation

@yongkangc

Copy link
Copy Markdown
Collaborator

Summary

Pull requests could already be opened on mobile, but only from the chat header's ⋯ menu ("Open PRs"), and they opened as a cramped overlay: a transient Back/Close bar, the inbox header, an "All pull requests" back row, the PR header, and the tabs stacked about 280px deep, with two different back arrows. This makes pull requests a first-class mobile destination and gives the detail a phone-first layout, refined against T3 Code's mobile pull request screens.

  • PRs tab. The mobile bottom navigation gains a PRs tab between Git and Files; it hosts the inbox like the Git and Files tabs, without the transient Back/Close bar. The chat-menu "Open PRs" entry is removed along with its now-unused onOpenPullRequests props and message.
  • Drill-down detail. In a narrow pane, selecting a pull request replaces the inbox with its detail; inbox chrome steps aside, and focus moves to Back on open and returns to the row the user left (which also scrolls it back into view). Dismissing on desktop also returns focus to the row.
  • Compact bar. A narrow detail is named by a small #128 / acme/app bar with Back and a ⋯ menu: Open on GitHub, Copy link, Copy branch name, Refresh. Copy feedback appears in the bar's subtitle as a live region.
  • Identity scrolls, tabs stay. The state, line totals, file count, title, meta, and description scroll away under the bar while the Review / Changes / Checks tabs stay pinned. The tab panel fills the scroll body, so the identity can always scroll fully away, and switching tabs keeps it there instead of jumping. Changes drops the identity entirely and gives the diff the full height.
  • Description. Shown inline and clipped at 240px with a fade and Show more / Show less, instead of hidden behind a closed disclosure; collapsing scrolls back to it. Its headings stay subordinate to the title.
  • Changes tab. "N files changed ▾" opens a jump-to-file menu (viewed state, status, +a -d); file headers pin while scrolling (as the Git views already do) on an opaque base; long paths truncate the directory before the file name; line totals move into the toolbar when narrow tabs drop them.
  • Inbox rows. A leading open / draft / merged / closed icon and a checks icon beside the review state, so rows can be triaged without reading them.
  • Touch sizing. On coarse pointers, tabs become equal-width segments and the header, action, check, and Viewed controls grow to comfortable targets; fine pointers keep the current density.
  • Desktop. The side-by-side detail keeps its header but now shares the summary component; the branch moves to the wrapping meta line, and "Checks passing" opens the Checks tab on every layout.

Screenshots

Before: reachable only from the chat menu, five stacked bars and two back arrows.

Inbox Detail

After: inbox and detail

PRs tab Detail Scrolled: tabs pinned
Actions menu Description expanded Checks

After: Changes

Full-height diff Jump to file Pinned file header

Desktop (layout kept; shared summary, pinned file headers, row signals)

Notes

  • The narrow layout keys off the panel's existing container presentation, so a narrow desktop pane gets the same drill-down.
  • PullRequestDetailPanel takes the presentation and a single onDismiss; it renders PullRequestNavBar + a scrolling identity when narrow and PullRequestHeader otherwise. Both share PullRequestSummary, PullRequestStateBadge, and PullRequestStateIcon.
  • The browser suite's desktop viewports move from 1080/1000px to 900px tall. Shrinking the viewport by more than the 80px soft-keyboard threshold is read as an open keyboard, which hides the bottom navigation; keeping desktop heights within the threshold makes the switch to 390x844 a pure width change. Real phones do not hit this, but resizing a desktop browser window narrower and shorter does, for every mobile tab; that heuristic is unchanged here.

Migration note

No API or WebSocket contract changes. Mobile users find pull requests in the new PRs tab instead of the chat ⋯ menu.

Validation

  • bun run check (server, CLI, web svelte-check, integration-tests typecheck) and bun run lint pass.
  • bun run --cwd web test: 8,889 tests pass. Five unrelated files (dialog-input-zoom, ChatBoardPanel, TicketsUx, ComposerMarkdownEditor, ConversationFeed) hit the 5s timeout during a full run on a heavily loaded host and pass on rerun.
  • Chromium, on this branch: pull-request-reviews, pull-request-inbox-access, and mobile-workspace-navigation pass. They cover the PRs tab at 320px and 390px, drill-down focus and return, the compact bar and its menu, the clipped description, tabs pinned while the identity scrolls, the jump-to-file menu scrolling to a below-the-fold file, pinned file headers, and the removed chat-menu entry.
  • New unit coverage: PullRequestChangesTab.test.ts (narrow line totals, jump-to-file plumbing), and PullRequestsPanel drill-down, compact bar, Changes full height, checks shortcut, copy link, and row checks state. AppShell, BottomTabBar, WorkspaceRoot, CurrentChatMenu, and ChatSurface were updated for the tab.
  • Lightpanda e2e/chat-map is updated for the six-item bottom bar but was not run locally.
  • Rapid chat switching: the PRs tab reuses the persistent singleton surface (not remounted on switch); the inbox keeps its selection and scroll across tab changes.

🤖 Generated with Claude Code

yongkangc and others added 2 commits October 30, 2026 05:50
Pull requests were reachable on mobile only from the chat menu and opened
as a transient overlay with five stacked bars. They now have a PRs tab in
the bottom navigation, and a narrow detail is a drill-down: a compact bar
with Back and an actions menu, an identity block that scrolls away under
pinned tabs, a clipped description, and a full-height Changes tab with a
jump-to-file menu and pinned file headers. Inbox rows show the pull
request's state and checks. Coarse pointers get larger touch targets.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 30, 2026 05:54
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@cfal

cfal commented Oct 30, 2026

Copy link
Copy Markdown
Owner

Reviewed commit: b3981f8dbc59aadff4a22ffbec87818688df9dea

Medium

  • Mobile tab switching discards pull-request review progress. PRs are promoted to a primary tab, but mobile renders only the active portable surface, so switching to Chat unmounts the entire PR panel. Component-local state such as viewed/collapsed files and inbox scroll position is consequently reset on return. This can erase substantial manual review progress. Retain primary mobile singleton frames while inactive, or move this state into PullRequestsStore and restore the viewport. Add a PR → Chat → PR test covering scroll position and viewed files. AppShell.svelte WorkspaceRoot.svelte PullRequestChangesTab.svelte

  • The file-jump menu defeats the diff’s virtualization for large PRs. Opening it eagerly creates a menu item, several spans, and icons for every changed file. The review model supports up to 10,000 summary files, making this capable of freezing or exhausting a mobile browser even though the diff itself is virtualized. Use a searchable virtualized list, or render a bounded result window, and add a maximum-size performance test. PullRequestChangesTab.svelte common/git.ts

  • Arrow-key navigation loses focus when entering or leaving Changes on narrow layouts. handleTabKeydown saves the current tab list before awaiting tick(), but Changes and the other tabs render different tab-list instances. The saved element is detached during the update, so focusing its child does nothing. Keep a single stable tab list or query a bound current root after tick(), and test both arrow directions across Changes. PullRequestDetailPanel.svelte PullRequestDetailPanel.svelte

Low

  • Show less scrolls before the description has collapsed. The handler changes expanded and immediately calls scrollIntoView; Svelte has not yet applied the 240px maximum height, so a long section can remain positioned with its heading above the viewport after collapsing. Await tick() before scrolling and assert the description heading is visible after collapsing a deeply scrolled body. PullRequestDescription.svelte

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.

3 participants