| author | |
| committer | |
| log | 75fca7d77cd4a5f72167606f248131edb633bb50 |
| tree | 081576c94b5c9ea3ea65ab83cac8d1b4bc790ab6 |
| parent | 6c130163941618480da333979bf6fc675d803e4f |
| signature | Signed by PGP key B5690EEEBB952194 |
6 files changed, 167 insertions(+), 43 deletions(-)
apps/web/src/components/pullRequest/PullRequestCodeTab.tsx+6-1| ... | ... | @@ -196,6 +196,7 @@ function getReviewPositionAnchor(position: PullRequestReviewPosition): { |
| 196 | 196 | * drafted rather than being posted as it is typed. |
| 197 | 197 | */ |
| 198 | 198 | function PullRequestCodeTab({ |
| 199 | scrollerRef, | |
| 199 | 200 | environmentId, |
| 200 | 201 | reference, |
| 201 | 202 | detail, |
| ... | ... | @@ -208,6 +209,7 @@ function PullRequestCodeTab({ |
| 208 | 209 | onRefresh, |
| 209 | 210 | refreshToken = 0, |
| 210 | 211 | }: { |
| 212 | scrollerRef: (node: HTMLDivElement | null) => void; | |
| 211 | 213 | environmentId: EnvironmentId; |
| 212 | 214 | reference: PullRequestRef; |
| 213 | 215 | detail: PullRequestDetailView; |
| ... | ... | @@ -1394,7 +1396,9 @@ function PullRequestCodeTab({ |
| 1394 | 1396 | const withToolbar = (body: ReactNode) => ( |
| 1395 | 1397 | <div className="flex h-full min-h-0 flex-col"> |
| 1396 | 1398 | {toolbar} |
| 1397 | <div className="min-h-0 flex-1 overflow-auto">{body}</div> | |
| 1399 | <div ref={scrollerRef} className="min-h-0 flex-1 overflow-auto"> | |
| 1400 | {body} | |
| 1401 | </div> | |
| 1398 | 1402 | </div> |
| 1399 | 1403 | ); |
| 1400 | 1404 | |
| ... | ... | @@ -1574,6 +1578,7 @@ function PullRequestCodeTab({ |
| 1574 | 1578 | // interaction, but its native host outline clips and competes with the focus |
| 1575 | 1579 | // indicators on its actual controls. |
| 1576 | 1580 | className="@container h-full overflow-auto [scrollbar-gutter:stable]" |
| 1581 | containerRef={scrollerRef} | |
| 1577 | 1582 | viewerRef={setViewer} |
| 1578 | 1583 | items={items} |
| 1579 | 1584 | selectedLines={selectedLines} |
apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx+115-30| ... | ... | @@ -108,43 +108,49 @@ vi.mock("./PullRequestThreadLinks", () => ({ PullRequestThreadLinks: () => null |
| 108 | 108 | vi.mock("./PullRequestSummaryTab", () => ({ |
| 109 | 109 | PullRequestSummaryTab: ({ |
| 110 | 110 | onFixFinding, |
| 111 | scrollerRef, | |
| 111 | 112 | }: ComponentProps<typeof import("./PullRequestSummaryTab").PullRequestSummaryTab>) => ( |
| 112 | <button | |
| 113 | onClick={() => | |
| 114 | onFixFinding?.({ | |
| 115 | kind: "check", | |
| 116 | check: { name: "Unit tests", status: "failure", description: "Test failed", url: null }, | |
| 117 | }) | |
| 118 | } | |
| 119 | > | |
| 120 | Fix check | |
| 121 | </button> | |
| 113 | <div ref={scrollerRef} data-test-scroll="summary"> | |
| 114 | <button | |
| 115 | onClick={() => | |
| 116 | onFixFinding?.({ | |
| 117 | kind: "check", | |
| 118 | check: { name: "Unit tests", status: "failure", description: "Test failed", url: null }, | |
| 119 | }) | |
| 120 | } | |
| 121 | > | |
| 122 | Fix check | |
| 123 | </button> | |
| 124 | </div> | |
| 122 | 125 | ), |
| 123 | 126 | })); |
| 124 | 127 | vi.mock("./PullRequestCodeTab", () => ({ |
| 125 | 128 | default: ({ |
| 126 | 129 | onAddToAgentSelection, |
| 130 | scrollerRef, | |
| 127 | 131 | }: ComponentProps<typeof import("./PullRequestCodeTab").default>) => ( |
| 128 | <button | |
| 129 | onClick={() => | |
| 130 | onAddToAgentSelection?.({ | |
| 131 | request: "Fix this line", | |
| 132 | comment: { | |
| 133 | id: "note-1", | |
| 134 | sectionId: "file:a.ts", | |
| 135 | sectionTitle: "a.ts", | |
| 136 | filePath: "a.ts", | |
| 137 | startIndex: 0, | |
| 138 | endIndex: 0, | |
| 139 | rangeLabel: "L1", | |
| 140 | text: "Please fix", | |
| 141 | diff: "+broken()", | |
| 142 | }, | |
| 143 | }) | |
| 144 | } | |
| 145 | > | |
| 146 | Add to agent | |
| 147 | </button> | |
| 132 | <div ref={scrollerRef} data-test-scroll="code"> | |
| 133 | <button | |
| 134 | onClick={() => | |
| 135 | onAddToAgentSelection?.({ | |
| 136 | request: "Fix this line", | |
| 137 | comment: { | |
| 138 | id: "note-1", | |
| 139 | sectionId: "file:a.ts", | |
| 140 | sectionTitle: "a.ts", | |
| 141 | filePath: "a.ts", | |
| 142 | startIndex: 0, | |
| 143 | endIndex: 0, | |
| 144 | rangeLabel: "L1", | |
| 145 | text: "Please fix", | |
| 146 | diff: "+broken()", | |
| 147 | }, | |
| 148 | }) | |
| 149 | } | |
| 150 | > | |
| 151 | Add to agent | |
| 152 | </button> | |
| 153 | </div> | |
| 148 | 154 | ), |
| 149 | 155 | })); |
| 150 | 156 | |
| ... | ... | @@ -343,3 +349,82 @@ describe.each([ |
| 343 | 349 | } |
| 344 | 350 | }); |
| 345 | 351 | }); |
| 352 | ||
| 353 | it("changes the title only for the active tab's main scroller", async () => { | |
| 354 | const scrollers = new Map<string, { scrollTop: number; scrollHeight: number }>(); | |
| 355 | await act(async () => { | |
| 356 | renderer = create( | |
| 357 | <PullRequestDetailPanel | |
| 358 | environmentId={threadRef.environmentId} | |
| 359 | reference={detail} | |
| 360 | context="page" | |
| 361 | shortcutsEnabled={false} | |
| 362 | getShortcutContext={() => ({ | |
| 363 | terminalFocus: false, | |
| 364 | terminalOpen: false, | |
| 365 | previewFocus: false, | |
| 366 | previewOpen: false, | |
| 367 | isWeb: true, | |
| 368 | isDesktop: false, | |
| 369 | })} | |
| 370 | />, | |
| 371 | { | |
| 372 | createNodeMock: (element) => { | |
| 373 | const props = element.props as Record<string, unknown>; | |
| 374 | const name = props["data-test-scroll"] as string | undefined; | |
| 375 | if (name) { | |
| 376 | const scroller = { scrollTop: 0, scrollHeight: 300 }; | |
| 377 | scrollers.set(name, scroller); | |
| 378 | return scroller; | |
| 379 | } | |
| 380 | return { scrollHeight: props.inert === false ? 100 : 20 }; | |
| 381 | }, | |
| 382 | }, | |
| 383 | ); | |
| 384 | }); | |
| 385 | ||
| 386 | const mainScroll = renderer.root.findByProps({ | |
| 387 | className: "relative flex min-h-0 flex-1 flex-col overflow-hidden", | |
| 388 | }); | |
| 389 | const titleFold = () => { | |
| 390 | let node = renderer.root.findByType("h1"); | |
| 391 | while (typeof node.props.inert !== "boolean") node = node.parent!; | |
| 392 | return node; | |
| 393 | }; | |
| 394 | const summary = scrollers.get("summary")!; | |
| 395 | const nested = { scrollTop: 200 }; | |
| 396 | await act(async () => mainScroll.props.onScrollCapture({ target: nested })); | |
| 397 | expect(titleFold().props.inert).toBe(false); | |
| 398 | summary.scrollTop = 180; | |
| 399 | nested.scrollTop = 0; | |
| 400 | await act(async () => { | |
| 401 | mainScroll.props.onScrollCapture({ target: summary }); | |
| 402 | mainScroll.props.onScrollCapture({ target: nested }); | |
| 403 | }); | |
| 404 | expect(titleFold().props.inert).toBe(true); | |
| 405 | expect(summary.scrollTop).toBe(100); | |
| 406 | expect(nested.scrollTop).toBe(0); | |
| 407 | ||
| 408 | await act(async () => mainScroll.props.onScrollCapture({ target: { scrollTop: 0 } })); | |
| 409 | expect(titleFold().props.inert).toBe(true); | |
| 410 | summary.scrollTop = 0; | |
| 411 | await act(async () => mainScroll.props.onScrollCapture({ target: summary })); | |
| 412 | expect(titleFold().props.inert).toBe(false); | |
| 413 | ||
| 414 | await click("Code"); | |
| 415 | const code = scrollers.get("code")!; | |
| 416 | summary.scrollTop = 180; | |
| 417 | await act(async () => mainScroll.props.onScrollCapture({ target: summary })); | |
| 418 | expect(titleFold().props.inert).toBe(false); | |
| 419 | code.scrollTop = 180; | |
| 420 | await act(async () => mainScroll.props.onScrollCapture({ target: code })); | |
| 421 | expect(titleFold().props.inert).toBe(true); | |
| 422 | expect(code.scrollTop).toBe(100); | |
| 423 | await click("Summary"); | |
| 424 | expect(titleFold().props.inert).toBe(false); | |
| 425 | await click("Code"); | |
| 426 | expect(titleFold().props.inert).toBe(true); | |
| 427 | code.scrollTop = 0; | |
| 428 | await act(async () => mainScroll.props.onScrollCapture({ target: code })); | |
| 429 | expect(titleFold().props.inert).toBe(false); | |
| 430 | }); |
apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx+39-10| ... | ... | @@ -526,18 +526,44 @@ export function PullRequestDetailPanel({ |
| 526 | 526 | setChromeCondensed(chromeStateByTab.current[tab] ?? false); |
| 527 | 527 | }, [tab]); |
| 528 | 528 | const condensed = chromeCondensed; |
| 529 | const scrollerRef = useRef<HTMLElement | null>(null); | |
| 529 | const scrollerByTab = useRef<Partial<Record<DetailTab, HTMLDivElement | null>>>({}); | |
| 530 | const scrollerRefs = useMemo( | |
| 531 | () => ({ | |
| 532 | summary: (node: HTMLDivElement | null) => { | |
| 533 | scrollerByTab.current.summary = node; | |
| 534 | }, | |
| 535 | timeline: (node: HTMLDivElement | null) => { | |
| 536 | scrollerByTab.current.timeline = node; | |
| 537 | }, | |
| 538 | code: (node: HTMLDivElement | null) => { | |
| 539 | scrollerByTab.current.code = node; | |
| 540 | }, | |
| 541 | }), | |
| 542 | [], | |
| 543 | ); | |
| 530 | 544 | const foldRef = useRef<HTMLDivElement | null>(null); |
| 531 | 545 | const condensedRowRef = useRef<HTMLDivElement | null>(null); |
| 532 | 546 | // Refund after the fold commits so the content under the reader does not jump with its height. |
| 533 | const compensationRef = useRef<number | null>(null); | |
| 547 | const compensationRef = useRef<{ | |
| 548 | tab: DetailTab; | |
| 549 | scroller: HTMLDivElement; | |
| 550 | delta: number; | |
| 551 | } | null>(null); | |
| 534 | 552 | useLayoutEffect(() => { |
| 535 | if (compensationRef.current === null) return; | |
| 536 | const scroller = scrollerRef.current; | |
| 537 | const delta = compensationRef.current; | |
| 553 | const compensation = compensationRef.current; | |
| 554 | if (compensation === null) return; | |
| 538 | 555 | compensationRef.current = null; |
| 539 | if (scroller) scroller.scrollTop = Math.max(0, scroller.scrollTop + delta); | |
| 540 | }, [condensed]); | |
| 556 | if ( | |
| 557 | !condensed || | |
| 558 | compensation.tab !== tab || | |
| 559 | scrollerByTab.current[tab] !== compensation.scroller | |
| 560 | ) | |
| 561 | return; | |
| 562 | compensation.scroller.scrollTop = Math.max( | |
| 563 | 0, | |
| 564 | compensation.scroller.scrollTop + compensation.delta, | |
| 565 | ); | |
| 566 | }, [condensed, tab]); | |
| 541 | 567 | const lastSelectedMergeMethod = useUiStateStore((state) => state.pullRequestMergeMethod); |
| 542 | 568 | const setLastSelectedMergeMethod = useUiStateStore((state) => state.setPullRequestMergeMethod); |
| 543 | 569 | const resolveProjectDefaultMergeMethod = usePullRequestDefaultMergeMethodResolver( |
| ... | ... | @@ -2737,8 +2763,8 @@ export function PullRequestDetailPanel({ |
| 2737 | 2763 | <div |
| 2738 | 2764 | className="relative flex min-h-0 flex-1 flex-col overflow-hidden" |
| 2739 | 2765 | onScrollCapture={(event) => { |
| 2740 | const scroller = event.target as HTMLElement; | |
| 2741 | scrollerRef.current = scroller; | |
| 2766 | const scroller = scrollerByTab.current[tab]; | |
| 2767 | if (event.target !== scroller || scroller === null || scroller === undefined) return; | |
| 2742 | 2768 | const top = scroller.scrollTop; |
| 2743 | 2769 | setChromeCondensed((previous) => { |
| 2744 | 2770 | let next = previous; |
| ... | ... | @@ -2753,7 +2779,7 @@ export function PullRequestDetailPanel({ |
| 2753 | 2779 | next = false; |
| 2754 | 2780 | } |
| 2755 | 2781 | } else if (foldHeight > 0 && top > foldHeight + 32) { |
| 2756 | compensationRef.current = -chromeDelta; | |
| 2782 | compensationRef.current = { tab, scroller, delta: -chromeDelta }; | |
| 2757 | 2783 | next = true; |
| 2758 | 2784 | } |
| 2759 | 2785 | chromeStateByTab.current[tab] = next; |
| ... | ... | @@ -2779,6 +2805,7 @@ export function PullRequestDetailPanel({ |
| 2779 | 2805 | {mountedTabs.has("summary") ? ( |
| 2780 | 2806 | <div className={cn("absolute inset-0", tab !== "summary" && "invisible")}> |
| 2781 | 2807 | <PullRequestSummaryTab |
| 2808 | scrollerRef={scrollerRefs.summary} | |
| 2782 | 2809 | environmentId={environmentId} |
| 2783 | 2810 | threadRef={threadRef} |
| 2784 | 2811 | reference={reference} |
| ... | ... | @@ -2806,6 +2833,7 @@ export function PullRequestDetailPanel({ |
| 2806 | 2833 | /> |
| 2807 | 2834 | ) : ( |
| 2808 | 2835 | <PullRequestTimelineTab |
| 2836 | scrollerRef={scrollerRefs.timeline} | |
| 2809 | 2837 | detail={detail} |
| 2810 | 2838 | environmentId={environmentId} |
| 2811 | 2839 | threadRef={threadRef} |
| ... | ... | @@ -2821,6 +2849,7 @@ export function PullRequestDetailPanel({ |
| 2821 | 2849 | <div className={cn("absolute inset-0", tab !== "code" && "invisible")}> |
| 2822 | 2850 | <Suspense fallback={<DiffPanelLoadingState label="Loading pull request diff..." />}> |
| 2823 | 2851 | <PullRequestCodeTab |
| 2852 | scrollerRef={scrollerRefs.code} | |
| 2824 | 2853 | onAddToAgentSelection={addSelectionToAgent} |
| 2825 | 2854 | environmentId={environmentId} |
| 2826 | 2855 | reference={reference} |
apps/web/src/components/pullRequest/PullRequestSummaryTab.test.tsx+1| ... | ... | @@ -101,6 +101,7 @@ afterEach(() => { |
| 101 | 101 | function render(value = detail) { |
| 102 | 102 | return ( |
| 103 | 103 | <PullRequestSummaryTab |
| 104 | scrollerRef={() => {}} | |
| 104 | 105 | environmentId={EnvironmentId.make("environment")} |
| 105 | 106 | threadRef={null} |
| 106 | 107 | reference={value} |
apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx+3-1| ... | ... | @@ -376,6 +376,7 @@ const CommentFilterSchema = Schema.Struct({ bots: Schema.Boolean, resolved: Sche |
| 376 | 376 | const DEFAULT_COMMENT_FILTER: typeof CommentFilterSchema.Type = { bots: false, resolved: false }; |
| 377 | 377 | |
| 378 | 378 | export function PullRequestSummaryTab({ |
| 379 | scrollerRef, | |
| 379 | 380 | environmentId, |
| 380 | 381 | threadRef, |
| 381 | 382 | reference, |
| ... | ... | @@ -390,6 +391,7 @@ export function PullRequestSummaryTab({ |
| 390 | 391 | onRefresh, |
| 391 | 392 | onRefreshChecks = onRefresh, |
| 392 | 393 | }: { |
| 394 | scrollerRef: (node: HTMLDivElement | null) => void; | |
| 393 | 395 | environmentId: EnvironmentId; |
| 394 | 396 | threadRef: ScopedThreadRef | null; |
| 395 | 397 | reference: PullRequestRef; |
| ... | ... | @@ -657,7 +659,7 @@ export function PullRequestSummaryTab({ |
| 657 | 659 | }; |
| 658 | 660 | |
| 659 | 661 | return ( |
| 660 | <div className="h-full overflow-y-auto" data-pull-request-summary-scroll> | |
| 662 | <div ref={scrollerRef} className="h-full overflow-y-auto" data-pull-request-summary-scroll> | |
| 661 | 663 | <section className="px-4 pt-2.5 pb-1"> |
| 662 | 664 | <div className="space-y-2"> |
| 663 | 665 | <MetaRow icon={<UsersIcon className="size-3.5" />} label="Reviewers"> |
apps/web/src/components/pullRequest/PullRequestTimelineTab.tsx+3-1| ... | ... | @@ -538,6 +538,7 @@ function ReviewVerdictEvent({ |
| 538 | 538 | } |
| 539 | 539 | |
| 540 | 540 | export function PullRequestTimelineTab({ |
| 541 | scrollerRef, | |
| 541 | 542 | detail, |
| 542 | 543 | environmentId, |
| 543 | 544 | threadRef = null, |
| ... | ... | @@ -546,6 +547,7 @@ export function PullRequestTimelineTab({ |
| 546 | 547 | onOpenCommit, |
| 547 | 548 | onRefresh, |
| 548 | 549 | }: { |
| 550 | scrollerRef: (node: HTMLDivElement | null) => void; | |
| 549 | 551 | detail: PullRequestDetailView; |
| 550 | 552 | environmentId: EnvironmentId; |
| 551 | 553 | threadRef?: ScopedThreadRef | null; |
| ... | ... | @@ -577,7 +579,7 @@ export function PullRequestTimelineTab({ |
| 577 | 579 | }; |
| 578 | 580 | |
| 579 | 581 | return ( |
| 580 | <div className="scrollbar-gutter-both h-full overflow-y-auto px-4 py-5"> | |
| 582 | <div ref={scrollerRef} className="scrollbar-gutter-both h-full overflow-y-auto px-4 py-5"> | |
| 581 | 583 | <div className="mx-auto max-w-3xl"> |
| 582 | 584 | <div className="relative"> |
| 583 | 585 | <span aria-hidden className="absolute bottom-5 left-[15px] top-1 w-px bg-border/45" /> |