Skip to content

fix(ui): phone pull request surfaces - #689

Merged
Tryanks merged 11 commits into
mainfrom
fix/phone-pr-surfaces
Oct 11, 2026
Merged

Tryanks merged 11 commits into
mainfrom
fix/phone-pr-surfaces

Conversation

@Tryanks

@Tryanks Tryanks commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Summary

#661  pull_request_tools ← pull_request_tools_offered ← mcp.pull_request_url ← HostServices.pull_requests
      desktop root   start_local      bind mcp-host, start 4 servers        ┐
      headless root  main             bind mcp-host, start 3 servers        ├─▶ HostServices::start_mcp_servers(computer_use)
      phone example  --local          HostServices::default()  ← no tools   ┘    (runtime, one owner)

#662  widgets/menu.rs     PopupMenu items: one 16pt leading slot (check | icon | blank) + aria_label;
                          dropdown_menu(..).touch(bool) → 44pt rows
      widgets/popover.rs  bottom sheet: the trigger Button gets a press action that does what a click does
      widgets/button.rs   (its own handler, then the sheet's toggle); none while disabled or loading;
                          pointer clicks still reach the sheet's wrapper as before
      pull_requests/*     row/detail ⋯ menus: Track A / A5 / A6 icons; ⋯ is touch on phone
                          row line 2 (desktop): [source][author, all or nothing][branches] · Layer N of M · ±stat
                          phone chip row: h_flex sized to its chips (was stretched → ~0 scroll range) + edge fades
      diff/model.rs       a new file's reconstructed old side is "" (was "\n" → old line 1 vs blank new line)
      diff/list.rs        file header spans the visible width and is pinned to it while the lines scroll
                          sideways; the path truncates, the counts and ⋯ never leave the screen

#661, localized. The compact sheet renders "What the agent sees" from SessionStatus.pull_request_tools, which the host sets only when it was started with the pull request MCP server. The desktop and headless roots each bound the MCP host and started the servers; the phone example's --local host was spawned with HostServices::default(), so it never had the server. A phone paired to a desktop or headless host was not affected. The binding moves into HostServices::start_mcp_servers (runtime), which all three roots now call; the headless root's note on why it serves preview moves there too. The app and headless crates drop their direct mcp-host/*-mcp dependencies, except computer-use-mcp in the app for --cu-smoke; mcp-host moves from runtime's dev-dependencies to its dependencies.

#662, every bullet:

  1. Row-menu rows 44pt on touch. dropdown_menu never passed the touch flag that context_menu has. It now does (.touch(compact) on the row's ⋯, .touch(true) on the phone page's ⋯).
  2. Diff header counts and the untracked-file quirk.
    • Reconstructing the old side from @@ -0,0 +1,N @@ joined zero lines into "\n", a blank old line the line diff matched against a blank new line. A new file now has an empty old side ("+8 −0", no old line; a one-line new file no longer shows a removed blank line).
    • The file header was as wide as the longest code line, so its counts sat past the right edge. It now spans the visible width and is offset by the horizontal scroll, so after scrolling sideways the whole header (path, kind, counts, ⋯) stays on screen. The list tracks its horizontal scroll with a ScrollHandle and draws again when the width or offset settles elsewhere. The path truncates instead of the counts.
  3. ⋯ menus: icons and accessible names. Every item carries the icon its design lists: Track A §3.4 (ExternalLink, Copy, Eye/EyeOff, Unlink, Link), A5 §5.3 (GitPullRequest/GitPullRequestDraft, ArrowUpDown, GitMerge, Pencil, GitPullRequestClosed, RotateCcw, Undo2), A6 §4.1/§5.1 (GitMerge, RefreshCw). Merge-method checks keep their check mark in the same fixed 16pt slot. Every labelled PopupMenu item now exposes its label as its accessible name (all Tcode menus, not only these two). Follow-up, not done: menu_element items (custom-rendered rows such as the sidebar's Delete project / Delete thread) have no label to derive a name from and stay unnamed.
  4. Phone chip row. The chips were a block child stretched to the viewport, so the row could scroll only a few points and "Layer N of M" could never be reached. The row now sizes to its chips, uses Tcode's horizontal scroll area, and fades whichever edge has more chips past it. Design decision (not in the docs): A4 §2.4 keeps phone chips in one scrolling row so the header height is fixed. "Conflicts with …" is therefore whole once scrolled to and faded at the edge at rest, not wrapped. gpui-base has no scroll-edge cue; the site says so.
  5. Layer selector and pill.
    • The selector trigger name ("Stack ci: build macOS releases on macos-26 runners, realign traffic-light insets #57, layer 5 of 6") and the layer names with their state ("chore(deps): bump android-actions/setup-android from 3 to 4 #415 checkout/cleanup, open, above, not linked") were already on main from A6. Verified in the live AX tree; no change.
    • Press action: fixed at the owner. Popover::render_sheet hands its toggle to the trigger, and Popover::trigger (which now takes a widgets::Button) passes it to Button::press_action.
      • The Button registers an accessibility click action that does what a click on it does: its own on_click handler first (with a keyboard ClickEvent), then the sheet's toggle. This is the same order a pointer click takes through the button and then the sheet's wrapper.
      • A disabled or loading Button registers none, so a disabled trigger (rewind while blocked, the composer chips when not interactive, the read-only editor row) has no press action.
      • Controlled sheets driven by .open(..) (the compose editor row, the reviewers/labels picker) run their own handler and open as on a pointer click.
      • GPUI's default press synthesizes a click at the node's centre, which misses a trigger clipped out of view (the selector at rest); the action calls the handlers directly.
      • Pointer clicks still reach the sheet's wrapper as before.
    • The phone pill and the phone layer selector now both expose [press], and pressing either through AX opens its sheet; the selector works while it is still off-screen. Every other Popover::trigger bottom sheet gains the same press action (e.g. the composer's Permissions and Context Window pickers on phone). The pill's call-site listener is gone.
  6. "Layer N of M".
    • Desktop rows: the layer and the diff stat never give way. The author gives way first and whole: a line too narrow for it wraps it onto a second line its one-line box clips, taking its separator and spacing with it. The box sets its own 16pt line height and is exactly one line tall, which the comment ties to line 2's 11px text. Stretching it to the row would not work: a stretched box takes its wrapped content's two lines. Spacing is each part's leading margin, so no double gap and no "· ·". The branches truncate next.
    • Follow-up, not done (pre-existing): on chore(deps): bump taiki-e/install-action from 2.87.7 to 2.87.9 #412/chore(deps): bump gpui-wry from 0.6.0 to 0.6.1 #414 there is a wider gap between a truncated "→…" and "· Layer", because the branches box keeps max_w(50%) and its truncated text does not fill it.
    • Phone rows: "source · author · branches" is one string that truncates from its end, so the branches give way first, then the author, then the source. The author-first order holds on desktop only. "· Layer N of M · ±stat" stays whole. Below the 393pt phone width a long diff stat could still clip.
    • The phone layer sheet already filled the sheet width on main (verified). The phone chip row shows "Layer 5 of 6" by scrolling, with the edge cue at rest.

Evidence

  • Before: phone row menu: ~30pt rows, no icons, AX menu_item ×8 unnamed; no "What the agent sees" in the sheet (footer runs straight into "+ Link")

    After: 44pt rows with icons; AX names "Open on GitHub", "Copy link", "Watch for changes", "Merge stack (4)…", …
  • After (Phone: "What the agent sees" missing from the Pull requests sheet #661): phone sheet on --local, "What the agent sees" expanded (tools and instruction block)
  • Before: phone PR page: chips cut at "Conflicts with c", no cue, layer selector unreachable

    After: at rest, trailing fade; scrolled, the whole conflict chip and "Layer 5 of 6", with a leading fade

  • Before (main, same data and framing): no counts on any header; src/cart.rs old line 1 against blank new line 4; the one-line new file shows a removed blank line

    After: "+3 −1", "+8 −0", "+1 −0"; no old side for new files; the long path truncates before its counts

    After, scrolled sideways to the end of the longest line: every header whole and pinned
  • After (desktop): the PR page ⋯ menu with icons; the rows with "Layer N of 6" on every row, the author dropped whole where it does not fit (chore(deps): bump taiki-e/install-action from 2.87.7 to 2.87.9 #412–chore(deps): bump gpui-wry from 0.6.0 to 0.6.1 #414) and kept where it does (Add support for settling threads #408)

All shots are from a throwaway TCODE_DATA_DIR seeded with a synthetic acme-api repository, one thread with a 6-layer native stack plus one single PR. English, light theme.

Live AX tree (macOS, phone example --local):

  • Pill: button "4 linked pull requests, open" [press]; AX press opens the sheet.
  • Selector: button "Stack #57, layer 5 of 6" [press]; AX press opens the stack sheet, including while the selector is clipped off-screen at rest.
  • Layer items: "#415 checkout/cleanup, open, above, not linked" … "#410 checkout/base-types, merged, merged, not linked".
  • A pointer click on either trigger opens its sheet once (no double toggle).
  • Disabled and controlled triggers: rewind-while-blocked and the compose editor row are not reachable offline (rewind needs a provider transcript; the editor row needs the GitHub conversation read, which has no credential here). They were checked with a local diagnostic patch, not committed, that rendered the real comment composer row and a bottom sheet with a disabled Button trigger under the conversation's load failure on chore(deps): bump gpui-wry from 0.6.0 to 0.6.1 #414:
    • button "Disabled sheet trigger" has no [press].
    • button "Add a comment…" [press]: an AX press opened the "Comment on chore(deps): bump gpui-wry from 0.6.0 to 0.6.1 #414" editor sheet. It stayed open with its input focused, through the row's own handler then the toggle.
    • The stack selector still opens by AX press.

Tests:

  • app::pull_requests::tests::the_agent_tools_are_disclosed_before_the_first_turn is moved to the boundary the roots use (CONTRIBUTING "When a test fails…" step 3: rewritten only to drive the code the way the app does). It now starts a host with HostServices::start_mcp_servers + spawn_host instead of pumping a hand-made server into an AppState. It fails with HostServices::default() (the old phone path) and passes with the fix.
  • diff::model::tests::reconstruction_requires_matching_unambiguous_current_text gains a unified new-file case (@@ -0,0 +1,3 @@ → old ""). It fails before the fix ("\n").
  • New widgets::menu::tests::a_touch_dropdown_menu_has_touch_sized_rows: a dropdown opened with .touch(true) has rows ≥ 44pt. It fails without passing the flag through.
  • Accessible names and press actions have no test: gpui's TestWindow never calls a11y_init, so no test window builds an accessibility tree, and crates/ui has no a11y-tree test to extend. They were verified in the live macOS AX tree.
  • The diff header pin and the row layout have no test; they are layout, verified by the screenshots above.
  • No test deleted.

Checks (final commit 8fb1afd):

cargo fmt --all --check                                         → clean
cargo clippy --workspace --all-targets --locked -- -D warnings  → exit 0
cargo nextest run --workspace --locked                          → 1112 tests run: 1112 passed, 13 skipped
cargo machete                                                   → didn't find any unused dependencies

Not run locally: the iOS, Android and Web builds (CI); dark theme and narrow widths were looked at but are not attached.

Merge Danger

Door: two-way
Blast Radius: UI

  • Every labelled PopupMenu item now exposes its label as its accessible name, and dropdown_menu gains an opt-in touch flag.
  • widgets::Popover::trigger now takes a widgets::Button (the only Selectable in the crate; every caller already passed one).
  • Every enabled bottom-sheet trigger built with it gains an AX press action that runs the button's own click handler and then opens the sheet, as a pointer click does. Disabled or loading triggers get none.
  • Triggers given with trigger_with (the agents pill, menus) are unchanged.
  • The desktop and headless roots start their MCP servers through one runtime function, with the same servers as before (headless still without computer use). Bind and start failures now log one combined line.
  • No wire, protocol or persisted-format change.

Closes #661
Closes #662

@Tryanks
Tryanks marked this pull request as ready for review October 11, 2026 00:19
@Tryanks
Tryanks enabled auto-merge (squash) October 11, 2026 00:19
@Tryanks
Tryanks merged commit a0cb5c3 into main Oct 11, 2026
7 checks passed
@Tryanks
Tryanks deleted the fix/phone-pr-surfaces branch October 11, 2026 00:33
Tryanks added a commit that referenced this pull request Oct 11, 2026
* feat: GitLab merge requests behind the pull request host boundary

HostKind::Gitlab (gitlab.com by name or a "gitlab" DNS label; self-managed
servers by configuration, with a port and no path) and GitLab's terms, which
take glab's merge commands from GitHub's. services::gitlab reaches each server
over REST v4 with its own token (saved, GITLAB_TOKEN for GITLAB_HOST's server
or gitlab.com, glab's stored login per host) and reads discussions, awards,
labels, permissions and blobs over GraphQL, which answers a public project
anonymously where REST's notes and labels refuse. Writes cover comments,
replies, resolution, awards, edits, labels by name, reviewers by id, the
Draft: title prefix, close and reopen, rebase, merge with squash, a cleaned
message and merge_when_pipeline_succeeds, and a review replayed as line
discussions, a note and an approval.

The conflict rule that holds a false for 30 seconds at one head, Tcode's
viewed marks, write answers, media reads and the CLI runner move to forge so
Forgejo and GitLab share them; one viewed-marks store serves both. Capabilities
gain update_merge, which GitLab lacks, so its branch update is a rebase.

* feat(ui): GitLab's mark, a rebase-only branch update and partial reviews

The GitLab mark joins the icon assets. On a host without update_merge the
behind chip and the menu offer only the rebase, and the primary update asks
for it; a review a host stopped part way through says so in its own toast.

* fix(services): GITLAB_TOKEN goes to the server any of glab's host variables names

* fix(services): gitlab.com is listed once something names it, like codeberg.org

* fix: GitLab merge requests carry GitLab's line counts, and a host without counts shows none

* fix(services): GitLab bodies name their uploads on the server, so they draw and read

* fix(services): GitLab reviewers change by append and remove, as far as GitLab's answer shows

* fix(ui): a rebase GitLab only started says so, and the rebase-only update describes a rebase

* test(services): the dispatcher refuses a merge-style update where the host only rebases

* fix(services): a GitLab pipeline waiting on a manual job is pending, and auto-merge reads the one REST field

* docs(protocol): note optional line counts and a pull request's RebaseStarted

* fix(services): GitLab upload links in code stay as written

* fix(services): a GitLab summary without line counts still syncs

* merge: keep the compact row's stat optional after #689
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.

Phone PR surfaces: row-menu rows below 44pt and clipped diff header counts Phone: "What the agent sees" missing from the Pull requests sheet

1 participant