From e7e51da7256fb074787215fdc8d7c82cfe01e013 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 29 May 2026 07:42:34 -0700 Subject: [PATCH] Add Source Control split-open modifier clicks --- .../right-sidebar/SourceControl.tsx | 72 ++++++++++++++----- .../source-control-split-open.test.ts | 35 +++++++++ .../source-control-split-open.ts | 14 ++++ .../useSourceControlSelection.ts | 17 ++++- src/renderer/src/store/slices/editor.test.ts | 21 ++++++ src/renderer/src/store/slices/editor.ts | 58 +++++++++++---- 6 files changed, 186 insertions(+), 31 deletions(-) create mode 100644 src/renderer/src/components/right-sidebar/source-control-split-open.test.ts create mode 100644 src/renderer/src/components/right-sidebar/source-control-split-open.ts diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index aac5d94ca2a..baf456dc02c 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -178,6 +178,10 @@ import { getCommitMessageModelDiscoveryHostKeyForScope } from '../../../../share import { getRuntimeGitScope } from '@/runtime/runtime-git-client' import { getRepositorySourceControlAiSectionId } from '@/components/settings/repository-settings-targets' import { hasExpandedCommitFailureDetails, summarizeCommitFailure } from './commit-failure-summary' +import { + isSourceControlSplitOpenModifier, + type SourceControlRowOpenEvent +} from './source-control-split-open' export type SourceControlScope = 'all' | 'uncommitted' type AbortConflictOperation = Extract @@ -895,6 +899,7 @@ export function HostedReviewHeaderLink({ function SourceControlInner(): React.JSX.Element { const sourceControlRef = useRef(null) + const isMac = useMemo(() => navigator.userAgent.includes('Mac'), []) // Why: React setState is async, so a rapid double-click on the Commit // button can both pass the isCommitting state guard before the disabled // state re-renders. A ref flipped synchronously at the start of @@ -967,6 +972,9 @@ function SourceControlInner(): React.JSX.Element { const openConflictFile = useAppStore((s) => s.openConflictFile) const openConflictReview = useAppStore((s) => s.openConflictReview) const openBranchDiff = useAppStore((s) => s.openBranchDiff) + const createEmptySplitGroup = useAppStore((s) => s.createEmptySplitGroup) + const groupsByWorktree = useAppStore((s) => s.groupsByWorktree) + const activeGroupIdByWorktree = useAppStore((s) => s.activeGroupIdByWorktree) const openAllDiffs = useAppStore((s) => s.openAllDiffs) const openBranchAllDiffs = useAppStore((s) => s.openBranchAllDiffs) const openCommitAllDiffs = useAppStore((s) => s.openCommitAllDiffs) @@ -2824,16 +2832,36 @@ function SourceControlInner(): React.JSX.Element { ] ) + // Why: modifier-click should keep the current pane intact by opening the + // selected Source Control file in a fresh split to the right. + const resolveSplitTargetGroupId = useCallback( + (event?: SourceControlRowOpenEvent): string | undefined => { + if (!event || !activeWorktreeId || !isSourceControlSplitOpenModifier(event, isMac)) { + return undefined + } + const sourceGroupId = + activeGroupIdByWorktree[activeWorktreeId] ?? groupsByWorktree[activeWorktreeId]?.[0]?.id + if (!sourceGroupId) { + return undefined + } + return createEmptySplitGroup(activeWorktreeId, sourceGroupId, 'right') ?? undefined + }, + [activeGroupIdByWorktree, activeWorktreeId, createEmptySplitGroup, groupsByWorktree, isMac] + ) + const handleOpenDiff = useCallback( - (entry: GitStatusEntry) => { + (entry: GitStatusEntry, event?: SourceControlRowOpenEvent) => { if (!activeWorktreeId || !worktreePath) { return } + const targetGroupId = resolveSplitTargetGroupId(event) if (entry.conflictKind && entry.conflictStatus) { if (entry.conflictStatus === 'unresolved') { trackConflictPath(activeWorktreeId, entry.path, entry.conflictKind) } - openConflictFile(activeWorktreeId, worktreePath, entry, detectLanguage(entry.path)) + openConflictFile(activeWorktreeId, worktreePath, entry, detectLanguage(entry.path), { + targetGroupId + }) return } const language = detectLanguage(entry.path) @@ -2847,21 +2875,27 @@ function SourceControlInner(): React.JSX.Element { // files keep the existing diff-tab flow until the diff-tab type is // eventually collapsed (see reviews/changes-view-mode-plan.md ยง"Follow-up"). if (language === 'markdown' && entry.area === 'unstaged') { - openFile({ - filePath, - relativePath: entry.path, - worktreeId: activeWorktreeId, - language, - mode: 'edit' - }) + openFile( + { + filePath, + relativePath: entry.path, + worktreeId: activeWorktreeId, + language, + mode: 'edit' + }, + { targetGroupId } + ) setEditorViewMode(filePath, 'changes') return } - openDiff(activeWorktreeId, filePath, entry.path, language, entry.area === 'staged') + openDiff(activeWorktreeId, filePath, entry.path, language, entry.area === 'staged', { + targetGroupId + }) }, [ activeWorktreeId, worktreePath, + resolveSplitTargetGroupId, trackConflictPath, openConflictFile, openDiff, @@ -2874,6 +2908,7 @@ function SourceControlInner(): React.JSX.Element { useSourceControlSelection({ flatEntries: visibleSelectionEntries, onOpenDiff: handleOpenDiff, + shouldOpenAsSplit: (event) => isSourceControlSplitOpenModifier(event, isMac), containerRef: sourceControlRef }) @@ -3439,7 +3474,7 @@ function SourceControlInner(): React.JSX.Element { }, []) const openCommittedDiff = useCallback( - (entry: GitBranchChangeEntry) => { + (entry: GitBranchChangeEntry, event?: SourceControlRowOpenEvent) => { if ( !activeWorktreeId || !worktreePath || @@ -3453,10 +3488,11 @@ function SourceControlInner(): React.JSX.Element { worktreePath, entry, branchSummary, - detectLanguage(entry.path) + detectLanguage(entry.path), + { targetGroupId: resolveSplitTargetGroupId(event) } ) }, - [activeWorktreeId, branchSummary, openBranchDiff, worktreePath] + [activeWorktreeId, branchSummary, openBranchDiff, resolveSplitTargetGroupId, worktreePath] ) const openHistoryCommitDiff = useCallback( @@ -4488,7 +4524,7 @@ function SourceControlInner(): React.JSX.Element { worktreePath={worktreePath} depth={node.depth} onRevealInExplorer={revealInExplorer} - onOpen={() => openCommittedDiff(node.entry)} + onOpen={(event) => openCommittedDiff(node.entry, event)} commentCount={diffCommentCountByPath.get(node.entry.path) ?? 0} showPathHint={false} /> @@ -4501,7 +4537,7 @@ function SourceControlInner(): React.JSX.Element { currentWorktreeId={currentWorktreeId} worktreePath={worktreePath} onRevealInExplorer={revealInExplorer} - onOpen={() => openCommittedDiff(entry)} + onOpen={(event) => openCommittedDiff(entry, event)} commentCount={diffCommentCountByPath.get(entry.path) ?? 0} /> )))} @@ -6230,7 +6266,7 @@ const UncommittedEntryRow = React.memo(function UncommittedEntryRow({ onSelect?: (e: React.MouseEvent, key: string, entry: GitStatusEntry) => void onContextMenu?: (key: string) => void onRevealInExplorer: (worktreeId: string, absolutePath: string) => void - onOpen: (entry: GitStatusEntry) => void + onOpen: (entry: GitStatusEntry, event?: SourceControlRowOpenEvent) => void onStage: (filePath: string) => Promise onUnstage: (filePath: string) => Promise onDiscard: (entry: GitStatusEntry) => void @@ -6300,7 +6336,7 @@ const UncommittedEntryRow = React.memo(function UncommittedEntryRow({ if (onSelect) { onSelect(e, entryKey, entry) } else { - onOpen(entry) + onOpen(entry, e) } }} > @@ -6435,7 +6471,7 @@ function BranchEntryRow({ worktreePath: string depth?: number onRevealInExplorer: (worktreeId: string, absolutePath: string) => void - onOpen: () => void + onOpen: (event: React.MouseEvent) => void commentCount: number showPathHint?: boolean }): React.JSX.Element { diff --git a/src/renderer/src/components/right-sidebar/source-control-split-open.test.ts b/src/renderer/src/components/right-sidebar/source-control-split-open.test.ts new file mode 100644 index 00000000000..7d6a7f133a0 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control-split-open.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from 'vitest' +import { + isSourceControlSplitOpenModifier, + type SourceControlRowOpenEvent +} from './source-control-split-open' + +function event(overrides: Partial = {}): SourceControlRowOpenEvent { + return { + altKey: false, + ctrlKey: false, + metaKey: false, + shiftKey: false, + ...overrides + } +} + +describe('isSourceControlSplitOpenModifier', () => { + it('uses Cmd on macOS and Ctrl elsewhere as the platform primary modifier', () => { + expect(isSourceControlSplitOpenModifier(event({ metaKey: true }), true)).toBe(true) + expect(isSourceControlSplitOpenModifier(event({ ctrlKey: true }), true)).toBe(false) + + expect(isSourceControlSplitOpenModifier(event({ ctrlKey: true }), false)).toBe(true) + expect(isSourceControlSplitOpenModifier(event({ metaKey: true }), false)).toBe(false) + }) + + it('treats Shift and Alt/Option as split-open modifiers', () => { + expect(isSourceControlSplitOpenModifier(event({ shiftKey: true }), true)).toBe(true) + expect(isSourceControlSplitOpenModifier(event({ altKey: true }), false)).toBe(true) + }) + + it('ignores a plain click', () => { + expect(isSourceControlSplitOpenModifier(event(), true)).toBe(false) + expect(isSourceControlSplitOpenModifier(event(), false)).toBe(false) + }) +}) diff --git a/src/renderer/src/components/right-sidebar/source-control-split-open.ts b/src/renderer/src/components/right-sidebar/source-control-split-open.ts new file mode 100644 index 00000000000..324a5b4d8a4 --- /dev/null +++ b/src/renderer/src/components/right-sidebar/source-control-split-open.ts @@ -0,0 +1,14 @@ +export type SourceControlRowOpenEvent = { + altKey: boolean + ctrlKey: boolean + metaKey: boolean + shiftKey: boolean +} + +export function isSourceControlSplitOpenModifier( + event: SourceControlRowOpenEvent, + isMac: boolean +): boolean { + const platformPrimary = isMac ? event.metaKey && !event.ctrlKey : event.ctrlKey && !event.metaKey + return platformPrimary || event.shiftKey || event.altKey +} diff --git a/src/renderer/src/components/right-sidebar/useSourceControlSelection.ts b/src/renderer/src/components/right-sidebar/useSourceControlSelection.ts index 1113b8b1cbf..67c28a6d7b7 100644 --- a/src/renderer/src/components/right-sidebar/useSourceControlSelection.ts +++ b/src/renderer/src/components/right-sidebar/useSourceControlSelection.ts @@ -1,5 +1,6 @@ import { useState, useCallback, useEffect, useRef, type RefObject } from 'react' import type { GitStatusEntry } from '../../../../shared/types' +import type { SourceControlRowOpenEvent } from './source-control-split-open' function isMacPlatform(): boolean { return typeof navigator !== 'undefined' && navigator.userAgent.includes('Mac') @@ -50,10 +51,12 @@ export function getSelectionRangeKeys( export function useSourceControlSelection({ flatEntries, onOpenDiff, + shouldOpenAsSplit, containerRef }: { flatEntries: FlatEntry[] - onOpenDiff: (entry: GitStatusEntry) => void + onOpenDiff: (entry: GitStatusEntry, event?: SourceControlRowOpenEvent) => void + shouldOpenAsSplit?: (event: SourceControlRowOpenEvent) => boolean containerRef: RefObject }) { const [selectedKeys, setSelectedKeys] = useState>(new Set()) @@ -62,6 +65,7 @@ export function useSourceControlSelection({ const anchorKeyRef = useRef(anchorKey) const selectedKeysRef = useRef(selectedKeys) const onOpenDiffRef = useRef(onOpenDiff) + const shouldOpenAsSplitRef = useRef(shouldOpenAsSplit) useEffect(() => { flatEntriesRef.current = flatEntries @@ -79,6 +83,10 @@ export function useSourceControlSelection({ onOpenDiffRef.current = onOpenDiff }, [onOpenDiff]) + useEffect(() => { + shouldOpenAsSplitRef.current = shouldOpenAsSplit + }, [shouldOpenAsSplit]) + // Clear stale selections if entries disappear useEffect(() => { const validKeys = new Set(flatEntries.map((e) => e.key)) @@ -95,6 +103,13 @@ export function useSourceControlSelection({ }, [flatEntries, selectedKeys, anchorKey]) const handleSelect = useCallback((e: React.MouseEvent, key: string, entry: GitStatusEntry) => { + if (shouldOpenAsSplitRef.current?.(e)) { + setSelectedKeys((prev) => (prev.size > 0 ? new Set() : prev)) + setAnchorKey(null) + onOpenDiffRef.current(entry, e) + return + } + const isShift = e.shiftKey const isCmdOrCtrl = isMacPlatform() ? e.metaKey : e.ctrlKey diff --git a/src/renderer/src/store/slices/editor.test.ts b/src/renderer/src/store/slices/editor.test.ts index ba1008bf881..fdd2880f490 100644 --- a/src/renderer/src/store/slices/editor.test.ts +++ b/src/renderer/src/store/slices/editor.test.ts @@ -321,6 +321,27 @@ describe('createEditorSlice openDiff', () => { ]) expect(store.getState().activeFileId).toBe('wt-1::diff::staged::file.ts') }) + + it('opens the visible diff tab in the requested split group', () => { + const store = createEditorTabsStore() + const sourceTab = store.getState().createUnifiedTab('wt-1', 'terminal', { id: 'terminal-1' }) + const targetGroupId = store.getState().createEmptySplitGroup('wt-1', sourceTab.groupId, 'right') + if (!targetGroupId) { + throw new Error('expected split group') + } + + store + .getState() + .openDiff('wt-1', '/repo/file.ts', 'file.ts', 'typescript', false, { targetGroupId }) + + const diffTab = store + .getState() + .unifiedTabsByWorktree['wt-1']?.find((tab) => tab.contentType === 'diff') + + expect(diffTab?.groupId).toBe(targetGroupId) + expect(diffTab?.entityId).toBe('wt-1::diff::unstaged::file.ts') + expect(store.getState().activeGroupIdByWorktree['wt-1']).toBe(targetGroupId) + }) }) describe('createEditorSlice floating editor activation', () => { diff --git a/src/renderer/src/store/slices/editor.ts b/src/renderer/src/store/slices/editor.ts index d162e124b0e..d6cd6d5aeea 100644 --- a/src/renderer/src/store/slices/editor.ts +++ b/src/renderer/src/store/slices/editor.ts @@ -197,6 +197,10 @@ export type ClosedEditorTabSnapshot = Omit const MAX_RECENT_CLOSED_EDITOR_TABS = 10 +type EditorOpenTargetOptions = { + targetGroupId?: string +} + export type PendingEditorReveal = { filePath: string fileId?: string @@ -327,21 +331,24 @@ export type EditorSlice = { filePath: string, relativePath: string, language: string, - staged: boolean + staged: boolean, + options?: EditorOpenTargetOptions ) => void openBranchDiff: ( worktreeId: string, worktreePath: string, entry: GitBranchChangeEntry, compare: BranchCompareLike, - language: string + language: string, + options?: EditorOpenTargetOptions ) => void openCommitDiff: ( worktreeId: string, worktreePath: string, entry: GitBranchChangeEntry, compare: CommitCompareLike, - language: string + language: string, + options?: EditorOpenTargetOptions ) => void openAllDiffs: ( worktreeId: string, @@ -353,7 +360,8 @@ export type EditorSlice = { worktreeId: string, worktreePath: string, entry: GitStatusEntry, - language: string + language: string, + options?: EditorOpenTargetOptions ) => void openConflictReviewFile: ( reviewFileId: string, @@ -1938,7 +1946,7 @@ export const createEditorSlice: StateCreator = (s openFiles: s.openFiles.map((f) => (f.id === fileId ? { ...f, isUntitled: undefined } : f)) })), - openDiff: (worktreeId, filePath, relativePath, language, staged) => { + openDiff: (worktreeId, filePath, relativePath, language, staged, options) => { set((s) => { const diffSource: DiffSource = staged ? 'staged' : 'unstaged' const id = `${worktreeId}::diff::${diffSource}::${relativePath}` @@ -1992,11 +2000,13 @@ export const createEditorSlice: StateCreator = (s `${worktreeId}::diff::${staged ? 'staged' : 'unstaged'}::${relativePath}`, worktreeId, relativePath, - 'diff' + 'diff', + undefined, + options?.targetGroupId ) }, - openBranchDiff: (worktreeId, worktreePath, entry, compare, language) => { + openBranchDiff: (worktreeId, worktreePath, entry, compare, language, options) => { const branchCompare = toBranchCompareSnapshot(compare) const id = `${worktreeId}::diff::branch::${compare.baseRef}::${branchCompare.compareVersion}::${entry.path}` set((s) => { @@ -2046,10 +2056,18 @@ export const createEditorSlice: StateCreator = (s activeTabTypeByWorktree: { ...s.activeTabTypeByWorktree, [worktreeId]: 'editor' } } }) - void openWorkspaceEditorItem(get(), id, worktreeId, entry.path, 'diff') + void openWorkspaceEditorItem( + get(), + id, + worktreeId, + entry.path, + 'diff', + undefined, + options?.targetGroupId + ) }, - openCommitDiff: (worktreeId, worktreePath, entry, compare, language) => { + openCommitDiff: (worktreeId, worktreePath, entry, compare, language, options) => { const commitCompare = toCommitCompareSnapshot(compare) const id = `${worktreeId}::diff::commit::${commitCompare.compareVersion}::${entry.path}` set((s) => { @@ -2099,7 +2117,15 @@ export const createEditorSlice: StateCreator = (s activeTabTypeByWorktree: { ...s.activeTabTypeByWorktree, [worktreeId]: 'editor' } } }) - void openWorkspaceEditorItem(get(), id, worktreeId, entry.path, 'diff') + void openWorkspaceEditorItem( + get(), + id, + worktreeId, + entry.path, + 'diff', + undefined, + options?.targetGroupId + ) }, openAllDiffs: (worktreeId, worktreePath, alternate, areaFilter) => { @@ -2182,7 +2208,7 @@ export const createEditorSlice: StateCreator = (s void openWorkspaceEditorItem(get(), id, worktreeId, label, 'diff') }, - openConflictFile: (worktreeId, worktreePath, entry, language) => { + openConflictFile: (worktreeId, worktreePath, entry, language, options) => { const absolutePath = joinPath(worktreePath, entry.path) set((s) => { const id = absolutePath @@ -2251,7 +2277,15 @@ export const createEditorSlice: StateCreator = (s : { ...s.trackedConflictPathsByWorktree, [worktreeId]: nextTracked } } }) - void openWorkspaceEditorItem(get(), absolutePath, worktreeId, entry.path, 'editor') + void openWorkspaceEditorItem( + get(), + absolutePath, + worktreeId, + entry.path, + 'editor', + undefined, + options?.targetGroupId + ) }, openConflictReviewFile: (reviewFileId, worktreeId, worktreePath, entry, language) => {