diff --git a/apps/desktop/src/settings/DesktopClientSettings.test.ts b/apps/desktop/src/settings/DesktopClientSettings.test.ts index ea2a80010124..f6622049ab49 100644 --- a/apps/desktop/src/settings/DesktopClientSettings.test.ts +++ b/apps/desktop/src/settings/DesktopClientSettings.test.ts @@ -34,6 +34,7 @@ const clientSettings: ClientSettings = { contextWindowMeterEnabled: false, composerCollapseOnScroll: true, dismissedProviderUpdateNotificationKeys: [], + diffFilesCollapsed: true, diffIgnoreWhitespace: true, diffLayout: "stacked", environmentIdentificationMode: "artwork", diff --git a/apps/web/src/clientPersistenceStorage.test.ts b/apps/web/src/clientPersistenceStorage.test.ts index a86177b48eb3..17e531918ab1 100644 --- a/apps/web/src/clientPersistenceStorage.test.ts +++ b/apps/web/src/clientPersistenceStorage.test.ts @@ -114,6 +114,21 @@ describe("clientPersistenceStorage", () => { expect(settings).not.toHaveProperty("diffWordWrap"); }); + it("keeps the default diff file state across reloads and defaults it to expanded", async () => { + const testWindow = getTestWindow(); + const { readBrowserClientSettings, writeBrowserClientSettings } = + await import("./clientPersistenceStorage"); + + testWindow.localStorage.setItem("t3code:client-settings:v1", JSON.stringify({})); + expect(readBrowserClientSettings()?.diffFilesCollapsed).toBe(false); + + writeBrowserClientSettings({ ...DEFAULT_CLIENT_SETTINGS, diffFilesCollapsed: true }); + expect(readBrowserClientSettings()?.diffFilesCollapsed).toBe(true); + + writeBrowserClientSettings({ ...DEFAULT_CLIENT_SETTINGS, diffFilesCollapsed: false }); + expect(readBrowserClientSettings()?.diffFilesCollapsed).toBe(false); + }); + it("keeps the diff layout across reloads and defaults it to stacked", async () => { const testWindow = getTestWindow(); const { readBrowserClientSettings, writeBrowserClientSettings } = diff --git a/apps/web/src/components/DiffPanel.tsx b/apps/web/src/components/DiffPanel.tsx index d764b9c6a9e2..ca0bdaa7b4d5 100644 --- a/apps/web/src/components/DiffPanel.tsx +++ b/apps/web/src/components/DiffPanel.tsx @@ -226,10 +226,6 @@ export default function DiffPanel({ ? `${routeThreadRef.environmentId}:${routeThreadRef.threadId}:${reviewSectionId}` : null; const codeViewMountKey = `${collapseScopeKey ?? reviewSectionId}:${codeViewRevision}`; - const collapsedDiffFileKeys = - collapsedDiffFiles.scopeKey === collapseScopeKey - ? collapsedDiffFiles.fileKeys - : EMPTY_COLLAPSED_DIFF_FILE_KEYS; const reviewSectionTitle = selectedTurn ? `Turn ${selectedCheckpointTurnCount ?? "?"}` : selectedGitScope === "unstaged" @@ -425,6 +421,17 @@ export default function DiffPanel({ })), [renderableFiles], ); + const defaultCollapsedDiffFileKeys = useMemo( + () => + settings.diffFilesCollapsed + ? new Set(renderableFileEntries.map((file) => file.fileKey)) + : EMPTY_COLLAPSED_DIFF_FILE_KEYS, + [renderableFileEntries, settings.diffFilesCollapsed], + ); + const collapsedDiffFileKeys = + collapsedDiffFiles.scopeKey === collapseScopeKey + ? collapsedDiffFiles.fileKeys + : defaultCollapsedDiffFileKeys; const codeViewFiles = useMemo( () => renderableFileEntries.map(({ fileDiff, fileKey, fileVersion }) => { @@ -462,14 +469,16 @@ export default function DiffPanel({ if (!file) return; if (file.collapsed) { setCollapsedDiffFiles((current) => { - const next = new Set(current.scopeKey === collapseScopeKey ? current.fileKeys : []); + const next = new Set( + current.scopeKey === collapseScopeKey ? current.fileKeys : defaultCollapsedDiffFileKeys, + ); next.delete(file.fileKey); return { scopeKey: collapseScopeKey, fileKeys: next }; }); } requestTreeReveal(file.fileKey); }, - [codeViewFiles, collapseScopeKey, requestTreeReveal], + [codeViewFiles, collapseScopeKey, defaultCollapsedDiffFileKeys, requestTreeReveal], ); const openDiffFile = useCallback( @@ -503,7 +512,9 @@ export default function DiffPanel({ const toggleDiffFileCollapsed = useCallback( (fileKey: string) => { setCollapsedDiffFiles((current) => { - const next = new Set(current.scopeKey === collapseScopeKey ? current.fileKeys : []); + const next = new Set( + current.scopeKey === collapseScopeKey ? current.fileKeys : defaultCollapsedDiffFileKeys, + ); if (next.has(fileKey)) { next.delete(fileKey); } else { @@ -512,21 +523,21 @@ export default function DiffPanel({ return { scopeKey: collapseScopeKey, fileKeys: next }; }); }, - [collapseScopeKey], + [collapseScopeKey, defaultCollapsedDiffFileKeys], ); const toggleDiffFileCollapse = useCallback(() => { setCodeViewRevision((current) => current + 1); setCollapsedDiffFiles((current) => { const currentKeys = - current.scopeKey === collapseScopeKey ? current.fileKeys : EMPTY_COLLAPSED_DIFF_FILE_KEYS; + current.scopeKey === collapseScopeKey ? current.fileKeys : defaultCollapsedDiffFileKeys; return { scopeKey: collapseScopeKey, fileKeys: toggleAllDiffFiles(diffFileKeys, currentKeys), }; }); - }, [collapseScopeKey, diffFileKeys]); + }, [collapseScopeKey, defaultCollapsedDiffFileKeys, diffFileKeys]); const selectTurn = (turnId: TurnId) => { if (!routeThreadRef) return; diff --git a/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx b/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx index b77a3711d90f..b0bc3fdb8e22 100644 --- a/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx @@ -485,7 +485,11 @@ function PullRequestCodeTab({ groupAt(anchor.side, anchor.line).draft = true; } - const collapsed = isFileDiffCollapsed(fileKey, foldOverride, toggledFiles); + const collapsed = isFileDiffCollapsed( + fileKey, + foldOverride ?? (settings.diffFilesCollapsed ? "folded" : "expanded"), + toggledFiles, + ); const annotations: ReviewAnnotation[] = [...groups.values()].map((group) => ({ side: toViewerSide(group.side), @@ -538,6 +542,7 @@ function PullRequestCodeTab({ foldOverride, pendingComments, placedThreadIds, + settings.diffFilesCollapsed, toggledFiles, ], ); diff --git a/apps/web/src/components/pullRequest/pullRequestDiff.logic.ts b/apps/web/src/components/pullRequest/pullRequestDiff.logic.ts index a286a4552cb6..e818e989d200 100644 --- a/apps/web/src/components/pullRequest/pullRequestDiff.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDiff.logic.ts @@ -30,8 +30,8 @@ export type DiffFoldOverride = "expanded" | "folded" | null; * A diff arrives a slice at a time, so the reader's own choices are kept as the difference from * what the toolbar last said rather than as the set of folded files: a file that has not loaded * yet cannot be in a set, and would otherwise land expanded moments after the reader folded - * everything. Files start expanded so opening the Code tab immediately shows the change; the - * reader can still fold individual files or the whole diff from the toolbar. + * everything. The caller supplies the saved default until the toolbar overrides it; individual + * files can still be toggled independently. */ export function isFileDiffCollapsed( fileKey: string, diff --git a/apps/web/src/components/settings/SettingsPanels.tsx b/apps/web/src/components/settings/SettingsPanels.tsx index 8a78d4138c6b..98684f50721b 100644 --- a/apps/web/src/components/settings/SettingsPanels.tsx +++ b/apps/web/src/components/settings/SettingsPanels.tsx @@ -533,6 +533,9 @@ export function useSettingsRestore(onRestored?: () => void) { : []), ...(settings.wordWrap !== DEFAULT_UNIFIED_SETTINGS.wordWrap ? ["Word wrap"] : []), ...getChangedTypographySettingLabels(settings), + ...(settings.diffFilesCollapsed !== DEFAULT_UNIFIED_SETTINGS.diffFilesCollapsed + ? ["Default diff file state"] + : []), ...(settings.diffIgnoreWhitespace !== DEFAULT_UNIFIED_SETTINGS.diffIgnoreWhitespace ? ["Diff whitespace changes"] : []), @@ -608,6 +611,7 @@ export function useSettingsRestore(onRestored?: () => void) { settings.addProjectBaseDirectory, settings.defaultThreadEnvMode, settings.newWorktreesStartFromOrigin, + settings.diffFilesCollapsed, settings.diffIgnoreWhitespace, settings.diffLayout, settings.proactivePanelsEnabled, @@ -706,6 +710,7 @@ export function useSettingsRestore(onRestored?: () => void) { diffColorScheme: DEFAULT_UNIFIED_SETTINGS.diffColorScheme, timestampFormat: DEFAULT_UNIFIED_SETTINGS.timestampFormat, wordWrap: DEFAULT_UNIFIED_SETTINGS.wordWrap, + diffFilesCollapsed: DEFAULT_UNIFIED_SETTINGS.diffFilesCollapsed, diffIgnoreWhitespace: DEFAULT_UNIFIED_SETTINGS.diffIgnoreWhitespace, diffLayout: DEFAULT_UNIFIED_SETTINGS.diffLayout, proactivePanelsEnabled: DEFAULT_UNIFIED_SETTINGS.proactivePanelsEnabled, @@ -2304,6 +2309,48 @@ export function GeneralSettingsPanel() { /> } /> + + updateSettings({ + diffFilesCollapsed: DEFAULT_UNIFIED_SETTINGS.diffFilesCollapsed, + }) + } + /> + ) : null + } + control={ + + } + /> { }); }); +describe("ClientSettings default diff file state", () => { + it("keeps files expanded when existing settings omit the preference", () => { + expect(decodeClientSettings({}).diffFilesCollapsed).toBe(false); + }); + + it.each([true, false])("preserves a saved collapsed preference of %s", (diffFilesCollapsed) => { + const settings = decodeClientSettings({ diffFilesCollapsed }); + expect(encodeClientSettings(settings).diffFilesCollapsed).toBe(diffFilesCollapsed); + expect(decodeClientSettingsPatch({ diffFilesCollapsed }).diffFilesCollapsed).toBe( + diffFilesCollapsed, + ); + }); +}); + describe("ClientSettings diff colors", () => { it("keeps red and green for existing settings without a saved palette", () => { expect(decodeClientSettings({}).diffColorScheme).toBe("red-green"); diff --git a/packages/contracts/src/settings.ts b/packages/contracts/src/settings.ts index 6b931f4ef3ea..b420b737ee0a 100644 --- a/packages/contracts/src/settings.ts +++ b/packages/contracts/src/settings.ts @@ -344,6 +344,7 @@ export const ClientSettingsSchema = Schema.Struct({ dismissedProviderUpdateNotificationKeys: Schema.Array(TrimmedNonEmptyString).pipe( Schema.withDecodingDefault(Effect.succeed([])), ), + diffFilesCollapsed: Schema.Boolean.pipe(Schema.withDecodingDefault(Effect.succeed(false))), diffIgnoreWhitespace: Schema.Boolean.pipe(Schema.withDecodingDefault(Effect.succeed(true))), diffLayout: DiffLayout.pipe(Schema.withDecodingDefault(Effect.succeed(DEFAULT_DIFF_LAYOUT))), environmentIdentificationMode: EnvironmentIdentificationMode.pipe( @@ -1432,6 +1433,7 @@ export const ClientSettingsPatch = Schema.Struct({ confirmThreadArchive: Schema.optionalKey(Schema.Boolean), confirmThreadDelete: Schema.optionalKey(Schema.Boolean), confirmThreadUnpin: Schema.optionalKey(Schema.Boolean), + diffFilesCollapsed: Schema.optionalKey(Schema.Boolean), diffIgnoreWhitespace: Schema.optionalKey(Schema.Boolean), diffLayout: Schema.optionalKey(DiffLayout), environmentIdentificationMode: Schema.optionalKey(EnvironmentIdentificationMode),