From c704c9e4c2c780d73563866178c6abb95eccd1e1 Mon Sep 17 00:00:00 2001 From: Mark Street Date: Wed, 18 Mar 2026 16:44:43 +0000 Subject: [PATCH 1/2] Add ability to hide diff columns --- frontend/src/components/Diff/Diff.tsx | 320 +++++++++++------- .../src/components/Diff/DiffRowAsmDiffer.tsx | 29 +- frontend/src/components/Diff/ToggleButton.tsx | 39 +++ 3 files changed, 249 insertions(+), 139 deletions(-) create mode 100644 frontend/src/components/Diff/ToggleButton.tsx diff --git a/frontend/src/components/Diff/Diff.tsx b/frontend/src/components/Diff/Diff.tsx index 9d180bbb4..b5b380af5 100644 --- a/frontend/src/components/Diff/Diff.tsx +++ b/frontend/src/components/Diff/Diff.tsx @@ -4,12 +4,13 @@ import { forwardRef, type HTMLAttributes, type RefObject, + useLayoutEffect, useMemo, useRef, useState, } from "react"; -import { VersionsIcon, FoldIcon, UnfoldIcon } from "@primer/octicons-react"; +import { FoldIcon, UnfoldIcon } from "@primer/octicons-react"; import type { EditorView } from "codemirror"; import AutoSizer from "react-virtualized-auto-sizer"; import { FixedSizeList } from "react-window"; @@ -29,22 +30,17 @@ import * as AsmDiffer from "./DiffRowAsmDiffer"; import DragBar from "./DragBar"; import { useHighlighers } from "./Highlighter"; import CopyButton from "../CopyButton"; +import ToggleButton from "./ToggleButton"; -const diffContentsToString = (diff: api.DiffOutput, kind: string): string => { - // kind is either "base", "current", or "previous" - const contents = diff.rows.map((row) => { - let text = ""; - if (kind === "base" && row.base) { - text = row.base.text.map((t) => t.text).join(""); - } else if (kind === "current" && row.current) { - text = row.current.text.map((t) => t.text).join(""); - } else if (kind === "previous" && row.previous) { - text = row.previous.text.map((t) => t.text).join(""); - } - return text; - }); +type ColumnKey = "base" | "current" | "previous"; - return contents.join("\n"); +const diffContentsToString = ( + diff: api.DiffOutput, + kind: ColumnKey, +): string => { + return diff.rows + .map((row) => row[kind]?.text.map((t) => t.text).join("") ?? "") + .join("\n"); }; // https://github.com/bvaughn/react-window#can-i-add-padding-to-the-top-and-bottom-of-a-list @@ -65,16 +61,24 @@ const innerElementType = forwardRef< }); innerElementType.displayName = "innerElementType"; +export type VisibleRow = { + key: string; + cells: Array; + isPlaceholder?: boolean; +}; + function DiffBody({ diff, diffLabel, fontSize, compressionEnabled, + columns, }: { diff: api.DiffOutput | null; diffLabel: string | null; fontSize: number | undefined; compressionEnabled: boolean; + columns: Array; }) { const { highlighters, setHighlightAll } = useHighlighers(3); const [compressionContext] = diffCompressionContext(); @@ -96,14 +100,26 @@ function DiffBody({ [groups, expandedGroups], ); + const visibleRows: VisibleRow[] = useMemo(() => { + return flattened.map((row) => { + const isPlaceholder = row.base?.text?.[0]?.format === "diff_skip"; + + return { + key: row.key, + isPlaceholder, + cells: columns.map((col: ColumnKey) => row[col]), + }; + }); + }, [flattened, columns]); + const itemData: AsmDiffer.DiffListData = useMemo( () => ({ - rows: flattened, + rows: visibleRows, highlighters, onToggle: (key: string) => setExpandedGroups((prev) => ({ ...prev, [key]: !prev[key] })), }), - [flattened, highlighters], + [visibleRows, highlighters], ); if (!diff) { @@ -144,30 +160,6 @@ function DiffBody({ ); } -function ThreeWayToggleButton({ - enabled, - setEnabled, -}: { enabled: boolean; setEnabled: (enabled: boolean) => void }) { - return ( - - ); -} - function CompressToggleButton({ enabled, setEnabled, @@ -350,6 +342,8 @@ export function flattenGroups( return flat; } +type ColumnState = Record; + export type Props = { diff: api.DiffOutput | null; diffLabel: string | null; @@ -374,60 +368,112 @@ export default function Diff({ threeWayDiffBase, }: Props) { const [fontSize] = useCodeFontSize(); - const container = useSize(); - const [bar1Pos, setBar1Pos] = useState(Number.NaN); - const [bar2Pos, setBar2Pos] = useState(Number.NaN); - - const columnMinWidth = 100; - const clampedBar1Pos = Math.max( - columnMinWidth, - Math.min( - container.width - - columnMinWidth - - (threeWayDiffEnabled ? columnMinWidth : 0), - bar1Pos, - ), - ); - const clampedBar2Pos = threeWayDiffEnabled - ? Math.max( - clampedBar1Pos + columnMinWidth, - Math.min(container.width - columnMinWidth, bar2Pos), - ) - : container.width; - - // Distribute the bar positions across the container when its width changes - const updateBarPositions = (threeWayDiffEnabled: boolean) => { - const numSections = threeWayDiffEnabled ? 3 : 2; - setBar1Pos(container.width / numSections); - setBar2Pos((container.width / numSections) * 2); - }; - const lastContainerWidthRef = useRef(Number.NaN); - if (lastContainerWidthRef.current !== container.width && container.width) { - lastContainerWidthRef.current = container.width; - updateBarPositions(threeWayDiffEnabled); + const [bar1Pos, setBar1Pos] = useState(Number.NaN); + const [bar2Pos, setBar2Pos] = useState(Number.NaN); + + const [columnState, setColumnState] = useState({ + base: true, + current: true, + previous: threeWayDiffEnabled, + }); + + const columns = [ + columnState.base && "base", + columnState.current && "current", + columnState.previous && "previous", + ].filter((col): col is ColumnKey => Boolean(col)); + + const columnCount = columns.length || 1; + + const rightmostColumn = columns[columns.length - 1]; + + const prevRef = useRef({ width: 0, columnCount: 0 }); + + useLayoutEffect(() => { + const width = container.width; + + if (!width) return; + + const prev = prevRef.current; + const columnChanged = prev.columnCount !== columnCount; + const widthChanged = prev.width !== width; + + if (columnChanged) { + // Reset positions based on new column count + if (columnCount === 1) { + setBar1Pos(width); + setBar2Pos(width); + } else { + setBar1Pos(width / columnCount); + setBar2Pos((width / columnCount) * 2); + } + } else if (widthChanged) { + // Preserve ratios ONLY when width changes + const ratio1 = + Number.isNaN(bar1Pos) || !prev.width + ? 1 / columnCount + : bar1Pos / prev.width; + + const ratio2 = + Number.isNaN(bar2Pos) || !prev.width + ? 2 / columnCount + : bar2Pos / prev.width; + + setBar1Pos(width * ratio1); + setBar2Pos(width * ratio2); + } + + prevRef.current = { width, columnCount }; + }, [container.width, columnCount, bar1Pos, bar2Pos]); + + const columnMinWidth = 80; + + function clampBars() { + const width = container.width; + + if (!width || columnCount <= 1) { + return { bar1: width, bar2: width }; + } else if (columnCount === 2) { + const b1 = Number.isNaN(bar1Pos) ? width / 2 : bar1Pos; + + const clampedBar1 = Math.max( + columnMinWidth, + Math.min(width - columnMinWidth, b1), + ); + + return { bar1: clampedBar1, bar2: width }; + } else { + const b1 = Number.isNaN(bar1Pos) ? width / 3 : bar1Pos; + const b2 = Number.isNaN(bar2Pos) ? (width / 3) * 2 : bar2Pos; + + const clampedBar1 = Math.max( + columnMinWidth, + Math.min(width - columnMinWidth * 2, b1), + ); + + const clampedBar2 = Math.max( + clampedBar1 + columnMinWidth, + Math.min(width - columnMinWidth, b2), + ); + + return { bar1: clampedBar1, bar2: clampedBar2 }; + } } - const threeWayButton = ( - <> -
- { - updateBarPositions(enabled); - setThreeWayDiffEnabled(enabled); - }} - /> - - ); + const { bar1: clampedBar1Pos, bar2: clampedBar2Pos } = clampBars(); - const compressButton = ( - - ); + const updateColumns = (updater: (prev: ColumnState) => ColumnState) => { + setColumnState((prev) => { + const next = updater(prev); + if (!next.base && !next.current && !next.previous) return prev; + return next; + }); + }; + const setColumn = (key: ColumnKey, value: boolean) => { + updateColumns((prev) => ({ ...prev, [key]: value })); + }; return (
- - {threeWayDiffEnabled && ( + {columnCount >= 2 && ( + + )} + {columnCount === 3 && ( )}
-
- Target - - diffContentsToString(diff as api.DiffOutput, "base") - } - /> -
-
- Current - - diffContentsToString( - diff as api.DiffOutput, - "current", - ) - } - /> - {isCompiling && } - {!threeWayDiffEnabled && threeWayButton} - {!threeWayDiffEnabled && compressButton} -
- {threeWayDiffEnabled && ( -
- {threeWayDiffBase === ThreeWayDiffBase.SAVED - ? "Saved" - : "Previous"} + {columns.map((col) => ( +
+ {col === "base" && "Target"} + {col === "current" && "Current"} + {col === "previous" && + (threeWayDiffBase === ThreeWayDiffBase.SAVED + ? "Saved" + : "Previous")} - diffContentsToString( - diff as api.DiffOutput, - "previous", - ) + diff ? diffContentsToString(diff, col) : "" } /> - {threeWayButton} - {compressButton} + {col === "current" && isCompiling && ( + + )} + + {col === rightmostColumn && ( + <> +
+ + setColumn("base", enabled) + } + /> + + setColumn("current", enabled) + } + /> + { + setColumn("previous", enabled); + setThreeWayDiffEnabled(enabled); + }} + /> + + + )}
- )} + ))}
+
); diff --git a/frontend/src/components/Diff/DiffRowAsmDiffer.tsx b/frontend/src/components/Diff/DiffRowAsmDiffer.tsx index 7fbf5d516..d8c864c77 100644 --- a/frontend/src/components/Diff/DiffRowAsmDiffer.tsx +++ b/frontend/src/components/Diff/DiffRowAsmDiffer.tsx @@ -11,7 +11,7 @@ import * as settings from "@/lib/settings"; import { ScrollContext } from "../ScrollContext"; import { useSelectedSourceLine } from "../SelectedSourceLineContext"; -import { PADDING_TOP, scrollToLineNumber } from "./Diff"; +import { PADDING_TOP, scrollToLineNumber, type VisibleRow } from "./Diff"; import styles from "./Diff.module.scss"; import type { Highlighter } from "./Highlighter"; @@ -123,7 +123,7 @@ function DiffCell({ } export type DiffListData = { - rows: api.DiffRow[]; + rows: VisibleRow[]; highlighters: Highlighter[]; onToggle: (groupKey: string) => void; }; @@ -132,23 +132,34 @@ export const DiffRow = memo(function DiffRow({ data, index, style, -}: { data: DiffListData; index: number; style: CSSProperties }) { +}: { + data: DiffListData; + index: number; + style: CSSProperties; +}) { const row = data.rows[index]; - const isPlaceholder = row.base?.text?.[0]?.format === "diff_skip"; return (
  • data.onToggle(row.key) : undefined} + onClick={ + row.isPlaceholder ? () => data.onToggle(row.key) : undefined + } > - - - + {row.cells.map((cell, i) => + cell ? ( + + ) : null, + )}
    • ); }, areEqual); diff --git a/frontend/src/components/Diff/ToggleButton.tsx b/frontend/src/components/Diff/ToggleButton.tsx new file mode 100644 index 000000000..bd61dac9d --- /dev/null +++ b/frontend/src/components/Diff/ToggleButton.tsx @@ -0,0 +1,39 @@ +import clsx from "clsx"; + +type ToggleButtonProps = { + label: string; + enabled: boolean; + setEnabled: (enabled: boolean) => void; + title?: string; +}; + +export default function ToggleButton({ + label, + enabled, + setEnabled, + title, +}: ToggleButtonProps) { + return ( + + ); +} From 567eb62ca21ea0c4fabf1db4862ba0378357a200 Mon Sep 17 00:00:00 2001 From: Mark Street Date: Wed, 18 Mar 2026 21:47:42 +0000 Subject: [PATCH 2/2] PR tweaks from gpt --- frontend/src/components/Diff/Diff.tsx | 210 +++++++++----------------- frontend/src/components/Diff/hooks.ts | 78 ++++++++++ 2 files changed, 152 insertions(+), 136 deletions(-) create mode 100644 frontend/src/components/Diff/hooks.ts diff --git a/frontend/src/components/Diff/Diff.tsx b/frontend/src/components/Diff/Diff.tsx index b5b380af5..909ab2b86 100644 --- a/frontend/src/components/Diff/Diff.tsx +++ b/frontend/src/components/Diff/Diff.tsx @@ -4,9 +4,8 @@ import { forwardRef, type HTMLAttributes, type RefObject, - useLayoutEffect, + useEffect, useMemo, - useRef, useState, } from "react"; @@ -31,15 +30,18 @@ import DragBar from "./DragBar"; import { useHighlighers } from "./Highlighter"; import CopyButton from "../CopyButton"; import ToggleButton from "./ToggleButton"; +import { useResizableColumns } from "./hooks"; type ColumnKey = "base" | "current" | "previous"; +type ColumnState = Record; +const ALL_COLUMNS: ColumnKey[] = ["base", "current", "previous"]; const diffContentsToString = ( diff: api.DiffOutput, kind: ColumnKey, ): string => { return diff.rows - .map((row) => row[kind]?.text.map((t) => t.text).join("") ?? "") + .map((row) => row[kind]?.text?.map((t) => t.text).join("") ?? "") .join("\n"); }; @@ -95,29 +97,37 @@ function DiffBody({ Record >({}); + useEffect(() => { + setExpandedGroups({}); + }, [groups]); + const flattened = useMemo( () => flattenGroups(groups, expandedGroups), [groups, expandedGroups], ); const visibleRows: VisibleRow[] = useMemo(() => { + const getCells = (row: api.DiffRow) => columns.map((col) => row[col]); + return flattened.map((row) => { const isPlaceholder = row.base?.text?.[0]?.format === "diff_skip"; return { key: row.key, isPlaceholder, - cells: columns.map((col: ColumnKey) => row[col]), + cells: getCells(row), }; }); }, [flattened, columns]); - const itemData: AsmDiffer.DiffListData = useMemo( + const handleToggle = (key: string) => { + setExpandedGroups((prev) => ({ ...prev, [key]: !prev[key] })); + }; + const itemData = useMemo( () => ({ rows: visibleRows, highlighters, - onToggle: (key: string) => - setExpandedGroups((prev) => ({ ...prev, [key]: !prev[key] })), + onToggle: handleToggle, }), [visibleRows, highlighters], ); @@ -170,9 +180,7 @@ function CompressToggleButton({ return (