Skip to content

Commit a2ca118

Browse files
committed
Fix jumping to a review when submitted
1 parent 2a4ced0 commit a2ca118

File tree

4 files changed

+146
-12
lines changed

4 files changed

+146
-12
lines changed

src/browser/components/pr-overview.tsx

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,31 @@ export const PROverview = memo(function PROverview() {
179179
document.title = `${pr.title} · Pull Request #${pr.number} · Pulldash`;
180180
}, [pr.title, pr.number]);
181181

182+
// Scroll to target when overviewScrollTarget changes
183+
const overviewScrollTarget = usePRReviewSelector(
184+
(s) => s.overviewScrollTarget
185+
);
186+
useEffect(() => {
187+
if (!overviewScrollTarget) return;
188+
189+
// Small delay to ensure the element is rendered
190+
const timer = setTimeout(() => {
191+
const element = document.getElementById(overviewScrollTarget);
192+
if (element) {
193+
element.scrollIntoView({ behavior: "smooth", block: "start" });
194+
// Flash highlight effect
195+
element.classList.add("ring-2", "ring-blue-500/50");
196+
setTimeout(() => {
197+
element.classList.remove("ring-2", "ring-blue-500/50");
198+
}, 2000);
199+
}
200+
// Clear the scroll target after scrolling
201+
store.clearOverviewScrollTarget();
202+
}, 100);
203+
204+
return () => clearTimeout(timer);
205+
}, [overviewScrollTarget, store]);
206+
182207
// Manual refresh handler
183208
const handleRefreshChecks = useCallback(async () => {
184209
setRefreshingChecks(true);
@@ -832,6 +857,7 @@ export const PROverview = memo(function PROverview() {
832857
return (
833858
<CommentBox
834859
key={`comment-${comment.id}`}
860+
id={`issuecomment-${comment.id}`}
835861
user={comment.user}
836862
createdAt={comment.created_at}
837863
body={comment.body ?? null}
@@ -1789,6 +1815,7 @@ function LabelsSection({
17891815
// ============================================================================
17901816

17911817
function CommentBox({
1818+
id,
17921819
user,
17931820
createdAt,
17941821
body,
@@ -1798,6 +1825,7 @@ function CommentBox({
17981825
onRemoveReaction,
17991826
currentUser,
18001827
}: {
1828+
id?: string;
18011829
user: { login: string; avatar_url: string } | null;
18021830
createdAt: string;
18031831
body: string | null;
@@ -1810,7 +1838,7 @@ function CommentBox({
18101838
if (!user) return null;
18111839

18121840
return (
1813-
<div className="border border-border rounded-md overflow-hidden">
1841+
<div id={id} className="border border-border rounded-md overflow-hidden">
18141842
{/* Header */}
18151843
<div
18161844
className={cn(
@@ -1890,7 +1918,10 @@ function ReviewBox({ review }: { review: Review }) {
18901918
}[review.state] || "bg-card/50";
18911919

18921920
return (
1893-
<div className={cn("border rounded-md overflow-hidden", stateBg)}>
1921+
<div
1922+
id={`pullrequestreview-${review.id}`}
1923+
className={cn("border rounded-md overflow-hidden", stateBg)}
1924+
>
18941925
<div className="flex items-center gap-2 px-4 py-2 text-sm border-b border-border">
18951926
<UserHoverCard login={review.user.login}>
18961927
<img

src/browser/contexts/pr-review/index.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,3 +635,45 @@ test("navigateFromHash returns false for invalid file", () => {
635635

636636
expect(result).toBe(false);
637637
});
638+
639+
test("navigateFromHash handles GitHub-style pullrequestreview hash", () => {
640+
const store = createStore();
641+
store.selectFile("src/index.ts"); // Start on a file view
642+
643+
const result = store.navigateFromHash("#pullrequestreview-12345");
644+
645+
expect(result).toBe(true);
646+
const state = store.getSnapshot();
647+
expect(state.showOverview).toBe(true);
648+
expect(state.overviewScrollTarget).toBe("pullrequestreview-12345");
649+
});
650+
651+
test("navigateFromHash handles GitHub-style issuecomment hash", () => {
652+
const store = createStore();
653+
store.selectFile("src/index.ts");
654+
655+
const result = store.navigateFromHash("#issuecomment-98765");
656+
657+
expect(result).toBe(true);
658+
const state = store.getSnapshot();
659+
expect(state.showOverview).toBe(true);
660+
expect(state.overviewScrollTarget).toBe("issuecomment-98765");
661+
});
662+
663+
test("getHashFromState returns overview scroll target when on overview", () => {
664+
const store = createStore();
665+
store.selectOverview("pullrequestreview-12345");
666+
667+
const hash = store.getHashFromState();
668+
669+
expect(hash).toBe("pullrequestreview-12345");
670+
});
671+
672+
test("clearOverviewScrollTarget clears the target", () => {
673+
const store = createStore();
674+
store.selectOverview("pullrequestreview-12345");
675+
676+
store.clearOverviewScrollTarget();
677+
678+
expect(store.getSnapshot().overviewScrollTarget).toBeNull();
679+
});

src/browser/contexts/pr-review/index.tsx

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,8 @@ interface PRReviewState {
194194
selectedFile: string | null;
195195
selectedFiles: Set<string>;
196196
showOverview: boolean;
197+
// Overview scroll target (GitHub-style hash: pullrequestreview-{id}, issuecomment-{id}, etc.)
198+
overviewScrollTarget: string | null;
197199

198200
// Viewed files
199201
viewedFiles: Set<string>;
@@ -357,6 +359,7 @@ export class PRReviewStore {
357359
selectedFile: null,
358360
selectedFiles: new Set(),
359361
showOverview: true,
362+
overviewScrollTarget: null,
360363
viewedFiles,
361364
hideViewed: true,
362365
diffViewMode,
@@ -421,10 +424,16 @@ export class PRReviewStore {
421424
// File Navigation Actions
422425
// ---------------------------------------------------------------------------
423426

424-
selectOverview = () => {
425-
if (this.state.showOverview) return;
427+
selectOverview = (scrollTarget?: string) => {
428+
// If already on overview and just updating scroll target
429+
if (this.state.showOverview && scrollTarget) {
430+
this.set({ overviewScrollTarget: scrollTarget });
431+
return;
432+
}
433+
if (this.state.showOverview && !scrollTarget) return;
426434
this.set({
427435
showOverview: true,
436+
overviewScrollTarget: scrollTarget ?? null,
428437
selectedFile: null,
429438
selectedFiles: new Set(),
430439
focusedLine: null,
@@ -442,6 +451,12 @@ export class PRReviewStore {
442451
});
443452
};
444453

454+
clearOverviewScrollTarget = () => {
455+
if (this.state.overviewScrollTarget) {
456+
this.set({ overviewScrollTarget: null });
457+
}
458+
};
459+
445460
selectFile = (filename: string) => {
446461
if (this.state.selectedFile === filename && !this.state.showOverview)
447462
return;
@@ -1474,6 +1489,10 @@ export class PRReviewStore {
14741489
this.recomputeCommentRangeLookup();
14751490
};
14761491

1492+
setReviews = (reviews: Review[]) => {
1493+
this.set({ reviews });
1494+
};
1495+
14771496
setPr = (pr: PullRequest) => {
14781497
this.set({ pr });
14791498
};
@@ -1680,6 +1699,7 @@ export class PRReviewStore {
16801699
/**
16811700
* Get the current navigation state as a URL hash string.
16821701
* Format: #file=<path>&L<line> or #file=<path>&L<start>-<end> or #file=<path>&C<commentId>
1702+
* Also supports GitHub-style: #pullrequestreview-{id} or #issuecomment-{id}
16831703
*/
16841704
getHashFromState = (): string => {
16851705
const {
@@ -1688,8 +1708,15 @@ export class PRReviewStore {
16881708
selectionAnchor,
16891709
focusedCommentId,
16901710
focusedPendingCommentId,
1711+
showOverview,
1712+
overviewScrollTarget,
16911713
} = this.state;
16921714

1715+
// If we're on overview with a scroll target, use GitHub-style hash
1716+
if (showOverview && overviewScrollTarget) {
1717+
return overviewScrollTarget;
1718+
}
1719+
16931720
if (!selectedFile) return "";
16941721

16951722
const params = new URLSearchParams();
@@ -1716,6 +1743,7 @@ export class PRReviewStore {
17161743
/**
17171744
* Navigate to a state from a URL hash string.
17181745
* Returns true if navigation was performed.
1746+
* Supports GitHub-style hashes: #pullrequestreview-{id}, #issuecomment-{id}, #discussion_r{id}
17191747
*/
17201748
navigateFromHash = (hash: string): boolean => {
17211749
if (!hash) return false;
@@ -1724,6 +1752,17 @@ export class PRReviewStore {
17241752
const hashStr = hash.startsWith("#") ? hash.slice(1) : hash;
17251753
if (!hashStr) return false;
17261754

1755+
// Check for GitHub-style overview hashes first
1756+
const reviewMatch = hashStr.match(/^pullrequestreview-(\d+)$/);
1757+
const commentMatch = hashStr.match(/^issuecomment-(\d+)$/);
1758+
const discussionMatch = hashStr.match(/^discussion_r(\d+)$/);
1759+
1760+
if (reviewMatch || commentMatch || discussionMatch) {
1761+
// Navigate to overview with scroll target
1762+
this.selectOverview(hashStr);
1763+
return true;
1764+
}
1765+
17271766
const params = new URLSearchParams(hashStr);
17281767
const file = params.get("file");
17291768
const lineParam = params.get("L");

src/browser/contexts/pr-review/useReviewActions.ts

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { ReviewComment } from "@/api/types";
2-
import { useGitHub } from "@/browser/contexts/github";
2+
import { useGitHub, type Review } from "@/browser/contexts/github";
33
import { useTelemetry } from "@/browser/contexts/telemetry";
44
import { usePRReviewStore, usePRReviewSelector } from ".";
55

@@ -17,16 +17,18 @@ export function useReviewActions() {
1717
const state = store.getSnapshot();
1818
store.setSubmittingReview(true);
1919

20+
let newReview: Review | null = null;
21+
2022
try {
2123
// Get the pending review node ID (from GraphQL)
2224
const reviewNodeId = store.getPendingReviewNodeId();
2325

2426
if (reviewNodeId) {
25-
// Submit via GraphQL
27+
// Submit via GraphQL - we'll find the review ID after refreshing
2628
await github.submitPendingReview(reviewNodeId, event, state.reviewBody);
2729
} else if (state.pendingComments.length > 0) {
2830
// Fallback: create a new review with all comments via REST
29-
await github.createPRReview(owner, repo, pr.number, {
31+
newReview = await github.createPRReview(owner, repo, pr.number, {
3032
commit_id: pr.head.sha,
3133
event,
3234
body: state.reviewBody,
@@ -42,7 +44,7 @@ export function useReviewActions() {
4244
});
4345
} else {
4446
// Just submitting a review with no comments (APPROVE, etc)
45-
await github.createPRReview(owner, repo, pr.number, {
47+
newReview = await github.createPRReview(owner, repo, pr.number, {
4648
commit_id: pr.head.sha,
4749
event,
4850
body: state.reviewBody,
@@ -60,14 +62,34 @@ export function useReviewActions() {
6062
files_reviewed: state.viewedFiles.size,
6163
});
6264

63-
// Refresh comments
64-
const newComments = await github.getPRComments(owner, repo, pr.number);
65+
// Refresh comments and reviews
66+
const [newComments, reviews] = await Promise.all([
67+
github.getPRComments(owner, repo, pr.number),
68+
github.getPRReviews(owner, repo, pr.number),
69+
]);
6570
store.setComments(newComments as ReviewComment[]);
71+
store.setReviews(reviews);
72+
73+
// If we got the review ID from REST, use it; otherwise find the latest review
74+
let scrollTarget: string | undefined;
75+
if (newReview?.id) {
76+
scrollTarget = `pullrequestreview-${newReview.id}`;
77+
} else if (reviews.length > 0) {
78+
// Find the most recent review (likely the one we just submitted)
79+
const sortedReviews = [...reviews].sort(
80+
(a, b) =>
81+
new Date(b.submitted_at ?? 0).getTime() -
82+
new Date(a.submitted_at ?? 0).getTime()
83+
);
84+
if (sortedReviews[0]) {
85+
scrollTarget = `pullrequestreview-${sortedReviews[0].id}`;
86+
}
87+
}
6688

6789
store.clearReviewState();
6890

69-
// Navigate to overview page after successful review submission
70-
store.selectOverview();
91+
// Navigate to overview page and scroll to the new review
92+
store.selectOverview(scrollTarget);
7193
} finally {
7294
store.setSubmittingReview(false);
7395
}

0 commit comments

Comments
 (0)