From b4b1d432c8aaa9050db12c91f5ae37aabe2a2bf1 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Wed, 29 Apr 2026 23:51:17 -0700 Subject: [PATCH] =?UTF-8?q?feat(review):=20diff=20display=20options=20?= =?UTF-8?q?=E2=80=94=20hide=20whitespace=20+=20quick-settings=20popover=20?= =?UTF-8?q?(#631)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(review): add hide whitespace setting for diffs Adds a "Hide Whitespace" toggle that suppresses whitespace-only changes in diffs, matching GitHub's ?w=1 behavior. Uses parseDiffFromFile with ignoreWhitespace when full file contents are available. Includes demo data with a whitespace-heavy file (settings.ts) and a vite dev server plugin to serve file contents in demo mode. Co-Authored-By: Claude Opus 4.6 (1M context) * feat(review): add quick-settings popover on file header Adds a gear icon to the file header that opens a compact Radix popover with all diff display options (style, overflow, indicators, inline diff, line numbers, background, hide whitespace). Provides quick access to settings without opening the full settings dialog. Exports option constants from Settings.tsx for reuse. Co-Authored-By: Claude Opus 4.6 (1M context) --------- Co-authored-by: Claude Opus 4.6 (1M context) --- apps/review/vite.config.ts | 21 ++- packages/review-editor/App.tsx | 4 +- .../components/DiffOptionsPopover.tsx | 122 ++++++++++++++ .../review-editor/components/DiffViewer.tsx | 16 +- .../review-editor/components/FileHeader.tsx | 2 + packages/review-editor/demoData.ts | 156 ++++++++++++++++++ .../review-editor/dock/ReviewStateContext.tsx | 1 + .../dock/panels/ReviewDiffPanel.tsx | 1 + packages/ui/components/Settings.tsx | 19 ++- packages/ui/config/settings.ts | 15 ++ 10 files changed, 347 insertions(+), 10 deletions(-) create mode 100644 packages/review-editor/components/DiffOptionsPopover.tsx diff --git a/apps/review/vite.config.ts b/apps/review/vite.config.ts index 24d90a6b..e0b18557 100644 --- a/apps/review/vite.config.ts +++ b/apps/review/vite.config.ts @@ -1,9 +1,26 @@ import path from 'path'; -import { defineConfig } from 'vite'; +import { defineConfig, type Plugin } from 'vite'; import react from '@vitejs/plugin-react'; import { viteSingleFile } from 'vite-plugin-singlefile'; import tailwindcss from '@tailwindcss/vite'; import pkg from '../../package.json'; +import { DEMO_FILE_CONTENTS } from '../../packages/review-editor/demoData'; + +function demoFileContentPlugin(): Plugin { + return { + name: 'demo-file-content', + configureServer(server) { + server.middlewares.use((req, res, next) => { + if (!req.url?.startsWith('/api/file-content')) return next(); + const filePath = new URL(req.url, 'http://localhost').searchParams.get('path'); + const entry = filePath ? DEMO_FILE_CONTENTS[filePath] : undefined; + if (!entry) return next(); + res.setHeader('Content-Type', 'application/json'); + res.end(JSON.stringify({ oldContent: entry.oldContent, newContent: entry.newContent })); + }); + }, + }; +} export default defineConfig({ server: { @@ -13,7 +30,7 @@ export default defineConfig({ define: { __APP_VERSION__: JSON.stringify(pkg.version), }, - plugins: [react(), tailwindcss(), viteSingleFile()], + plugins: [demoFileContentPlugin(), react(), tailwindcss(), viteSingleFile()], resolve: { alias: { '@': path.resolve(__dirname, '.'), diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index 94a9be23..9775399a 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -142,6 +142,7 @@ const ReviewApp: React.FC = () => { const diffLineDiffType = useConfigValue('diffLineDiffType'); const diffShowLineNumbers = useConfigValue('diffShowLineNumbers'); const diffShowBackground = useConfigValue('diffShowBackground'); + const diffHideWhitespace = useConfigValue('diffHideWhitespace'); const diffFontFamily = useConfigValue('diffFontFamily'); const diffFontSize = useConfigValue('diffFontSize'); @@ -1191,6 +1192,7 @@ const ReviewApp: React.FC = () => { lineDiffType: diffLineDiffType, disableLineNumbers: !diffShowLineNumbers, disableBackground: !diffShowBackground, + hideWhitespace: diffHideWhitespace, fontFamily: diffFontFamily || undefined, fontSize: diffFontSize || undefined, // Only propagate base for modes where it affects old/new content. Avoids @@ -1247,7 +1249,7 @@ const ReviewApp: React.FC = () => { openTourPanel: handleOpenTour, }), [ files, activeFileIndex, diffStyle, diffOverflow, diffIndicators, - diffLineDiffType, diffShowLineNumbers, diffShowBackground, + diffLineDiffType, diffShowLineNumbers, diffShowBackground, diffHideWhitespace, diffFontFamily, diffFontSize, activeDiffBase, committedBase, feedbackDiffContext, prReviewScopeLabel, prDiffScope, allAnnotations, externalAnnotations, selectedAnnotationId, pendingSelection, handleLineSelection, diff --git a/packages/review-editor/components/DiffOptionsPopover.tsx b/packages/review-editor/components/DiffOptionsPopover.tsx new file mode 100644 index 00000000..b7d820b6 --- /dev/null +++ b/packages/review-editor/components/DiffOptionsPopover.tsx @@ -0,0 +1,122 @@ +import React from 'react'; +import * as Popover from '@radix-ui/react-popover'; +import { configStore, useConfigValue } from '@plannotator/ui/config'; +import { + DIFF_STYLE_OPTIONS, + OVERFLOW_OPTIONS, + INDICATOR_OPTIONS, + LINE_DIFF_OPTIONS, +} from '@plannotator/ui/components/Settings'; + +function CompactSegmented({ options, value, onChange }: { + options: { value: T; label: string }[]; + value: T; + onChange: (v: T) => void; +}) { + return ( +
+ {options.map((opt) => ( + + ))} +
+ ); +} + +function CompactToggle({ checked, onChange, label }: { + checked: boolean; + onChange: (v: boolean) => void; + label: string; +}) { + return ( + + ); +} + +export const DiffOptionsPopover: React.FC = () => { + const diffStyle = useConfigValue('diffStyle'); + const diffOverflow = useConfigValue('diffOverflow'); + const diffIndicators = useConfigValue('diffIndicators'); + const diffLineDiffType = useConfigValue('diffLineDiffType'); + const diffShowLineNumbers = useConfigValue('diffShowLineNumbers'); + const diffShowBackground = useConfigValue('diffShowBackground'); + const diffHideWhitespace = useConfigValue('diffHideWhitespace'); + + return ( + + + + + + +
+
+
+
Layout
+
+
+ configStore.set('diffStyle', v)} /> +
+
+ configStore.set('diffOverflow', v)} /> +
+
+
+
+
Indicators
+ configStore.set('diffIndicators', v)} /> +
+
+
Inline diff
+ configStore.set('diffLineDiffType', v)} /> +
+
+ +
+ +
+ configStore.set('diffShowLineNumbers', v)} label="Line numbers" /> + configStore.set('diffShowBackground', v)} label="Diff background" /> + configStore.set('diffHideWhitespace', v)} label="Hide whitespace" /> +
+
+ + + + ); +}; diff --git a/packages/review-editor/components/DiffViewer.tsx b/packages/review-editor/components/DiffViewer.tsx index 195a442b..546d73dd 100644 --- a/packages/review-editor/components/DiffViewer.tsx +++ b/packages/review-editor/components/DiffViewer.tsx @@ -1,6 +1,6 @@ import React, { useMemo, useRef, useEffect, useLayoutEffect, useCallback, useState } from 'react'; import { FileDiff, type DiffLineAnnotation } from '@pierre/diffs/react'; -import { getSingularPatch, processFile } from '@pierre/diffs'; +import { getSingularPatch, processFile, parseDiffFromFile } from '@pierre/diffs'; import { CodeAnnotation, CodeAnnotationType, SelectedLineRange, DiffAnnotationMetadata, TokenAnnotationMeta, ConventionalLabel, ConventionalDecoration } from '@plannotator/ui/types'; import type { DiffTokenEventBaseProps } from '@pierre/diffs'; import { useTheme } from '@plannotator/ui/components/ThemeProvider'; @@ -127,6 +127,7 @@ interface DiffViewerProps { lineDiffType?: 'word-alt' | 'word' | 'char' | 'none'; disableLineNumbers?: boolean; disableBackground?: boolean; + hideWhitespace?: boolean; fontFamily?: string; fontSize?: string; annotations: CodeAnnotation[]; @@ -172,6 +173,7 @@ export const DiffViewer: React.FC = ({ lineDiffType, disableLineNumbers, disableBackground, + hideWhitespace, fontFamily, fontSize, annotations, @@ -292,19 +294,27 @@ export const DiffViewer: React.FC = ({ // Re-parse the patch with full file contents so hunk indices are computed // against the complete file (isPartial: false), enabling expansion. + // When hideWhitespace is on, recompute the diff from file contents to + // exclude whitespace-only changes (like GitHub's ?w=1). const augmentedDiff = useMemo(() => { if (!fileContents || fileContents.forPath !== filePath || (fileContents.old == null && fileContents.new == null)) return fileDiff; try { + if (hideWhitespace && fileContents.old != null && fileContents.new != null) { + return parseDiffFromFile( + { name: oldPath || filePath, contents: fileContents.old }, + { name: filePath, contents: fileContents.new }, + { ignoreWhitespace: true }, + ); + } const result = processFile(patch, { oldFile: fileContents.old != null ? { name: oldPath || filePath, contents: fileContents.old } : undefined, newFile: fileContents.new != null ? { name: filePath, contents: fileContents.new } : undefined, }); return result || fileDiff; } catch { - // Fall back to partial diff if file contents don't match hunks return fileDiff; } - }, [patch, filePath, oldPath, fileContents, fileDiff]); + }, [patch, filePath, oldPath, fileContents, fileDiff, hideWhitespace]); const previousScrollFilePathRef = useRef(filePath); useLayoutEffect(() => { diff --git a/packages/review-editor/components/FileHeader.tsx b/packages/review-editor/components/FileHeader.tsx index 30759cf0..abba5a4e 100644 --- a/packages/review-editor/components/FileHeader.tsx +++ b/packages/review-editor/components/FileHeader.tsx @@ -1,4 +1,5 @@ import React, { useEffect, useRef, useState } from 'react'; +import { DiffOptionsPopover } from './DiffOptionsPopover'; interface FileHeaderProps { filePath: string; @@ -196,6 +197,7 @@ export const FileHeader: React.FC = ({ )} +
); diff --git a/packages/review-editor/demoData.ts b/packages/review-editor/demoData.ts index fa1285eb..37a58678 100644 --- a/packages/review-editor/demoData.ts +++ b/packages/review-editor/demoData.ts @@ -2,6 +2,93 @@ * Demo diff data for development mode */ +/** + * Full old/new file contents for demo files that need file-content serving + * (e.g. for hideWhitespace to work in demo mode). Keyed by file path. + */ +export const DEMO_FILE_CONTENTS: Record = { + 'src/config/settings.ts': { + oldContent: `import { z } from 'zod'; + +const envSchema = z.object({ + NODE_ENV: z.enum(['development', 'staging', 'production']), + PORT: z.coerce.number().default(3000), + DATABASE_URL: z.string().url(), + REDIS_URL: z.string().url().optional(), + JWT_SECRET: z.string().min(32), + LOG_LEVEL: z.enum(['debug', 'info', 'warn', 'error']).default('info'), +}); + +export const config = envSchema.parse(process.env); + +export function getLogConfig() { + return { + level: config.LOG_LEVEL, + pretty: config.NODE_ENV === 'development', + timestamp: true, + }; +} + +export function getDatabaseConfig() { + return { + connectionString: config.DATABASE_URL, + pool: { + min: 2, + max: config.NODE_ENV === 'production' ? 20 : 5, + }, + ssl: config.NODE_ENV === 'production', + }; +} + +export function getCacheConfig() { + return { + url: config.REDIS_URL, + ttl: config.NODE_ENV === 'production' ? 3600 : 60, + prefix: 'app:', + }; +}`, + newContent: `import { z } from 'zod'; + +const envSchema = z.object({ + NODE_ENV: z.enum(['development', 'staging', 'production']), + PORT: z.coerce.number().default(3000), + DATABASE_URL: z.string().url(), + REDIS_URL: z.string().url().optional(), + JWT_SECRET: z.string().min(32), + LOG_LEVEL: z.enum(['debug', 'info', 'warn', 'error']).default('info'), +}); + +export const config = envSchema.parse(process.env); + +export function getLogConfig() { + return { + level: config.LOG_LEVEL, + pretty: config.NODE_ENV === 'development', + timestamp: true, + }; +} + +export function getDatabaseConfig() { + return { + connectionString: config.DATABASE_URL, + pool: { + min: 2, + max: config.NODE_ENV === 'production' ? 20 : 5, + }, + ssl: config.NODE_ENV === 'production', + }; +} + +export function getCacheConfig() { + return { + url: config.REDIS_URL, + ttl: config.NODE_ENV === 'production' ? 3600 : 60, + prefix: 'app:', + }; +}`, + }, +}; + export const DEMO_DIFF = `diff --git a/src/components/Button.tsx b/src/components/Button.tsx index 1234567..abcdefg 100644 --- a/src/components/Button.tsx @@ -197,6 +284,75 @@ index 4567890..defghij 100644 + delete: (id: string) => request(\`/users/\${id}\`, { method: 'DELETE' }), }, }; +diff --git a/src/config/settings.ts b/src/config/settings.ts +index 5678901..efghijk 100644 +--- a/src/config/settings.ts ++++ b/src/config/settings.ts +@@ -1,38 +1,38 @@ +-import { z } from 'zod'; ++import { z } from 'zod'; + + const envSchema = z.object({ +- NODE_ENV: z.enum(['development', 'staging', 'production']), +- PORT: z.coerce.number().default(3000), +- DATABASE_URL: z.string().url(), +- REDIS_URL: z.string().url().optional(), +- JWT_SECRET: z.string().min(32), +- LOG_LEVEL: z.enum(['debug', 'info', 'warn', 'error']).default('info'), ++ NODE_ENV: z.enum(['development', 'staging', 'production']), ++ PORT: z.coerce.number().default(3000), ++ DATABASE_URL: z.string().url(), ++ REDIS_URL: z.string().url().optional(), ++ JWT_SECRET: z.string().min(32), ++ LOG_LEVEL: z.enum(['debug', 'info', 'warn', 'error']).default('info'), + }); + + export const config = envSchema.parse(process.env); + + export function getLogConfig() { +- return { +- level: config.LOG_LEVEL, +- pretty: config.NODE_ENV === 'development', +- timestamp: true, +- }; ++ return { ++ level: config.LOG_LEVEL, ++ pretty: config.NODE_ENV === 'development', ++ timestamp: true, ++ }; + } + + export function getDatabaseConfig() { +- return { +- connectionString: config.DATABASE_URL, +- pool: { +- min: 2, +- max: config.NODE_ENV === 'production' ? 20 : 5, +- }, +- ssl: config.NODE_ENV === 'production', +- }; ++ return { ++ connectionString: config.DATABASE_URL, ++ pool: { ++ min: 2, ++ max: config.NODE_ENV === 'production' ? 20 : 5, ++ }, ++ ssl: config.NODE_ENV === 'production', ++ }; + } + + export function getCacheConfig() { +- return { +- url: config.REDIS_URL, +- ttl: config.NODE_ENV === 'production' ? 3600 : 60, +- prefix: 'app:', +- }; ++ return { ++ url: config.REDIS_URL, ++ ttl: config.NODE_ENV === 'production' ? 3600 : 60, ++ prefix: 'app:', ++ }; + } diff --git a/src/components/Modal.tsx b/src/components/Modal.tsx new file mode 100644 index 0000000..1234567 diff --git a/packages/review-editor/dock/ReviewStateContext.tsx b/packages/review-editor/dock/ReviewStateContext.tsx index 2ca097ae..1f71d6cb 100644 --- a/packages/review-editor/dock/ReviewStateContext.tsx +++ b/packages/review-editor/dock/ReviewStateContext.tsx @@ -26,6 +26,7 @@ export interface ReviewState { lineDiffType?: 'word-alt' | 'word' | 'char' | 'none'; disableLineNumbers?: boolean; disableBackground?: boolean; + hideWhitespace?: boolean; fontFamily?: string; fontSize?: string; /** User-selected base branch; feeds the `base` query param on file-content fetches. */ diff --git a/packages/review-editor/dock/panels/ReviewDiffPanel.tsx b/packages/review-editor/dock/panels/ReviewDiffPanel.tsx index 7de62e3e..7933648c 100644 --- a/packages/review-editor/dock/panels/ReviewDiffPanel.tsx +++ b/packages/review-editor/dock/panels/ReviewDiffPanel.tsx @@ -78,6 +78,7 @@ export const ReviewDiffPanel: React.FC = (props) => { lineDiffType={state.lineDiffType} disableLineNumbers={state.disableLineNumbers} disableBackground={state.disableBackground} + hideWhitespace={state.hideWhitespace} fontFamily={state.fontFamily} fontSize={state.fontSize} annotations={fileAnnotations} diff --git a/packages/ui/components/Settings.tsx b/packages/ui/components/Settings.tsx index 72a41b35..a6fe95b9 100644 --- a/packages/ui/components/Settings.tsx +++ b/packages/ui/components/Settings.tsx @@ -102,20 +102,20 @@ const DIFF_FONT_OPTIONS = [ { value: 'Atkinson Hyperlegible Mono', label: 'Atkinson Hyperlegible' }, ]; -const DIFF_STYLE_OPTIONS = [ +export const DIFF_STYLE_OPTIONS = [ { value: 'split' as const, label: 'Split' }, { value: 'unified' as const, label: 'Unified' }, ]; -const OVERFLOW_OPTIONS = [ +export const OVERFLOW_OPTIONS = [ { value: 'scroll' as const, label: 'Scroll' }, { value: 'wrap' as const, label: 'Wrap' }, ]; -const INDICATOR_OPTIONS = [ +export const INDICATOR_OPTIONS = [ { value: 'bars' as const, label: 'Bars' }, { value: 'classic' as const, label: 'Classic' }, { value: 'none' as const, label: 'None' }, ]; -const LINE_DIFF_OPTIONS = [ +export const LINE_DIFF_OPTIONS = [ { value: 'word-alt' as const, label: 'Word-Alt' }, { value: 'word' as const, label: 'Word' }, { value: 'char' as const, label: 'Char' }, @@ -228,6 +228,7 @@ const ReviewDisplayTab: React.FC = () => { const diffLineDiffType = useConfigValue('diffLineDiffType'); const diffShowLineNumbers = useConfigValue('diffShowLineNumbers'); const diffShowBackground = useConfigValue('diffShowBackground'); + const diffHideWhitespace = useConfigValue('diffHideWhitespace'); const diffFontFamily = useConfigValue('diffFontFamily'); const diffFontSize = useConfigValue('diffFontSize'); @@ -361,6 +362,16 @@ const ReviewDisplayTab: React.FC = () => { description="Colored backgrounds on added/deleted lines" /> +
+ + {/* Hide Whitespace */} + configStore.set('diffHideWhitespace', v)} + label="Hide Whitespace" + description="Ignore whitespace-only changes in diffs" + /> + ); }; diff --git a/packages/ui/config/settings.ts b/packages/ui/config/settings.ts index ef98d84b..4739c7d9 100644 --- a/packages/ui/config/settings.ts +++ b/packages/ui/config/settings.ts @@ -154,6 +154,21 @@ export const SETTINGS = { toServer: (v: string) => ({ diffOptions: { fontFamily: v } }), }, + diffHideWhitespace: { + defaultValue: false as boolean, + fromCookie: () => { + const v = storage.getItem('plannotator-diff-hide-whitespace'); + return v === 'true' ? true : v === 'false' ? false : undefined; + }, + toCookie: (v: boolean) => storage.setItem('plannotator-diff-hide-whitespace', String(v)), + serverKey: 'diffOptions', + fromServer: (sc: Record) => { + const v = (sc.diffOptions as Record | undefined)?.hideWhitespace; + return typeof v === 'boolean' ? v : undefined; + }, + toServer: (v: boolean) => ({ diffOptions: { hideWhitespace: v } }), + }, + diffFontSize: { defaultValue: '' as string, // empty = theme default fromCookie: () => storage.getItem('plannotator-diff-font-size') || undefined,