diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx index 884bef0bf0b8..a39e144ca382 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx @@ -32,6 +32,7 @@ vi.mock("~/state/environments", () => ({ usePrimaryEnvironmentId: () => EnvironmentId.make("env-1"), })); vi.mock("~/hooks/useSettings", () => ({ + useEnvironmentSettings: () => undefined, useClientSettings: (select: (settings: typeof DEFAULT_CLIENT_SETTINGS) => unknown) => select(DEFAULT_CLIENT_SETTINGS), })); diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index bf5bd62de91c..53dc7642d9be 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -6,7 +6,6 @@ import { scopedThreadKey, scopeProjectRef } from "@t3tools/client-runtime/enviro import { squashAtomCommandFailure } from "@t3tools/client-runtime/state/runtime"; import { type EnvironmentId, - DEFAULT_SERVER_SETTINGS, type PullRequestAction, type PullRequestMergeMethod, type PullRequestListEntry, @@ -15,7 +14,6 @@ import { resolveEnvironmentMachineKind, type ScopedThreadRef, } from "@t3tools/contracts"; -import { resolveProjectSettings } from "@t3tools/shared/projectSettings"; import { ArrowDownUpIcon, ArrowLeftIcon, @@ -63,20 +61,14 @@ import { type ShortcutMatchContext, } from "~/keybindings"; import { primaryServerKeybindingsAtom } from "~/state/server"; -import { useClientSettings } from "~/hooks/useSettings"; -import { - deriveLogicalProjectKeyFromSettings, - derivePhysicalProjectKey, - selectProjectGroupingSettings, -} from "~/logicalProject"; +import { usePullRequestDefaultMergeMethodResolver } from "./usePullRequestActions"; import { changeRequestRepositoryUrl, gitHubPullRequestBrowserUrl } from "~/lib/openPullRequestLink"; import { usePreparePullRequestThreadAction } from "~/lib/sourceControlActions"; import { cn } from "~/lib/utils"; import { readLocalApi } from "~/localApi"; import type { ReviewCommentContext } from "~/reviewCommentContext"; -import { buildPhysicalToLogicalProjectKeyMap } from "~/sidebarProjectGrouping"; import { useProjects, useServerConfigs } from "~/state/entities"; -import { useEnvironments, usePrimaryEnvironmentId } from "~/state/environments"; +import { useEnvironments } from "~/state/environments"; import { useEnvironmentQuery } from "~/state/query"; import { useLiveRefresh } from "~/hooks/useLiveRefresh"; import { @@ -563,19 +555,14 @@ export function PullRequestDetailPanel({ }, [condensed]); const lastSelectedMergeMethod = useUiStateStore((state) => state.pullRequestMergeMethod); const setLastSelectedMergeMethod = useUiStateStore((state) => state.setPullRequestMergeMethod); - // Server-side and per project, like every other project setting. The - // client-local per-project map from before still answers when the server - // has no value, so a choice made on an older release keeps applying until - // it is set (or reset) in Settings. - const legacyMergeMethodOverrides = useClientSettings( - (settings) => settings.pullRequestMergeMethodOverrides, + const resolveProjectDefaultMergeMethod = usePullRequestDefaultMergeMethodResolver( + environmentId, + reference.projectId, + ); + const projectDefaultMergeMethod = useMemo( + () => resolveProjectDefaultMergeMethod(), + [resolveProjectDefaultMergeMethod], ); - const projectGroupingSettings = useClientSettings(selectProjectGroupingSettings); - const projectDefaultMergeMethod = - resolveProjectSettings( - environmentConfigs.get(environmentId)?.settings ?? DEFAULT_SERVER_SETTINGS, - reference.projectId, - ).settings.pullRequestMergeMethod ?? undefined; const [mergeMethodSelection, setMergeMethodSelection] = useState<{ readonly pullRequestKey: string; readonly method: PullRequestMergeMethod; @@ -887,39 +874,12 @@ export function PullRequestDetailPanel({ const [titleSaving, setTitleSaving] = useState(false); const newThread = useNewThreadHandler(); const { environments } = useEnvironments(); - const primaryEnvironmentId = usePrimaryEnvironmentId(); const unavailableGitHubUrl = useMemo(() => { const identity = projects.find( (project) => project.id === reference.projectId && project.environmentId === environmentId, )?.repositoryIdentity; return gitHubPullRequestBrowserUrl(identity, reference.repository, reference.number); }, [environmentId, projects, reference.number, reference.projectId, reference.repository]); - // Project settings stored the override under the sidebar group's key, which a duplicate row - // borrows from its siblings, so the project alone does not always name the same key. - const legacyProjectDefaultMergeMethod = useMemo(() => { - if (projectDefaultMergeMethod !== undefined) return undefined; - const project = projects.find( - (candidate) => - candidate.environmentId === environmentId && candidate.id === reference.projectId, - ); - if (!project) return undefined; - const projectKey = - buildPhysicalToLogicalProjectKeyMap({ - projects, - settings: projectGroupingSettings, - primaryEnvironmentId, - }).get(derivePhysicalProjectKey(project)) ?? - deriveLogicalProjectKeyFromSettings(project, projectGroupingSettings); - return legacyMergeMethodOverrides[projectKey]; - }, [ - environmentId, - legacyMergeMethodOverrides, - primaryEnvironmentId, - projectDefaultMergeMethod, - projectGroupingSettings, - projects, - reference.projectId, - ]); // Beside a thread there is nothing to pick: the hand-offs land in that thread's composer, and // the thread is already on one server's copy of the branch. const pickableEnvironments = useMemo( @@ -1421,7 +1381,7 @@ export function PullRequestDetailPanel({ const selectedMergeMethod = resolvePullRequestMergeMethod( allowedMergeMethods, currentMergeMethod, - projectDefaultMergeMethod ?? legacyProjectDefaultMergeMethod, + projectDefaultMergeMethod, lastSelectedMergeMethod, ); const selectedMergeMethodLabel = PULL_REQUEST_MERGE_METHOD_LABELS[selectedMergeMethod]; diff --git a/apps/web/src/components/pullRequest/PullRequestRow.tsx b/apps/web/src/components/pullRequest/PullRequestRow.tsx index 04008281f5e7..1f4de90f55b8 100644 --- a/apps/web/src/components/pullRequest/PullRequestRow.tsx +++ b/apps/web/src/components/pullRequest/PullRequestRow.tsx @@ -1,5 +1,9 @@ import { SearchIcon } from "lucide-react"; import { PullRequestStackPopover } from "./PullRequestStackPopover"; +import { + PullRequestSpeedActions, + type PullRequestSpeedActionResult, +} from "./PullRequestSpeedActions"; import { memo, type RefCallback } from "react"; import { cn } from "~/lib/utils"; @@ -66,10 +70,9 @@ function PullRequestRowLabels({ labels }: { labels: EnvironmentPullRequestEntry[ /** * The page row keeps a little more room around the shared lines than the panel, which sits in - * a narrow column. The intrinsic size is the content box a skipped row reserves, which is the - * two lines without the padding: a 56px row less 20px of `py-2.5`. + * a narrow column. Its outer wrapper reserves the full row height when offscreen. */ -const PAGE_ROW_CLASS = "px-3 py-2.5 [contain-intrinsic-block-size:36.5px]"; +const PAGE_ROW_CLASS = "px-3 py-2.5"; export type PullRequestRowTarget = Pick< EnvironmentPullRequestEntry, @@ -86,6 +89,8 @@ function PullRequestRowImpl({ statsKey, statsRef, onSelect, + speedMode, + onActed, }: { entry: EnvironmentPullRequestEntry; selected: boolean; @@ -101,144 +106,152 @@ function PullRequestRowImpl({ matchedElsewhere?: boolean; /** Used by the list's shared visibility observer to defer optional line-count reads. */ statsKey?: string; - statsRef?: RefCallback; + statsRef?: RefCallback; onSelect: (entry: PullRequestRowTarget) => void; + speedMode: boolean; + onActed: (result: PullRequestSpeedActionResult) => void; }) { const { Icon, providerName } = getSourceControlPresentationForKind(entry.provider); return ( - + /> + ) : null} + + + } + metaClassName="@container/pr-row-meta" + meta={ + <> + {matchedElsewhere ? ( + + + } + > + matched in the description + + + matched in the description + + + Matched in the description + + ) : null} + {showProvider ? ( + + }> + + + {providerName} + + ) : null} + + {showProjectTitle ? {entry.repository} : null} + {environmentLabel ? ( + {environmentLabel} + ) : null} + {entry.labels.length > 0 ? : null} + + } + updatedAt={entry.updatedAt} + /> + + {entry.state !== "merged" && entry.provider === "github" ? ( + + ) : null} + ); } diff --git a/apps/web/src/components/pullRequest/PullRequestSpeedActions.tsx b/apps/web/src/components/pullRequest/PullRequestSpeedActions.tsx new file mode 100644 index 000000000000..613928733d37 --- /dev/null +++ b/apps/web/src/components/pullRequest/PullRequestSpeedActions.tsx @@ -0,0 +1,135 @@ +import type { PullRequestAction } from "@t3tools/contracts"; +import { Effect } from "effect"; +import { AtomRegistry } from "effect/unstable/reactivity"; +import { appAtomRegistry } from "~/rpc/atomRegistry"; +import { pullRequestEnvironment, pullRequestStackAtom } from "~/state/pullRequests"; +import { useUiStateStore } from "~/uiStateStore"; +import { Button } from "../ui/button"; +import { Spinner } from "../ui/spinner"; +import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; +import { resolvePullRequestMergeMethod } from "./pullRequestDetail.logic"; +import { PullRequestGlyph } from "./pullRequestIcons"; +import type { EnvironmentPullRequestEntry } from "./pullRequestList.logic"; +import { + usePullRequestActionRunner, + usePullRequestDefaultMergeMethodResolver, +} from "./usePullRequestActions"; + +export interface PullRequestSpeedActionResult { + readonly entry: EnvironmentPullRequestEntry; + readonly action: PullRequestAction; +} + +/** No detail or stack reads until a merge is clicked, even on a long list. */ +export function PullRequestSpeedActions({ + entry, + visible, + onActed, +}: { + entry: EnvironmentPullRequestEntry; + visible: boolean; + onActed: (result: PullRequestSpeedActionResult) => void; +}) { + const resolveProjectDefault = usePullRequestDefaultMergeMethodResolver( + entry.environmentId, + entry.projectId, + ); + const reference = { + projectId: entry.projectId, + host: entry.host, + repository: entry.repository, + number: entry.number, + }; + const { actionPending, perform } = usePullRequestActionRunner({ + environmentId: entry.environmentId, + reference, + onSuccess: (action) => onActed({ entry, action }), + resolveMergeMethod: async () => { + const target = { environmentId: entry.environmentId, input: reference }; + const detailAtom = pullRequestEnvironment.detail({ + ...target, + input: { ...reference, allowStale: false }, + }); + appAtomRegistry.refresh(detailAtom); + const detail = await Effect.runPromise( + AtomRegistry.getResult(appAtomRegistry, detailAtom, { suspendOnWaiting: true }), + ); + if ( + detail.state !== "open" || + detail.isDraft || + !detail.capabilities.actions.includes("merge") || + !detail.viewerPermissions.actions.includes("merge") + ) { + throw new Error("This pull request cannot be merged."); + } + if (detail.capabilities.stackActions) { + const stackAtom = pullRequestStackAtom(target); + appAtomRegistry.refresh(stackAtom); + const stack = await Effect.runPromise( + AtomRegistry.getResult(appAtomRegistry, stackAtom, { suspendOnWaiting: true }), + ); + if (stack !== null) throw new Error("Open this pull request to merge its stack."); + } + const allowed = detail.capabilities.mergeMethods.filter( + (method) => detail.mergeCapabilities[method], + ); + if (allowed.length === 0) + throw new Error("No merge method is available for this repository."); + return resolvePullRequestMergeMethod( + allowed, + null, + resolveProjectDefault(), + useUiStateStore.getState().pullRequestMergeMethod, + ); + }, + }); + const actions = + entry.state === "closed" + ? (["reopen"] as const) + : entry.isDraft + ? (["close", "ready"] as const) + : (["close", "merge"] as const); + return ( +
+ {actions.map((action) => { + const label = ACTIONS[action].label; + const Icon = ACTIONS[action].Icon; + return ( + + void perform(action)} + /> + } + > + {actionPending ? : } + {label} + + + {action === "merge" && entry.stack + ? "Open this pull request to merge its stack" + : `${label} immediately`} + + + ); + })} +
+ ); +} + +const ACTIONS = { + close: { label: "Close", Icon: PullRequestGlyph.closed }, + merge: { label: "Merge", Icon: PullRequestGlyph.merged }, + ready: { label: "Ready for review", Icon: PullRequestGlyph.pullRequest }, + reopen: { label: "Reopen", Icon: PullRequestGlyph.reopen }, +} as const; diff --git a/apps/web/src/components/pullRequest/pullRequestChecks.test.tsx b/apps/web/src/components/pullRequest/pullRequestChecks.test.tsx index 48a371a7a9c0..b0e8c69517bc 100644 --- a/apps/web/src/components/pullRequest/pullRequestChecks.test.tsx +++ b/apps/web/src/components/pullRequest/pullRequestChecks.test.tsx @@ -102,6 +102,8 @@ function row(overrides: Partial): ReactNode { showProjectTitle: false, showProvider: false, onSelect: () => {}, + speedMode: false, + onActed: () => {}, }); } diff --git a/apps/web/src/components/pullRequest/usePullRequestActions.ts b/apps/web/src/components/pullRequest/usePullRequestActions.ts index 3ba3b60bd90e..01c2789e8209 100644 --- a/apps/web/src/components/pullRequest/usePullRequestActions.ts +++ b/apps/web/src/components/pullRequest/usePullRequestActions.ts @@ -8,12 +8,23 @@ import { scopeProjectRef } from "@t3tools/client-runtime/environment"; import { squashAtomCommandFailure } from "@t3tools/client-runtime/state/runtime"; import type { EnvironmentId, + ProjectId, PullRequestAction, PullRequestDetail, PullRequestMergeMethod, PullRequestRef, } from "@t3tools/contracts"; -import { useState } from "react"; +import { useCallback, useRef, useState } from "react"; +import { resolveProjectSettings } from "@t3tools/shared/projectSettings"; +import { useClientSettings, useEnvironmentSettings } from "~/hooks/useSettings"; +import { + deriveLogicalProjectKeyFromSettings, + derivePhysicalProjectKey, + selectProjectGroupingSettings, +} from "~/logicalProject"; +import { buildPhysicalToLogicalProjectKeyMap } from "~/sidebarProjectGrouping"; +import { useProjects } from "~/state/entities"; +import { usePrimaryEnvironmentId } from "~/state/environments"; import { type DraftId, useComposerDraftStore } from "~/composerDraftStore"; import { useNewThreadHandler } from "~/hooks/useHandleNewThread"; @@ -25,8 +36,48 @@ import { useAtomCommand } from "~/state/use-atom-command"; import { toastManager } from "../ui/toast"; import { handoffPrompt, handoffReviewComments, readableFailure } from "./pullRequestDetail.logic"; +/** Resolve on demand so hidden quick actions do not rebuild the legacy project grouping. */ +export function usePullRequestDefaultMergeMethodResolver( + environmentId: EnvironmentId, + projectId: ProjectId, +) { + const projectDefault = useEnvironmentSettings( + environmentId, + (settings) => resolveProjectSettings(settings, projectId).settings.pullRequestMergeMethod, + ); + const legacyOverrides = useClientSettings((settings) => settings.pullRequestMergeMethodOverrides); + const grouping = useClientSettings(selectProjectGroupingSettings); + const projects = useProjects(); + const primaryEnvironmentId = usePrimaryEnvironmentId(); + return useCallback(() => { + if (projectDefault != null) return projectDefault; + if (Object.keys(legacyOverrides).length === 0) return undefined; + const project = projects.find( + (candidate) => candidate.environmentId === environmentId && candidate.id === projectId, + ); + if (!project) return undefined; + // Duplicate sidebar rows borrow their logical group key from their siblings. + const key = + buildPhysicalToLogicalProjectKeyMap({ + projects, + settings: grouping, + primaryEnvironmentId, + }).get(derivePhysicalProjectKey(project)) ?? + deriveLogicalProjectKeyFromSettings(project, grouping); + return legacyOverrides[key]; + }, [ + projectDefault, + projects, + environmentId, + projectId, + grouping, + primaryEnvironmentId, + legacyOverrides, + ]); +} + const ACTION_SUCCESS_LABELS: Record = { - merge: "Pull request merged", + merge: "Merge requested", ready: "Marked ready for review", draft: "Converted to draft", close: "Pull request closed", @@ -80,37 +131,41 @@ export function usePullRequestActionRunner({ environmentId, reference, onSuccess, + resolveMergeMethod, }: { environmentId: EnvironmentId; reference: PullRequestRef | null; onSuccess?: (action: PullRequestAction) => void; + /** Small surfaces resolve repository settings on the click, not for every visible row. */ + resolveMergeMethod?: () => Promise; }) { const runAction = useAtomCommand(pullRequestEnvironment.runAction, { reportFailure: false }); const [actionPending, setActionPending] = useState(false); + const pendingRef = useRef(false); const perform = async (action: PullRequestAction, method?: PullRequestMergeMethod) => { - if (actionPending || reference === null) return; + if (pendingRef.current || reference === null) return; + pendingRef.current = true; setActionPending(true); - const result = await runAction({ - environmentId, - input: { ...reference, action, ...(method ? { mergeMethod: method } : {}) }, - }); - setActionPending(false); - if (result._tag === "Failure") { - // The host's own sentence, because it is the only thing that says why. A merge strategy a - // branch policy forbids is refused at completion and nowhere earlier — Azure DevOps - // publishes no per-strategy availability to hide the control with — so "action failed" - // would leave the reader pressing the same button again. - const failure = squashAtomCommandFailure(result); + try { + const mergeMethod = method ?? (action === "merge" ? await resolveMergeMethod?.() : undefined); + const result = await runAction({ + environmentId, + input: { ...reference, action, ...(mergeMethod ? { mergeMethod } : {}) }, + }); + if (result._tag === "Failure") throw squashAtomCommandFailure(result); + toastManager.add({ type: "success", title: ACTION_SUCCESS_LABELS[action] }); + onSuccess?.(action); + } catch (failure) { toastManager.add({ type: "error", title: ACTION_FAILURE_LABELS[action], description: readableFailure(failure, ACTION_FAILURE_HINTS[action]), }); - return; + } finally { + pendingRef.current = false; + setActionPending(false); } - toastManager.add({ type: "success", title: ACTION_SUCCESS_LABELS[action] }); - onSuccess?.(action); }; return { actionPending, perform }; diff --git a/apps/web/src/routes/_chat.pull-requests.tsx b/apps/web/src/routes/_chat.pull-requests.tsx index 8f75fcb66cac..b67f79234813 100644 --- a/apps/web/src/routes/_chat.pull-requests.tsx +++ b/apps/web/src/routes/_chat.pull-requests.tsx @@ -1,5 +1,7 @@ import { RefreshIcon } from "~/components/ui/refresh-icon"; import { Spinner } from "~/components/ui/spinner"; +import { useShortcutModifierState } from "~/shortcutModifierState"; +import type { PullRequestSpeedActionResult } from "~/components/pullRequest/PullRequestSpeedActions"; import { pullRequestHostOf, resolveEnvironmentMachineKind } from "@t3tools/contracts"; import type { EnvironmentId, @@ -342,6 +344,9 @@ export const Route = createFileRoute("/_chat/pull-requests")({ function PullRequestsRouteView() { useEscapeToGoBack(); + const modifiers = useShortcutModifierState(true); + const speedMode = + modifiers.shiftKey && !modifiers.metaKey && !modifiers.ctrlKey && !modifiers.altKey; const search = Route.useSearch(); const sort = search.sort ?? "ready"; const statsPolicy: PullRequestStatsPolicy = @@ -952,6 +957,16 @@ function PullRequestsRouteView() { }; /** The detail panel's own writes, by row, so its failure takes back its own note. */ const detailOverrideTokens = useRef(new Map()); + const speedActionRef = useRef<(result: PullRequestSpeedActionResult) => void>(() => {}); + speedActionRef.current = ({ entry, action }) => { + // Some hosts accept a merge before it completes. Let the next host read declare it merged. + if (action !== "merge") overrideEntry(entry, action); + setDetailRefreshToken((token) => token + 1); + refreshListAndStats(undefined, entry.environmentId); + }; + const onSpeedAction = useCallback((result: PullRequestSpeedActionResult) => { + speedActionRef.current(result); + }, []); // A reload recreates the registry the queries live in, so with nothing held the page would // cold-start into skeletons even though almost every row is unchanged. The last answer for // this set of environments is kept across reloads and hydrated here as the carried rows: they @@ -1395,11 +1410,11 @@ function PullRequestsRouteView() { [statsBatches], ); const statsObserver = useRef(null); - const statsRows = useRef(new Set()); + const statsRows = useRef(new Set()); const statsPending = useRef(true); const statsPolicyRef = useRef(statsPolicy); statsPolicyRef.current = statsPolicy; - const registerStatsRow = useCallback((node: HTMLButtonElement | null) => { + const registerStatsRow = useCallback((node: HTMLDivElement | null) => { if (node === null || typeof IntersectionObserver === "undefined") return; statsRows.current.add(node); statsObserver.current?.observe(node); @@ -1817,6 +1832,8 @@ function PullRequestsRouteView() { selected.number === entry.number } onSelect={selectEntry} + speedMode={speedMode} + onActed={onSpeedAction} /> ); })} diff --git a/apps/web/src/shortcutModifierState.ts b/apps/web/src/shortcutModifierState.ts index 15d6e0a1bcae..20091622158e 100644 --- a/apps/web/src/shortcutModifierState.ts +++ b/apps/web/src/shortcutModifierState.ts @@ -1,4 +1,5 @@ import { useEffect, useRef, useState } from "react"; +import { isEditableFocused } from "./lib/editableFocus"; export interface ShortcutModifierState { metaKey: boolean; @@ -26,7 +27,7 @@ export function areShortcutModifierStatesEqual( ); } -export function useShortcutModifierState(): ShortcutModifierState { +export function useShortcutModifierState(ignoreEditable = false): ShortcutModifierState { const [state, setState] = useState(EMPTY_SHORTCUT_MODIFIER_STATE); const stateRef = useRef(EMPTY_SHORTCUT_MODIFIER_STATE); @@ -39,7 +40,11 @@ export function useShortcutModifierState(): ShortcutModifierState { setState(next); }; const onKeyboardEvent = (event: KeyboardEvent) => { - updateState(shortcutModifierStateAfterKeyboardEvent(stateRef.current, event)); + updateState( + ignoreEditable && isEditableFocused(event.target) + ? EMPTY_SHORTCUT_MODIFIER_STATE + : shortcutModifierStateAfterKeyboardEvent(stateRef.current, event), + ); }; // Dictation tools (Wispr Flow) paste with a synthetic ⌘V whose Meta keyup // never reaches the page, so the tracked state stays "⌘ held" forever and @@ -50,17 +55,22 @@ export function useShortcutModifierState(): ShortcutModifierState { updateState(EMPTY_SHORTCUT_MODIFIER_STATE); }; + const onFocus = (event: FocusEvent) => { + if (ignoreEditable && isEditableFocused(event.target)) onResetEvent(); + }; + window.addEventListener("focusin", onFocus); window.addEventListener("keydown", onKeyboardEvent, true); window.addEventListener("keyup", onKeyboardEvent, true); window.addEventListener("paste", onResetEvent, true); window.addEventListener("blur", onResetEvent); return () => { + window.removeEventListener("focusin", onFocus); window.removeEventListener("keydown", onKeyboardEvent, true); window.removeEventListener("keyup", onKeyboardEvent, true); window.removeEventListener("paste", onResetEvent, true); window.removeEventListener("blur", onResetEvent); }; - }, []); + }, [ignoreEditable]); return state; }