Skip to content

Commit bec93b9

Browse files
committed
Various review overview improvements to align with github
1 parent c667649 commit bec93b9

File tree

4 files changed

+92
-32
lines changed

4 files changed

+92
-32
lines changed

src/browser/components/pr-overview.tsx

Lines changed: 78 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -790,9 +790,16 @@ export const PROverview = memo(function PROverview() {
790790
});
791791
});
792792

793-
// Add reviews with bodies
794-
latestReviews
795-
.filter((r) => r.body && r.submitted_at)
793+
// Add ALL reviews to timeline - show APPROVED/CHANGES_REQUESTED always, COMMENTED only if they have a body
794+
// Note: we use `reviews` not `latestReviews` because latestReviews only keeps one review per user
795+
reviews
796+
.filter(
797+
(r) =>
798+
r.submitted_at &&
799+
(r.body ||
800+
r.state === "APPROVED" ||
801+
r.state === "CHANGES_REQUESTED")
802+
)
796803
.forEach((review) => {
797804
entries.push({
798805
type: "review",
@@ -936,6 +943,8 @@ export const PROverview = memo(function PROverview() {
936943
setShowMergeOptions(!showMergeOptions)
937944
}
938945
onUpdateBranch={handleUpdateBranch}
946+
markingReady={markingReady}
947+
onMarkReadyForReview={handleMarkReadyForReview}
939948
/>
940949
{/* Still in progress - only show if NOT a draft and user can merge */}
941950
{canMergeRepo && !pr.draft && (
@@ -1446,24 +1455,6 @@ export const PROverview = memo(function PROverview() {
14461455
canWrite={canMergeRepo}
14471456
/>
14481457

1449-
{/* Draft - Mark as ready for review */}
1450-
{canMergeRepo && pr.state === "open" && !pr.merged && pr.draft && (
1451-
<div className="pt-2 border-t border-border">
1452-
<p className="text-xs text-muted-foreground mb-1">
1453-
This pull request is still a work in progress.
1454-
</p>
1455-
<button
1456-
onClick={handleMarkReadyForReview}
1457-
disabled={markingReady}
1458-
className="text-sm text-blue-400 hover:underline disabled:opacity-50"
1459-
>
1460-
{markingReady
1461-
? "Marking ready..."
1462-
: "Mark as ready for review"}
1463-
</button>
1464-
</div>
1465-
)}
1466-
14671458
{/* Participants */}
14681459
<SidebarSection
14691460
title={`${participants.length} participant${participants.length !== 1 ? "s" : ""}`}
@@ -2121,6 +2112,8 @@ function MergeSection({
21212112
onSetMergeMethod,
21222113
onToggleMergeOptions,
21232114
onUpdateBranch,
2115+
markingReady,
2116+
onMarkReadyForReview,
21242117
}: {
21252118
pr: {
21262119
draft?: boolean;
@@ -2141,6 +2134,8 @@ function MergeSection({
21412134
onSetMergeMethod: (method: "merge" | "squash" | "rebase") => void;
21422135
onToggleMergeOptions: () => void;
21432136
onUpdateBranch: () => void;
2137+
markingReady?: boolean;
2138+
onMarkReadyForReview?: () => void;
21442139
}) {
21452140
const buttonRef = useRef<HTMLButtonElement>(null);
21462141
const [dropdownPosition, setDropdownPosition] = useState({
@@ -2518,8 +2513,40 @@ function MergeSection({
25182513
</div>
25192514
</div>
25202515

2521-
{/* Merge controls - only show when user can merge */}
2522-
{canMergeRepo && (
2516+
{/* Draft section - show when PR is a draft */}
2517+
{pr.draft && (
2518+
<div className="p-4">
2519+
<div className="flex items-start gap-3">
2520+
<div className="p-2 rounded-full bg-muted text-muted-foreground">
2521+
<GitPullRequest className="w-5 h-5" />
2522+
</div>
2523+
<div className="flex-1">
2524+
<p className="font-medium text-sm">
2525+
This pull request is still a work in progress
2526+
</p>
2527+
<p className="text-xs text-muted-foreground">
2528+
Draft pull requests cannot be merged.
2529+
</p>
2530+
</div>
2531+
{canMergeRepo && onMarkReadyForReview && (
2532+
<button
2533+
onClick={onMarkReadyForReview}
2534+
disabled={markingReady}
2535+
className="px-3 py-1.5 border border-border rounded-md hover:bg-muted transition-colors text-sm font-medium disabled:opacity-50"
2536+
>
2537+
{markingReady ? (
2538+
<Loader2 className="w-4 h-4 animate-spin" />
2539+
) : (
2540+
"Ready for review"
2541+
)}
2542+
</button>
2543+
)}
2544+
</div>
2545+
</div>
2546+
)}
2547+
2548+
{/* Merge controls - only show when user can merge and PR is not a draft */}
2549+
{canMergeRepo && !pr.draft && (
25232550
<div className="p-4 space-y-3">
25242551
{mergeError && (
25252552
<p className="text-sm text-destructive">{mergeError}</p>
@@ -3496,6 +3523,34 @@ function TimelineItem({ event, pr }: TimelineItemProps) {
34963523
};
34973524
}
34983525

3526+
case "head_ref_force_pushed": {
3527+
// Timeline API only provides commit_id (the "to" SHA), no "before" SHA
3528+
const forcePush = event as { commit_id?: string };
3529+
return {
3530+
icon: <GitBranch className="w-4 h-4" />,
3531+
text: (
3532+
<span>
3533+
<span className="font-medium">{actor?.login}</span> force-pushed
3534+
the{" "}
3535+
<code className="px-1.5 py-0.5 bg-blue-500/20 text-blue-300 rounded text-xs">
3536+
{pr?.head?.ref || "branch"}
3537+
</code>{" "}
3538+
branch
3539+
{forcePush.commit_id && (
3540+
<>
3541+
{" "}
3542+
to{" "}
3543+
<code className="px-1.5 py-0.5 bg-muted rounded text-xs font-mono">
3544+
{forcePush.commit_id.slice(0, 7)}
3545+
</code>
3546+
</>
3547+
)}
3548+
</span>
3549+
),
3550+
color: "text-amber-400",
3551+
};
3552+
}
3553+
34993554
case "merged": {
35003555
const merged = event as { commit_id?: string };
35013556
return {

src/browser/components/pr-review.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2902,7 +2902,7 @@ const SubmitReviewDropdown = memo(function SubmitReviewDropdown() {
29022902
<label className="flex items-start gap-3 cursor-pointer group">
29032903
<RadioGroupItem value="APPROVE" className="mt-0.5" />
29042904
<div className="flex flex-col gap-0.5">
2905-
<span className="font-medium text-sm text-green-500">
2905+
<span className="font-medium text-sm text-green-400">
29062906
Approve
29072907
</span>
29082908
<span className="text-xs text-muted-foreground">
@@ -2914,7 +2914,7 @@ const SubmitReviewDropdown = memo(function SubmitReviewDropdown() {
29142914
<label className="flex items-start gap-3 cursor-pointer group">
29152915
<RadioGroupItem value="REQUEST_CHANGES" className="mt-0.5" />
29162916
<div className="flex flex-col gap-0.5">
2917-
<span className="font-medium text-sm text-orange-500">
2917+
<span className="font-medium text-sm text-amber-400">
29182918
Request changes
29192919
</span>
29202920
<span className="text-xs text-muted-foreground">
@@ -2977,9 +2977,9 @@ const SubmitReviewDropdown = memo(function SubmitReviewDropdown() {
29772977
className={cn(
29782978
"flex items-center gap-1.5 px-2 py-1 text-xs font-medium rounded-md transition-colors disabled:opacity-50",
29792979
reviewType === "APPROVE" &&
2980-
"bg-green-600 text-white hover:bg-green-700",
2980+
"bg-green-500/20 text-green-400 hover:bg-green-500/30 border border-green-500/30",
29812981
reviewType === "REQUEST_CHANGES" &&
2982-
"bg-orange-600 text-white hover:bg-orange-700",
2982+
"bg-amber-500/20 text-amber-400 hover:bg-amber-500/30 border border-amber-500/30",
29832983
reviewType === "COMMENT" &&
29842984
"bg-primary text-primary-foreground hover:bg-primary/90"
29852985
)}

src/browser/contexts/pr-review.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2840,6 +2840,9 @@ export function useReviewActions() {
28402840
store.setComments(newComments as ReviewComment[]);
28412841

28422842
store.clearReviewState();
2843+
2844+
// Navigate to overview page after successful review submission
2845+
store.selectOverview();
28432846
} finally {
28442847
store.setSubmittingReview(false);
28452848
}

src/browser/index.css

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@
409409
font-size: 0.85em;
410410
padding: 0.2em 0.4em;
411411
margin: 0;
412-
background-color: oklch(0.25 0 0);
412+
background-color: oklch(0.22 0.01 260 / 80%);
413413
border-radius: 4px;
414414
}
415415

@@ -420,19 +420,21 @@
420420
overflow: auto;
421421
font-size: 0.85em;
422422
line-height: 1.45;
423-
background-color: oklch(0.16 0 0);
423+
background-color: oklch(0.18 0.01 260 / 60%);
424424
border-radius: 6px;
425-
border: 1px solid var(--border);
425+
border: 1px solid oklch(1 0 0 / 8%);
426426
}
427427

428-
.markdown-body pre code {
428+
.markdown-body pre code,
429+
.markdown-body pre code[class*="language-"],
430+
.markdown-body pre code.hljs {
429431
display: block;
430432
padding: 0;
431433
margin: 0;
432434
overflow: visible;
433435
line-height: inherit;
434436
word-wrap: normal;
435-
background-color: transparent;
437+
background: transparent !important;
436438
border: 0;
437439
font-size: 100%;
438440
}

0 commit comments

Comments
 (0)