diff --git a/packages/app/e2e/browser/changes-pane.spec.ts b/packages/app/e2e/browser/changes-pane.spec.ts index 1caa7312b..b02743c50 100644 --- a/packages/app/e2e/browser/changes-pane.spec.ts +++ b/packages/app/e2e/browser/changes-pane.spec.ts @@ -596,32 +596,41 @@ test("Changes keeps review navigation and controls inside its workspace tab", as await expect(visiblePanel).toBeVisible(); await expect(visiblePanel.getByText("use-mounted-tab-set.ts", { exact: true })).toBeVisible(); await expect(visiblePanel).toContainText("zz-deleted.ts"); - await expect(visiblePanel.getByTestId("changes-open-tab")).toHaveCount(0); await expect(visiblePanel.getByTestId("changes-primary-cta")).toHaveCount(0); await expect(page.getByTestId("changes-primary-cta")).toHaveCount(1); await expect(page.getByTestId("changes-primary-cta")).toContainText("Commit"); await expect(visiblePanel.getByTestId("diff-file-0-body")).toBeVisible(); await visiblePanel.getByTestId("diff-file-0-toggle").click(); await expect(visiblePanel.getByTestId("diff-file-0-body")).not.toBeVisible(); - await visiblePanel.getByRole("button", { name: "Collapse all files" }).click(); - await expect(visiblePanel.getByRole("button", { name: "Expand all files" })).toBeVisible(); - await visiblePanel.getByRole("button", { name: "Expand all files" }).click(); + + // Collapse-all and the layout switch are overflow-menu items, not toolbar icons. + const diffOptions = visiblePanel.getByRole("button", { name: "Diff options" }); + await expect(page.getByTestId("changes-toggle-collapse-all")).toHaveCount(0); + await diffOptions.click(); + // A workspace tab has nowhere to open itself, so the menu withholds that item. + await expect(page.getByTestId("changes-open-tab")).toHaveCount(0); + await page.getByTestId("changes-toggle-collapse-all").click(); + await diffOptions.click(); + await expect(page.getByTestId("changes-toggle-collapse-all")).toContainText("Expand all files"); + await page.getByTestId("changes-toggle-collapse-all").click(); await expect(visiblePanel.getByTestId("diff-file-0-body")).toBeVisible(); - const workingDiffLayoutToggle = visiblePanel.getByRole("button", { - name: "Switch to side-by-side diff", - }); - await expect(workingDiffLayoutToggle).toHaveAccessibleName("Switch to side-by-side diff"); - await workingDiffLayoutToggle.click(); - await expect(visiblePanel.getByRole("button", { name: "Switch to unified diff" })).toBeVisible(); - await visiblePanel.getByRole("button", { name: "Diff options" }).click(); + + const layoutItem = page.getByTestId("changes-toggle-layout"); + await diffOptions.click(); + await expect(layoutItem).toContainText("Side-by-side diff"); + // Leading icon only: the layout is still unified, so no trailing check. + await expect(layoutItem.locator("svg")).toHaveCount(1); + await layoutItem.click(); + await diffOptions.click(); + await expect(layoutItem.locator("svg")).toHaveCount(2); + await expect(page.getByText("Hide whitespace", { exact: true })).toBeVisible(); await expect(page.getByText("Wrap long lines", { exact: true })).toBeVisible(); - await expect(page.getByText(/^Refresh/)).toBeVisible(); + await expect(page.getByTestId("changes-refresh")).toHaveText("Refresh"); await page.getByText("Wrap long lines", { exact: true }).click(); - await visiblePanel.getByRole("button", { name: "Diff options" }).click(); + await diffOptions.click(); await expect(page.getByText("Scroll long lines", { exact: true })).toBeVisible(); await page.keyboard.press("Escape"); - await expect(visiblePanel.getByRole("button", { name: "Collapse all files" })).toBeVisible(); await expect(page.getByTestId(/^workspace-working-diff-close-/)).toHaveCount(1); await writeFile(path.join(workspace.repoPath, "src/use-mounted-tab-set.ts"), BEFORE); @@ -645,15 +654,20 @@ test("desktop Changes toggles a navigation tree beside the expanded diff documen await openWorkspaceChanges(page, workspace); await expectFlatFileList(page); - await expect(page.getByTestId("changes-toggle-layout")).toBeVisible(); await expect(page.getByTestId("changes-layout-unified")).toHaveCount(0); await expect(page.getByTestId("changes-layout-split")).toHaveCount(0); + // The tree toggle is the one icon action that stays outside the overflow menu. + await expect(page.getByTestId("changes-toggle-tree")).toBeVisible(); + await expect(page.getByTestId("changes-toggle-collapse-all")).toHaveCount(0); + await expect(page.getByTestId("changes-toggle-layout")).toHaveCount(0); await page.getByTestId("changes-options-menu").click(); await expect(page.getByTestId("changes-options-menu-content")).toBeVisible(); + await expect(page.getByTestId("changes-toggle-collapse-all")).toContainText("Collapse all files"); + await expect(page.getByTestId("changes-toggle-layout")).toContainText("Side-by-side diff"); await expect(page.getByTestId("changes-toggle-whitespace")).toContainText("Hide whitespace"); await expect(page.getByTestId("changes-toggle-wrap-lines")).toContainText("Wrap long lines"); - await expect(page.getByTestId("changes-refresh")).toContainText("Refresh"); + await expect(page.getByTestId("changes-refresh")).toHaveText("Refresh"); await page.getByTestId("changes-toggle-whitespace").click(); await page.getByTestId("changes-options-menu").click(); await expect(page.getByTestId("changes-toggle-whitespace")).toContainText("Show whitespace"); diff --git a/packages/app/src/git/diff-pane.tsx b/packages/app/src/git/diff-pane.tsx index de59f700e..2774e19ef 100644 --- a/packages/app/src/git/diff-pane.tsx +++ b/packages/app/src/git/diff-pane.tsx @@ -2,6 +2,7 @@ import { useState, useCallback, useMemo, type ReactElement } from "react"; import { useTranslation } from "react-i18next"; import type { TFunction } from "i18next"; import { TreeRail } from "@/components/tree-rail"; +import { DiffStat } from "@/components/diff-stat"; import { View, Text, @@ -22,6 +23,7 @@ import { ListChevronsDownUp, ListChevronsUpDown, Maximize2, + MoreHorizontal, Pilcrow, RotateCw, WrapText, @@ -84,6 +86,22 @@ export function resolveDiffLayout( return canUseSplitLayout ? layout : "unified"; } +function computeSelectedDiffStat( + files: ParsedDiffFile[], + isLoading: boolean, +): { additions: number; deletions: number } | null { + if (isLoading) { + return null; + } + return files.reduce( + (total, file) => ({ + additions: total.additions + file.additions, + deletions: total.deletions + file.deletions, + }), + { additions: 0, deletions: 0 }, + ); +} + function useDiscardChangesAction({ serverId, cwd, @@ -163,12 +181,25 @@ const ThemedListChevronsDownUp = withUnistyles(ListChevronsDownUp); const ThemedListChevronsUpDown = withUnistyles(ListChevronsUpDown); const ThemedMaximize2 = withUnistyles(Maximize2); const ThemedChevronDown = withUnistyles(ChevronDown); +const ThemedMoreHorizontal = withUnistyles(MoreHorizontal); const DIFF_OPTIONS_WHITESPACE_ICON = ( ); const DIFF_OPTIONS_WRAP_ICON = ( ); +const DIFF_OPTIONS_SPLIT_ICON = ( + +); +const DIFF_OPTIONS_COLLAPSE_ICON = ( + +); +const DIFF_OPTIONS_EXPAND_ICON = ( + +); +const DIFF_OPTIONS_CHANGES_TAB_ICON = ( + +); interface DiffLayoutToggleProps { layout: "unified" | "split"; @@ -220,12 +251,6 @@ export function DiffLayoutToggle({ ); } -interface ChangesTabToggleProps { - isMobile: boolean; - selected: boolean; - onPress: () => void; -} - function resolveChangesTabOpen(host: "explorer" | "panel", changesTabOpen: boolean): boolean { return host === "explorer" ? changesTabOpen : false; } @@ -237,18 +262,6 @@ function resolveChangesFilePress( return host === "explorer" ? onChangesFilePress : undefined; } -function ChangesTabToggleForHost({ - host, - isMobile, - selected, - onPress, -}: ChangesTabToggleProps & { host: "explorer" | "panel" }) { - if (host === "panel") { - return null; - } - return ; -} - interface DiffModeMenuProps { diffMode: "uncommitted" | "base"; committedDescription?: string; @@ -303,38 +316,6 @@ export function DiffModeMenu({ ); } -function ChangesTabToggle({ isMobile, selected, onPress }: ChangesTabToggleProps) { - const { t } = useTranslation(); - const buttonStyle = useMemo( - () => buildToggleButtonStyle(selected, styles.toolbarIconButton), - [selected], - ); - const label = t( - selected ? "workspace.git.diff.closeChangesTab" : "workspace.git.diff.openChangesTab", - ); - if (isMobile) { - return null; - } - return ( - - - - - - - - {label} - - - ); -} - interface DesktopTreeToggleProps { visible: boolean; toggleStyle?: PressableStyleFn; @@ -368,54 +349,6 @@ function DesktopTreeToggle({ visible, toggleStyle, onToggle }: DesktopTreeToggle ); } -interface CollapseAllToggleProps { - allCollapsed: boolean; - isMobile: boolean; - onCollapseAll: () => void; - onExpandAll: () => void; -} - -function CollapseAllToggle({ - allCollapsed, - isMobile, - onCollapseAll, - onExpandAll, -}: CollapseAllToggleProps) { - const { t } = useTranslation(); - const buttonStyle = useMemo(() => buildToggleButtonStyle(false, styles.toolbarIconButton), []); - const label = t( - allCollapsed ? "workspace.git.diff.expandAllFiles" : "workspace.git.diff.collapseAllFiles", - ); - return ( - - - - {allCollapsed ? ( - - ) : ( - - )} - - - - {label} - - - ); -} - interface ChangesToolbarProps { branchName: string | null; allFilesCollapsed: boolean; @@ -425,7 +358,6 @@ interface ChangesToolbarProps { cwd: string; desktopTreeVisible: boolean; diffMode: "uncommitted" | "base"; - forge: Forge; gitActions: GitActions; hasFiles: boolean; hideWhitespace: boolean; @@ -433,9 +365,9 @@ interface ChangesToolbarProps { isMobile: boolean; isRefreshing: boolean; layout: "unified" | "split"; - layoutToggleStyle: PressableStyleFn; overflowToggleStyle: PressableStyleFn; refreshSupported: boolean; + selectedDiffStat: { additions: number; deletions: number } | null; serverId: string; treeToggleStyle: PressableStyleFn; workspaceId?: string | null; @@ -452,155 +384,129 @@ interface ChangesToolbarProps { onToggleWrapLines: () => void; } -function ChangesToolbar({ - branchName, +// One row: the diff-mode and branch pickers lead, the git actions and the +// overflow menu trail. The tree toggle is the only icon action that stays out +// of the menu, and it only exists on desktop, so a phone-width row holds two +// pickers, the split button, and the trigger without wrapping. +function ChangesToolbar(props: ChangesToolbarProps) { + const { + branchName, + committedDescription, + cwd, + desktopTreeVisible, + diffMode, + gitActions, + hasFiles, + isMobile, + selectedDiffStat, + serverId, + treeToggleStyle, + workspaceId, + onSelectBase, + onSelectUncommitted, + onToggleDesktopTree, + } = props; + return ( + + + + + {!isMobile && selectedDiffStat ? ( + + ) : null} + + + {!isMobile && hasFiles ? ( + + ) : null} + {isMobile ? : null} + + + + ); +} + +type ChangesOptionsMenuProps = Pick< + ChangesToolbarProps, + | "allFilesCollapsed" + | "canUseSplitLayout" + | "changesTabOpen" + | "hasFiles" + | "hideWhitespace" + | "host" + | "isMobile" + | "isRefreshing" + | "layout" + | "overflowToggleStyle" + | "refreshSupported" + | "wrapLines" + | "onCollapseAll" + | "onExpandAll" + | "onRefresh" + | "onToggleChangesTab" + | "onToggleHideWhitespace" + | "onToggleLayout" + | "onToggleWrapLines" +>; + +function ChangesOptionsMenu({ allFilesCollapsed, canUseSplitLayout, changesTabOpen, - committedDescription, - cwd, - desktopTreeVisible, - diffMode, - forge, - gitActions, hasFiles, hideWhitespace, host, isMobile, isRefreshing, layout, - layoutToggleStyle, overflowToggleStyle, refreshSupported, - serverId, - treeToggleStyle, - workspaceId, wrapLines, - onRefresh, onCollapseAll, onExpandAll, - onSelectBase, - onSelectUncommitted, + onRefresh, onToggleChangesTab, - onToggleDesktopTree, onToggleHideWhitespace, onToggleLayout, onToggleWrapLines, -}: ChangesToolbarProps) { - return ( - - - - - - - - - {hasFiles ? ( - - ) : null} - {canUseSplitLayout && !changesTabOpen ? ( - - ) : null} - {!isMobile && hasFiles ? ( - - ) : null} - - - - {isMobile ? ( - - - - ) : null} - - ); -} - -interface DiffOptionsMenuProps { - brand?: string; - hideWhitespace: boolean; - isMobile: boolean; - isRefreshing?: boolean; - overflowToggleStyle?: PressableStyleFn; - refreshSupported?: boolean; - testIDPrefix?: string; - wrapLines: boolean; - onRefresh?: () => void; - onToggleHideWhitespace: () => void; - onToggleWrapLines: () => void; -} - -export function DiffOptionsMenu({ - brand, - hideWhitespace, - isMobile, - isRefreshing = false, - overflowToggleStyle, - refreshSupported = false, - testIDPrefix = "changes", - wrapLines, - onRefresh, - onToggleHideWhitespace, - onToggleWrapLines, -}: DiffOptionsMenuProps) { +}: ChangesOptionsMenuProps) { const { t } = useTranslation(); - const defaultToggleStyle = useMemo(() => buildOverflowButtonStyle(), []); + const optionsLabel = t("workspace.git.diff.options"); + const collapseLabel = t( + allFilesCollapsed ? "workspace.git.diff.expandAllFiles" : "workspace.git.diff.collapseAllFiles", + ); + const changesTabLabel = t( + changesTabOpen ? "workspace.git.diff.closeChangesTab" : "workspace.git.diff.openChangesTab", + ); const whitespaceLabel = hideWhitespace ? t("workspace.git.diff.showWhitespace") : t("workspace.git.diff.hideWhitespace"); const wrapLinesLabel = wrapLines ? t("workspace.git.diff.scrollLongLines") : t("workspace.git.diff.wrapLongLines"); - const optionsLabel = t("workspace.git.diff.options"); - let refreshLabel = t("workspace.git.diff.refresh"); - if (isRefreshing) { - refreshLabel = t("workspace.git.diff.refreshing"); - } else if (brand) { - refreshLabel = t("workspace.git.diff.refreshState", { brand }); - } + const refreshLabel = isRefreshing + ? t("workspace.git.diff.refreshing") + : t("workspace.git.diff.refresh"); const refreshIcon = useMemo( () => isRefreshing ? ( @@ -611,6 +517,9 @@ export function DiffOptionsMenu({ [isRefreshing], ); + const showChangesTab = host === "explorer" && !isMobile; + const showLayout = canUseSplitLayout && !changesTabOpen; + return ( @@ -618,10 +527,10 @@ export function DiffOptionsMenu({ - @@ -631,11 +540,40 @@ export function DiffOptionsMenu({ {optionsLabel} - + + {hasFiles ? ( + + {collapseLabel} + + ) : null} + {showChangesTab ? ( + + {changesTabLabel} + + ) : null} + {hasFiles || showChangesTab ? : null} + {showLayout ? ( + + {t("workspace.git.diff.split")} + + ) : null} {whitespaceLabel} @@ -643,18 +581,18 @@ export function DiffOptionsMenu({ {wrapLinesLabel} - {refreshSupported && onRefresh ? ( + {refreshSupported ? ( <> {refreshLabel} @@ -1188,11 +1126,6 @@ export function ChangesSurface({ }, [changesPreferences.layout, updateChangesPreferences]); const codeFontSize = appSettings.codeFontSize; - const layoutToggleStyle = useMemo( - () => buildToggleButtonStyle(false, styles.toolbarIconButton), - [], - ); - const treeToggleStyle = useMemo( () => buildToggleButtonStyle(desktopTreeVisible, styles.toolbarIconButton), [desktopTreeVisible], @@ -1419,6 +1352,10 @@ export function ChangesSurface({ ); const hasChanges = files.length > 0; + const selectedDiffStat = useMemo( + () => computeSelectedDiffStat(files, isDiffLoading), + [files, isDiffLoading], + ); const allFilesCollapsed = hasChanges && files.every((file) => collapsedFilePaths.includes(file.path)); const handleCollapseAllFiles = useCallback( @@ -1506,7 +1443,6 @@ export function ChangesSurface({ cwd={cwd} desktopTreeVisible={desktopTreeVisible} diffMode={diffMode} - forge={forge} gitActions={gitActions} hasFiles={hasChanges} hideWhitespace={changesPreferences.hideWhitespace} @@ -1514,9 +1450,9 @@ export function ChangesSurface({ isMobile={isMobile} isRefreshing={isRefreshing} layout={changesPreferences.layout} - layoutToggleStyle={layoutToggleStyle} overflowToggleStyle={overflowToggleStyle} refreshSupported={refreshSupported} + selectedDiffStat={selectedDiffStat} serverId={serverId} treeToggleStyle={treeToggleStyle} workspaceId={workspaceId} @@ -1555,11 +1491,6 @@ const styles = StyleSheet.create((theme) => ({ minHeight: 0, }, changesToolbar: { - borderBottomWidth: 1, - borderBottomColor: theme.colors.border, - flexShrink: 0, - }, - changesToolbarMainRow: { minHeight: WORKSPACE_SECONDARY_HEADER_HEIGHT, flexDirection: "row", alignItems: "center", @@ -1567,6 +1498,9 @@ const styles = StyleSheet.create((theme) => ({ gap: theme.spacing[2], paddingLeft: theme.spacing[2], paddingRight: theme.spacing[3], + borderBottomWidth: 1, + borderBottomColor: theme.colors.border, + flexShrink: 0, }, changesToolbarIdentity: { minWidth: 0, @@ -1582,15 +1516,6 @@ const styles = StyleSheet.create((theme) => ({ gap: theme.spacing[1], flexShrink: 0, }, - mobileGitActionsRow: { - minHeight: WORKSPACE_SECONDARY_HEADER_HEIGHT, - flexDirection: "row", - alignItems: "center", - justifyContent: "flex-end", - paddingHorizontal: theme.spacing[3], - borderTopWidth: 1, - borderTopColor: theme.colors.border, - }, diffModeTrigger: { flexDirection: "row", alignItems: "center",