Synced from GitHub
fix(web): quieter Ready and Merge buttons on PR rows#25346
Note
🤖 Claude Opus 5.5 on behalf of Oliver
Problem
#25251 turned the PR row's Ready and Merge buttons into solid primary-colour pills in the new design. Next to the PR title they're the loudest thing in the thread details panel, even though nothing needs attention.
Fix
Ready and Merge are now a quiet neutral pill: the PR title's grey (text-foreground/80) on the panel's existing soft fill, with a slightly stronger fill on hover. Fix and Resolve keep their red tint, so the spot only gets loud when checks fail or the branch conflicts. The canvas-coloured focus ring the solid pill needed is gone, so focus uses the same inset ring as Fix and Resolve. The classic design is unchanged.
Evidence
All four actions that can sit in that spot. Merge uses an approved, green PR; Ready a draft; Fix failing CI; Resolve a conflict.
| Dark | Light | |
|---|---|---|
| Hover | Dark hover | Light hover |
| Keyboard focus | Dark focus | Light focus |
Validation
Text contrast is about 9.9:1 in dark and 8.8:1 in light. Fix and Resolve are pixel-identical to main. vp run check:changed passes. No test added: it's a purely visual change.
Claude Opus 5.5 via Claude Code in T3 Code; captures and review by GPT-6.1 Sol via Codex.
Checks
24 successful · 8 skipped
Synced from GitHub
Approvability
Verdict: Approved at ead45ec
Macroscope's review found this PR approvable — Ready and Merge pills receive subtler light/dark styling while retaining keyboard focus indicators. Action behavior and disabled-state safeguards are unchanged.
You can add or adjust custom eligibility rules. Learn more.
Synced from GitHub
Walkthrough
ThreadDetailsControlnow uses a faint background and foreground text for the primary tone, with stronger hover and pressed fills. The primary-specific background focus ring was removed. Shared compact height and inset focus-ring styling remain for primary and destructive meta actions.Changes: Thread details styling
Layer / File(s) Summary Primary tone styling
apps/web/src/components/chat/ThreadDetailsControl.tsxPrimary controls use a faint background and foreground text, with hover and pressed fills. The primary-specific background focus ring was removed; shared compact height and inset focus-ring styling remain. Priority: External image Low
Estimated code review effort: 1 (Trivial) | ~4 minutes
Change: Bug fix
Possibly related PRs
- pingdotgg/t3code#25251: Adds the primary tone and compact metadata-action styling in
ThreadDetailsControl. - pingdotgg/t3code#25778: Updates primary and metadata-action classes in
ThreadDetailsControl. - pingdotgg/t3code#24865: Establishes the redesigned thread-details presentation and renders Merge as a filled primary action.
Suggested reviewers:
t3dotgg- pingdotgg/t3code#25251: Adds the primary tone and compact metadata-action styling in
Merge Risk: External image Minimal · up to ead45
No actionable issue is identified with the Ready and Merge styling change; it is mergeable after normal checks.
Pre-merge checks: Passed 3 · Inconclusive 1
Failed Failed checks: 1 inconclusive
Check name Status Explanation Resolution Description check Inconclusive The description follows the template's main sections: Problem, Change, Scope and approval, and Verification. The Problem and Change sections are clear and specific. The Verification section includes b… Add a Scope and approval entry. Link the triaged bug issue or the discussion with the maintainer approval comment. If no prior issue exists, explain why this focused visual fix of an obvious bug qualifies for an exemption. Optionally state … Passed Passed checks: 3 passed
Check name Status Explanation Title check Passed The title names the main change: Ready and Merge buttons on PR rows become quieter. It matches the summary, which shows the primary fill replaced by a neutral pill. The title uses the conventional com… Linked Issues check Passed Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request. Full details: Title check
Explanation
The title names the main change: Ready and Merge buttons on PR rows become quieter. It matches the summary, which shows the primary fill replaced by a neutral pill. The title uses the conventional commit form and plain language. It does not cover the focus ring removal, but that is a secondary detail.
Full details: Description check
Explanation
The description follows the template's main sections: Problem, Change, Scope and approval, and Verification. The Problem and Change sections are clear and specific. The Verification section includes before/after screenshots for keyboard focus, hover, and both themes, plus a contrast figure and a check command. It does not give a reason for the Scope and approval section. The author gives no linked issue, no maintainer approval, and no explanation for an exemption. The template requires one of these. The author also gives no explicit statement that the classic design was checked by a test, beyond saying it is unchanged. The description is mostly complete, but the missing approval link is a required item.
Resolution
Add a Scope and approval entry. Link the triaged bug issue or the discussion with the maintainer approval comment. If no prior issue exists, explain why this focused visual fix of an obvious bug qualifies for an exemption. Optionally state which focused checks were run on the classic design.
Finishing Touches
🧪 Generate unit tests (beta)
- Create a new PR
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Added 2 commits
Synced from GitHub
Note
Written by gpt-6.1-sol on behalf of Maria
@flamboh, kept your neutral Ready and Merge pills, stronger hover fill, and inset keyboard focus. Merged current main into your branch without rewriting your commit, then shortened the focus comment. The final diff against main is one file, 3 additions and 5 deletions. Actions and destructive styles are unchanged. This is the approved presentation-only scope from triage.
Verification passed: vp run check:changed (format, lint, knip, affected typechecks, React compiler coverage) and both existing ThreadDetailsPrRow.browser.test.tsx cases, all through Blacksmith. In the real dev app with live PR data, checked light/dark rendering, hover, keyboard focus, Enter opening the merge confirmation, and Cancel. Also checked focus at 1280×800. Uploaded media was reopened; video playback and seeking work.
Unverified: executing remote Ready/Merge mutations and the native Electron shell. Browser verification stopped at confirmation and did not mutate any remote PR.
Note
Written by gpt-6.1-sol on behalf of Maria
Verified in a real dev environment
Verified head ead45ecb9d5fb3f52bf05a8dd4fca6d80ffbb003 as-is on this Linux Boat VM. Read the description, discussion, commits, and one-file diff first. No source changes or pushes were needed.
Ran vp i and the real dev app on github/main (32ecd71726) in a separate worktree first. Created a project pointing at the real t3code repository, submitted a Codex gpt-6.1-sol turn in the web composer, and linked this PR through the command palette. Main shows the solid blue Merge pill. Stopped main before starting the exact PR head with an explicit isolated home, carrying over that app-created database. Browser automation used the repo's Playwright and /usr/bin/google-chrome, with no mocks, fixtures, or demo data.
On the PR head, checked the live PR row in light and dark themes. Merge uses the neutral fill, with stronger hover fill. Keyboard Tab shows the 1px inset focus ring on the 20px pill. Enter opens "Merge pull request?" for #25346 using squash. Cancel closes it without merging. Repeated focus/confirmation/cancel at 1280×800. The neighboring checks popover still opens and displays the real passing checks. A second real Codex gpt-6.1-sol turn returned "pr provider path verified." through the running client. Main's first reply needed a reload to appear; the PR turn appeared live.
vp run check:changed passed through blacksmith-check: format, lint, both knip passes, affected typechecks, and React compiler coverage. Both existing ThreadDetailsPrRow.browser.test.tsx cases also passed on Blacksmith after installing its missing Chromium runtime. CI reports 32 successful contexts, zero pending or failing. The local checkout is clean.
Matching crops below show the same live PR and title. The recording shows hover, keyboard focus, confirmation, cancel, and the checks popover. Inspected the captures and reopened the uploaded video, played it through, and exercised seeking.
main, light: solid blue Merge pill
main, dark: solid blue Merge pill
live hover, keyboard focus, merge confirmation, cancel, and checks
Not verified
Ready, Fix, and Resolve states were not exercised with live data: this PR is non-draft with passing checks, and the brief prohibits changing its state or touching another PR. Ready shares the changed primary styling, but that is source evidence only. No remote Ready/Merge mutation was executed. Classic layout, native Electron shell, iOS/Android, other providers, and remote/relay/tunnel connections were not exercised on this Linux web pass.