feat(web): make review verdicts legible in the pull request detail - #7077
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
One finding on the new review-verdict UI. Details inline.
Posted via Macroscope — UI Consistency
ApprovabilityVerdict: Needs human review New UI feature introducing user-facing behavior (review verdict display across multiple views with staleness tracking). While purely frontend and well-tested, the substantial new capability warrants human review. You can customize Macroscope's approvability policy. Learn more. |
accfaf0 to
70cbf6e
Compare
There was a problem hiding this comment.
Two consistency issues in the new review-verdict surfaces. Details inline.
Posted via Macroscope — UI Consistency
a5277aa to
395c738
Compare
There was a problem hiding this comment.
Reviewed the review-verdict surfaces for consistency with the existing comment cards. Two related findings: both new verdict surfaces drop the reaction affordance that every other comment card keeps.
Posted via Macroscope — UI Consistency
395c738 to
ac4be9a
Compare
There was a problem hiding this comment.
Three consistency issues in the new verdict surfaces — two of them are hover-affordance scoping (group) on the timeline rows.
Posted via Macroscope — UI Consistency
139a491 to
02c764b
Compare
There was a problem hiding this comment.
Two findings on the new verdict presentation, both in apps/web/src/components/pullRequest. The main one is that the reviewer-avatar verdict ring cannot render: it is painted into the exact band the avatar's own opaque ring-background separator fills, and colour is the only visible signal on that row.
Posted via Macroscope — UI Consistency
02c764b to
fa27a76
Compare
There was a problem hiding this comment.
One finding on the reviewer avatar row: the new p-0.5 wrapper makes the verdict ring visible but cancels the stack's overlap. Details inline.
Posted via Macroscope — UI Consistency
fa27a76 to
220673d
Compare
There was a problem hiding this comment.
One finding on the reviewer avatar row: the verdict ring now paints an opaque 6px halo around a 16px avatar that overlaps its neighbours by 4px, so an approving reviewer largely eclipses the avatar to their left. Details inline.
Posted via Macroscope — UI Consistency
220673d to
7b610fa
Compare
7b610fa to
ffa10c5
Compare
Dismissing prior approval to re-evaluate ffa10c5
ffa10c5 to
cf1dbda
Compare
Dismissing prior approval to re-evaluate cf1dbda
There was a problem hiding this comment.
Two colour-handling issues in the new verdict presentation; both are theme/stack regressions rather than style preferences.
Posted via Macroscope — UI Consistency
cf1dbda to
6b27837
Compare
An approval used to arrive as a line inside a collapsed "9 comments" section, wearing a grey lowercase word and an empty markdown block where its body would be. Whether a change is approved is the question the page is opened with, and answering it took a press and a squint. Verdicts now read the way a host's own page reads them: - `pullRequestReviewOutcome` normalises the three verdict states across hosts (GitHub `CHANGES_REQUESTED`, Bitbucket `changes_requested`), and `latestPullRequestReviewOutcomes` keeps one entry per reviewer — their last word, with a dismissal clearing it rather than replacing it. It replaces the ad-hoc lowercased comparisons the summary made inline. - The timeline gives a verdict its own row beside commits and merges, with the reviewer's avatar on the rail, a toned icon, and "approved these changes". A review carrying only remarks still groups as before, and the row keeps the reaction bar it had inside the group. - The summary badges the verdict instead of printing "Approved" in grey, and stops rendering an empty body block under it — except where this reader may rewrite the remark, since the edit pencil lives in there. - The reviewers' verdicts get a "Reviews" row above the conversation, and the timeline header counts approvals. - A bodiless review no longer offers "Fix in a thread": an approval names nothing to fix. The verdict is named in words everywhere it is drawn. The icon is decorative, and a name written onto a generic span is not announced, so the "Reviews" row carries the label as text rather than as an aria-label the reader would never hear. Green is the green a passing check already wears in the same panel, taken from one shared presentation map. Known gap: GitLab and Azure DevOps report no verdict with their comments, so both new summaries stay hidden there rather than claiming nobody has approved.
6b27837 to
1bcde52
Compare
The header's approval count included verdicts the branch had already moved past, so a change request could report an approval in a number while the reviewer who gave it sat dimmed in the row underneath. The count now takes only approvals that still stand, and the two surfaces agree.
Dismissing prior approval to re-evaluate 67fb978
## What's Changed * feat(web): make review verdicts legible in the pull request detail by @Bil0000 in pingdotgg/t3code#7077 **Full Changelog**: pingdotgg/t3code@v0.0.34-nightly.20260817.1116...v0.0.34-nightly.20260817.1119 Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.34-nightly.20260817.1119
…ingdotgg#7077) (cherry picked from commit c7e6d71)
An approval arrives in the pull request detail as a line inside a collapsed "9 comments" section, wearing a grey lowercase word and an empty markdown block where its body would be. Whether a change is approved is the question the page is opened with, and answering it takes a press and a squint.
This makes verdicts read the way the host's own page reads them.
Before
After
What changed
A verdict is a first-class timeline row
It sits beside commits and merges rather than folding into the conversation group, with the reviewer's avatar on the rail, a toned icon, and
octocat approved these changes. A review carrying only remarks (COMMENTED) still groups as ordinary conversation, so nothing that was readable before became noisier.The summary badges the verdict
Instead of printing
Approvedin grey next to the timestamp. The empty body block underneath it is gone — an approval usually carries no words, and the blank block read as a card somebody forgot to fill in. The block stays wherever this reader may rewrite the remark, since the edit pencil lives inside it.Where the reviewers landed is above the conversation
A
Reviewsmeta row besideReviewerslists each reviewer with their verdict, and the timeline header counts approvals — both derived from comments already loaded, no new request.No dead space under a verdict
The reaction bar's add button is invisible until hovered but still occupies
h-6+mt-2. Under a one-line verdict that reads as a hole rather than an affordance, so the bar is drawn only once a reaction exists.A bodiless review no longer offers "Fix in a thread"
Handing an empty review to a fresh thread promised work it does not describe. The whole-review sweep already skipped these; the per-comment button did not.
How it is derived
pullRequestReviewOutcomenormalises the three verdict states across hosts — GitHub reportsCHANGES_REQUESTED, Bitbucketchanges_requested— so case and separator are ignored, and anything else is not a verdict. It replaces the ad-hoccomment.reviewState?.toLowerCase()comparisons the summary tab was making inline.latestPullRequestReviewOutcomeskeeps one entry per reviewer and only their last word: a host keeps every review ever submitted, and an approval later followed by a request for changes is not an approval any more. It picks by timestamp rather than by read order, because not every host returns reviews sorted. A dismissal is a verdict taken back, so it clears the entry rather than showing itself.Known gap
GitLab and Azure DevOps report no verdict alongside their comments, so both new summaries stay hidden on those hosts rather than claiming nobody has approved. Surfacing GitLab approvals needs a new read in the server adapter and is left for its own change.
Verification
Rebased onto
main(20a70420a) and re-resolved against the newer pull request surface — collapsed resolved/dismissed comments, reactions, in-place editing.vp run -r testvp run -r typecheckvp lintSix files change, all under
apps/web/src/components/pullRequest/. No contract, server, provider or listing code is touched — this is presentation over data the detail view already carries.Note
Show review verdicts as colored badges, avatar rings, and timeline rows in pull request detail
pullRequestReviewOutcomemaps host-specific review states to a canonical set (approved,changes-requested,dismissed),latestPullRequestReviewOutcomesdeduplicates per-reviewer, marks stale verdicts, and filters dismissed ones.ReviewVerdictEventrows with icons, colored labels, staleness indicators, and reaction bars, instead of hiding them inside collapsed conversations.groupPullRequestTimelineConversationsno longer collapses verdict events into comment groups; they appear as separate timeline rows.Macroscope summarized 6068440.