From e4e6b80902d45f485669b401846fa53d01f809a3 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Mon, 26 Oct 2026 07:46:08 -0700 Subject: [PATCH] refactor(web): pull request page reacts to turns and sweep Escape without effects The pull request page waited for a commit to notice that a turn finished, then reset a continued list to one full read. It also kept a document-wide Escape listener mounted for the page's whole lifetime, only to end a close sweep that is rarely running. The page now records which turn refreshes a continuation was read under and asks for the whole loaded list again during render when a later turn lands, so the stale continuation never commits next to the refreshed first pages. A close sweep registers its own capture-phase Escape scope with the shortcut dispatcher when it starts and releases it when it finishes. Effects in PullRequestsPage.tsx: 8 -> 6. The stats observer, the keyboard shortcut listener and the remaining effects stay; the refresh-token and stats changes that would remove them depend on state helpers not on main. Co-Authored-By: Claude Opus 5.5 --- .../pullRequest/PullRequestsPage.tsx | 84 +++++++++++-------- 1 file changed, 48 insertions(+), 36 deletions(-) diff --git a/apps/web/src/components/pullRequest/PullRequestsPage.tsx b/apps/web/src/components/pullRequest/PullRequestsPage.tsx index 7d4fee18c92d6..9e4de110fac23 100644 --- a/apps/web/src/components/pullRequest/PullRequestsPage.tsx +++ b/apps/web/src/components/pullRequest/PullRequestsPage.tsx @@ -78,6 +78,7 @@ import { RightPanelTabs } from "../RightPanelTabs"; import { isCommandPaletteOpen } from "~/commandPaletteBus"; import { isElectron } from "~/env"; import { resolveShortcutCommand, shortcutLabelForCommand } from "~/keybindings"; +import { registerCommands } from "~/shortcuts/dispatcher"; import { isTerminalFocused } from "~/lib/terminalFocus"; import { PanelLayoutControls, RightPanelMaximizeControl } from "../chat/PanelLayoutControls"; import { Button } from "../ui/button"; @@ -286,13 +287,21 @@ export function PullRequestsPage() { * host forever. */ regrown: ReadonlyArray; - }>({ key: filterKey, size: PAGE_SIZE, cursors: null, regrown: [] }); + /** The turn refreshes a continuation was read under; a later turn makes its cursors stale. */ + turn: string; + }>({ key: filterKey, size: PAGE_SIZE, cursors: null, regrown: [], turn: turnRefreshToken }); const pageSize = page.key === filterKey ? page.size : PAGE_SIZE; const sentCursors = page.key === filterKey ? page.cursors : null; const sentRegrown = page.key === filterKey ? page.regrown : []; if (page.key !== filterKey) { - setPage({ key: filterKey, size: PAGE_SIZE, cursors: null, regrown: [] }); + setPage({ + key: filterKey, + size: PAGE_SIZE, + cursors: null, + regrown: [], + turn: turnRefreshToken, + }); } const listTargets = environmentQueries.flatMap(({ environmentId, projectIds }) => { @@ -591,6 +600,7 @@ export function PullRequestsPage() { size: regrown.length === 0 ? pageSize : Math.min(pageSize + PAGE_SIZE, MAX_PAGE_SIZE), cursors: nextCursors, regrown, + turn: turnRefreshToken, }); return; } @@ -599,9 +609,28 @@ export function PullRequestsPage() { size: Math.min(pageSize + PAGE_SIZE, MAX_PAGE_SIZE), cursors: null, regrown: [], + turn: turnRefreshToken, }); }; + const collapsedPage = () => { + const loadedCount = + ordered?.key === filterKey ? largestPullRequestReadCount(ordered.entries) : pageSize; + return { + key: filterKey, + regrown: [], + size: Math.min( + Math.max(pageSize, Math.ceil(loadedCount / PAGE_SIZE) * PAGE_SIZE), + MAX_PAGE_SIZE, + ), + cursors: null, + turn: turnRefreshToken, + }; + }; + // Every list read refetches itself when a turn completes, but a continuation would refetch only + // its later page and strand the rows before it, so the whole loaded list is asked for again. + if (sentCursors !== null && page.turn !== turnRefreshToken) setPage(collapsedPage()); + const refreshList = (includeRelated = false, actedEnvironmentId?: EnvironmentId) => { const related = ( includeRelated @@ -627,31 +656,9 @@ export function PullRequestsPage() { return; } if (related.length > 0) listQuery.refresh(related); - const loadedCount = - ordered?.key === filterKey ? largestPullRequestReadCount(ordered.entries) : pageSize; - setPage({ - key: filterKey, - regrown: [], - size: Math.min( - Math.max(pageSize, Math.ceil(loadedCount / PAGE_SIZE) * PAGE_SIZE), - MAX_PAGE_SIZE, - ), - cursors: null, - }); + setPage(collapsedPage()); }; - const appliedTurnRefreshToken = useRef(""); - const refreshAfterTurn = useEffectEvent(() => { - if (sentCursors !== null) refreshList(); - }); - useEffect(() => { - if (turnRefreshToken.length === 0 || appliedTurnRefreshToken.current === turnRefreshToken) { - return; - } - appliedTurnRefreshToken.current = turnRefreshToken; - refreshAfterTurn(); - }, [turnRefreshToken]); - useLiveRefresh( () => { refreshList(true); @@ -917,15 +924,6 @@ export function PullRequestsPage() { // oxlint-disable-next-line react/exhaustive-effect-dependencies -- The pointer sensor lives outside React; a new list under a sweep in progress must end it. [filterKey, search.q, sort], ); - useEffect(() => { - const onKeyDown = (event: KeyboardEvent) => { - if (event.key !== "Escape" || closeSensorRef.current === null) return; - event.preventDefault(); - closeSensorRef.current.cancel(); - }; - document.addEventListener("keydown", onKeyDown, true); - return () => document.removeEventListener("keydown", onKeyDown, true); - }, []); const startCloseSweep = useStableCallback( (entry: EnvironmentPullRequestEntry, event: PointerEvent) => { closeSensorRef.current?.cancel(); @@ -957,14 +955,27 @@ export function PullRequestsPage() { sweptKeys = resolveSidebarSweepKeys(orderedKeys, originKey, key, canClose); setCloseSweepKeys(new Set(sweptKeys)); }; - closeSensorRef.current = new SidebarPointerSensor({ + // Only while a sweep runs: Escape ends it, ahead of any other scope that would also take it. + const releaseEscape = registerCommands( + {}, + { + capture: true, + onKeyDown: (keyEvent) => { + if (keyEvent.key !== "Escape") return false; + keyEvent.preventDefault(); + sensor.cancel(); + }, + }, + ); + const sensor = new SidebarPointerSensor({ active: originKey, event, options: { distance: 6, onAttach: () => {}, onFinish: () => { - closeSensorRef.current = null; + releaseEscape(); + if (closeSensorRef.current === sensor) closeSensorRef.current = null; setCloseSweepKeys(new Set()); }, }, @@ -992,6 +1003,7 @@ export function PullRequestsPage() { onCancel: () => {}, onAbort: () => {}, }); + closeSensorRef.current = sensor; }, ); const rightPanelAvailable = selectedPullRequestSurface !== null;