From 69417adcbf410ec2869633e888e350b117cd7714 Mon Sep 17 00:00:00 2001 From: Abhishek S Lal Date: Tue, 5 May 2026 22:26:46 +0530 Subject: [PATCH] feat(openapi-sync): virtualize spec diff rendering + spec change block navigation (#7848) * feat: implement side-by-side diff viewer for spec synchronization - Added a new SpecDiffModal component to display differences between current and updated specs. - Introduced buildRows function to flatten parsed diff data for rendering. - Created DiffRow component for rendering individual rows in the diff view. - Implemented highlightCache for efficient word-level diff highlighting. - Enhanced user experience with navigation controls for changes and loading indicators. - Added tests for buildRows functionality to ensure accurate diff representation. * fix: update comments and dependencies for consistency in SpecDiffModal and StyledWrapper - Added a comment in StyledWrapper.js to clarify the min-height requirement for Virtuoso's fixedItemHeight. - Updated comment in highlightCache.js to reflect the change from character-level to word-level diff highlighting. - Adjusted dependency array in SpecDiffModal.js to include cache for improved performance. --- .../OpenAPISyncTab/SpecDiffModal/DiffRow.js | 46 +++ .../SpecDiffModal/__tests__/buildRows.spec.js | 160 +++++++++++ .../OpenAPISyncTab/SpecDiffModal/buildRows.js | 164 +++++++++++ .../SpecDiffModal/highlightCache.js | 55 ++++ .../OpenAPISyncTab/SpecDiffModal/index.js | 163 ++++++++--- .../OpenAPISyncTab/StyledWrapper.js | 268 +++++++++++------- .../OpenAPISyncTab/SyncReviewPage/index.js | 24 +- 7 files changed, 737 insertions(+), 143 deletions(-) create mode 100644 packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/DiffRow.js create mode 100644 packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/__tests__/buildRows.spec.js create mode 100644 packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/buildRows.js create mode 100644 packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/highlightCache.js diff --git a/packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/DiffRow.js b/packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/DiffRow.js new file mode 100644 index 000000000..920dffa8e --- /dev/null +++ b/packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/DiffRow.js @@ -0,0 +1,46 @@ +import React from 'react'; + +/** + * One virtualized row in the spec diff. Renders the side-by-side cells + * (left line number, left code, right line number, right code) for a normal + * row, or a single full-width cell for a hunk header. + * + * Paired del+ins rows render via dangerouslySetInnerHTML so the / + * markup from the word-level diff cache shows through. Solo rows render as + * React text children and let React handle escaping. + */ +const DiffRow = ({ row, active, cache }) => { + if (!row) return null; // guard: Virtuoso race on rapid open/close or theme switch + if (row.leftKind === 'hunk') { + return ( +
+
{row.leftText}
+
+ ); + } + + const isChange = row.leftKind === 'del' && row.rightKind === 'ins'; + const wd = isChange ? cache.getWordDiff(row.leftText, row.rightText) : null; + + const renderContent = (text, html) => + html !== null + ? + : {text}; + + return ( +
+
{row.leftNum ?? ''}
+
+ {row.leftKind === 'del' ? '-' : ' '} + {renderContent(row.leftText, wd ? wd.left : null)} +
+
{row.rightNum ?? ''}
+
+ {row.rightKind === 'ins' ? '+' : ' '} + {renderContent(row.rightText, wd ? wd.right : null)} +
+
+ ); +}; + +export default React.memo(DiffRow); diff --git a/packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/__tests__/buildRows.spec.js b/packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/__tests__/buildRows.spec.js new file mode 100644 index 000000000..8d8593667 --- /dev/null +++ b/packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/__tests__/buildRows.spec.js @@ -0,0 +1,160 @@ +import { buildRows, wrapIndex } from '../buildRows'; + +// Helpers to construct fixture "parsed" data in the shape Diff2Html.parse() +// actually returns. Line types come from the LineType enum +// ('context' | 'insert' | 'delete'), NOT the CSSLineClass enum +// ('d2h-cntx' | 'd2h-ins' | 'd2h-del'). Verified at +// packages/bruno-app/public/static/diff2Html.js:3172. +const ctx = (text, oldNum, newNum) => ({ + type: 'context', + content: ` ${text}`, + oldNumber: oldNum, + newNumber: newNum +}); +const del = (text, oldNum) => ({ type: 'delete', content: `-${text}`, oldNumber: oldNum }); +const ins = (text, newNum) => ({ type: 'insert', content: `+${text}`, newNumber: newNum }); +const block = (header, lines) => ({ header, lines }); +const file = (...blocks) => [{ blocks }]; + +describe('buildRows', () => { + test('1. empty/missing input → empty rows and changeBlocks', () => { + expect(buildRows(null)).toEqual({ rows: [], changeBlocks: [] }); + expect(buildRows(undefined)).toEqual({ rows: [], changeBlocks: [] }); + expect(buildRows([])).toEqual({ rows: [], changeBlocks: [] }); + expect(buildRows([{ blocks: [] }])).toEqual({ rows: [], changeBlocks: [] }); + }); + + test('2. all-context hunk → 0 change blocks, only ctx + hunk rows', () => { + const parsed = file(block('@@ -1,3 +1,3 @@', [ctx('a', 1, 1), ctx('b', 2, 2), ctx('c', 3, 3)])); + const { rows, changeBlocks } = buildRows(parsed); + expect(changeBlocks).toEqual([]); + expect(rows).toHaveLength(4); // 1 hunk + 3 ctx + expect(rows[0].leftKind).toBe('hunk'); + expect(rows[1].leftKind).toBe('ctx'); + expect(rows[1].leftText).toBe('a'); + expect(rows[1].rightText).toBe('a'); + expect(rows[1].leftNum).toBe(1); + expect(rows[1].rightNum).toBe(1); + }); + + test('3. pure-deletion run → del rows with empty placeholders on right', () => { + const parsed = file( + block('@@ -1,3 +1,1 @@', [ctx('keep', 1, 1), del('gone1', 2), del('gone2', 3)]) + ); + const { rows, changeBlocks } = buildRows(parsed); + expect(rows).toHaveLength(4); // 1 hunk + 1 ctx + 2 del rows + expect(rows[2].leftKind).toBe('del'); + expect(rows[2].rightKind).toBe('empty'); + expect(rows[2].leftText).toBe('gone1'); + expect(rows[2].rightText).toBe(''); + expect(rows[2].leftNum).toBe(2); + expect(rows[2].rightNum).toBeNull(); + expect(rows[3].leftKind).toBe('del'); + expect(rows[3].leftText).toBe('gone2'); + // Two consecutive deletions form one block + expect(changeBlocks).toEqual([{ startIdx: 2, endIdx: 3 }]); + }); + + test('4. pure-insertion run → empty placeholders on left, ins on right', () => { + const parsed = file( + block('@@ -1,1 +1,3 @@', [ctx('keep', 1, 1), ins('new1', 2), ins('new2', 3)]) + ); + const { rows, changeBlocks } = buildRows(parsed); + expect(rows).toHaveLength(4); + expect(rows[2].leftKind).toBe('empty'); + expect(rows[2].rightKind).toBe('ins'); + expect(rows[2].leftText).toBe(''); + expect(rows[2].rightText).toBe('new1'); + expect(rows[2].leftNum).toBeNull(); + expect(rows[2].rightNum).toBe(2); + expect(changeBlocks).toEqual([{ startIdx: 2, endIdx: 3 }]); + }); + + test('matched del+ins pair → paired row with leftKind=del, rightKind=ins', () => { + const parsed = file(block('@@ -1,1 +1,1 @@', [del('old', 1), ins('new', 1)])); + const { rows, changeBlocks } = buildRows(parsed); + expect(rows).toHaveLength(2); // hunk + 1 paired change row + // Paired row wears natural del/ins kinds — DiffRow detects this combo + // to run word-level diff. Matches GitHub's side-by-side convention + // (red left = deleted content, green right = inserted content). + expect(rows[1].leftKind).toBe('del'); + expect(rows[1].rightKind).toBe('ins'); + expect(rows[1].leftText).toBe('old'); + expect(rows[1].rightText).toBe('new'); + expect(rows[1].leftNum).toBe(1); + expect(rows[1].rightNum).toBe(1); + expect(changeBlocks).toEqual([{ startIdx: 1, endIdx: 1 }]); + }); + + test('5. multi-hunk diff → hunk rows insert correctly + blocks segment per change region', () => { + const parsed = file( + block('@@ -1,2 +1,2 @@', [ctx('a', 1, 1), del('b', 2), ins('B', 2)]), + block('@@ -10,2 +10,2 @@', [ctx('x', 10, 10), del('y', 11), ins('Y', 11)]) + ); + const { rows, changeBlocks } = buildRows(parsed); + // Block 1: hunk + ctx + 1 paired change = 3 rows + // Block 2: hunk + ctx + 1 paired change = 3 rows + expect(rows).toHaveLength(6); + expect(rows[0].leftKind).toBe('hunk'); + expect(rows[3].leftKind).toBe('hunk'); + // Two distinct change blocks (separated by hunk header reset) + expect(changeBlocks).toEqual([ + { startIdx: 2, endIdx: 2 }, + { startIdx: 5, endIdx: 5 } + ]); + }); + + test('6. REGRESSION: change-block count matches expected counts for 3 fixture shapes', () => { + // The old DOM walker counted contiguous DOM rows containing + // .d2h-ins/.d2h-del/.d2h-change as one block. The new row-list walker + // must produce the same count for the same diff shape. + + // Fixture A: small diff, one contiguous change region + const fixtureA = file( + block('@@ -1,4 +1,4 @@', [ctx('a', 1, 1), del('b', 2), ins('B', 2), ctx('c', 3, 3)]) + ); + expect(buildRows(fixtureA).changeBlocks).toHaveLength(1); + + // Fixture B: medium, two separate change regions in one hunk + const fixtureB = file( + block('@@ -1,7 +1,7 @@', [ + ctx('a', 1, 1), + del('b', 2), + ins('B', 2), + ctx('c', 3, 3), + ctx('d', 4, 4), + del('e', 5), + ins('E', 5), + ctx('f', 6, 6) + ]) + ); + expect(buildRows(fixtureB).changeBlocks).toHaveLength(2); + + // Fixture C: multi-hunk with adjacent del+ins runs that form a single + // contiguous change region per hunk + const fixtureC = file( + block('@@ -1,3 +1,4 @@', [ctx('a', 1, 1), del('b', 2), ins('B', 2), ins('C', 3)]), + block('@@ -10,4 +11,4 @@', [ + ctx('x', 10, 11), + del('y', 11), + del('z', 12), + ins('Y', 12), + ins('Z', 13) + ]) + ); + expect(buildRows(fixtureC).changeBlocks).toHaveLength(2); + }); +}); + +describe('wrapIndex', () => { + test('7. wrap-around modulo handles negative and overflow', () => { + expect(wrapIndex(0, 5)).toBe(0); + expect(wrapIndex(4, 5)).toBe(4); + expect(wrapIndex(5, 5)).toBe(0); + expect(wrapIndex(6, 5)).toBe(1); + expect(wrapIndex(-1, 5)).toBe(4); + expect(wrapIndex(-6, 5)).toBe(4); + expect(wrapIndex(0, 0)).toBe(0); + expect(wrapIndex(3, 0)).toBe(0); + }); +}); diff --git a/packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/buildRows.js b/packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/buildRows.js new file mode 100644 index 000000000..ca2d38754 --- /dev/null +++ b/packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/buildRows.js @@ -0,0 +1,164 @@ +/** + * Flatten Diff2Html's parsed unified-diff output into what the virtualized + * renderer needs: + * + * rows[] — one entry per visual row in the side-by-side layout + * (exactly what Virtuoso renders) + * changeBlocks[] — index ranges into rows[], drives Next/Prev navigation + * + * Row shape: + * { leftNum, leftText, leftKind, rightNum, rightText, rightKind } + * *Kind ∈ 'ctx' | 'del' | 'ins' | 'empty' | 'hunk' + * + * When a row has leftKind='del' AND rightKind='ins', DiffRow recognises it + * as a matched change and renders word-level highlights. + */ + +// Diff2Html's parse() leaves the leading '+' / '-' / ' ' on each line's +// content. DiffRow renders that marker in its own styled span, so we strip +// it from the displayed text. +const stripLeadingMarker = (content) => (content || '').replace(/^[+\- ]/, ''); + +// Row factories — keep the row object shape consistent in one place. +const hunkRow = (header) => ({ + leftKind: 'hunk', + rightKind: 'hunk', + leftText: header, + rightText: header, + leftNum: null, + rightNum: null +}); + +const contextRow = (line) => ({ + leftKind: 'ctx', + rightKind: 'ctx', + leftText: stripLeadingMarker(line.content), + rightText: stripLeadingMarker(line.content), + leftNum: line.oldNumber ?? null, + rightNum: line.newNumber ?? null +}); + +const pairedChangeRow = (deletion, insertion) => ({ + leftKind: 'del', + rightKind: 'ins', + leftText: stripLeadingMarker(deletion.content), + rightText: stripLeadingMarker(insertion.content), + leftNum: deletion.oldNumber ?? null, + rightNum: insertion.newNumber ?? null +}); + +const soloDeletionRow = (deletion) => ({ + leftKind: 'del', + rightKind: 'empty', + leftText: stripLeadingMarker(deletion.content), + rightText: '', + leftNum: deletion.oldNumber ?? null, + rightNum: null +}); + +const soloInsertionRow = (insertion) => ({ + leftKind: 'empty', + rightKind: 'ins', + leftText: '', + rightText: stripLeadingMarker(insertion.content), + leftNum: null, + rightNum: insertion.newNumber ?? null +}); + +export function buildRows(parsed) { + const rows = []; + + if (!parsed || !Array.isArray(parsed) || parsed.length === 0) { + return { rows, changeBlocks: [] }; + } + + // Spec sync always produces a single-file diff; ignore any others. + const hunks = parsed[0]?.blocks || []; + + // ── Pass 1: flatten each hunk's lines into visual rows ── + for (const hunk of hunks) { + if (hunk.header) rows.push(hunkRow(hunk.header)); + + const lines = hunk.lines || []; + let i = 0; + + while (i < lines.length) { + const line = lines[i]; + + if (line.type === 'context') { + rows.push(contextRow(line)); + i++; + continue; + } + + // Collect the next run of deletions, then the run of insertions that + // immediately follows. Pair them 1:1 into side-by-side change rows; + // any leftovers spill as solo rows. + // + // e.g. del A, del B, del C, ins X, ins Y + // → (A ↔ X) (B ↔ Y) (C ↔ ∅) + const deletions = []; + while (i < lines.length && lines[i].type === 'delete') { + deletions.push(lines[i]); + i++; + } + const insertions = []; + while (i < lines.length && lines[i].type === 'insert') { + insertions.push(lines[i]); + i++; + } + + const pairCount = Math.min(deletions.length, insertions.length); + for (let p = 0; p < pairCount; p++) { + rows.push(pairedChangeRow(deletions[p], insertions[p])); + } + for (let p = pairCount; p < deletions.length; p++) { + rows.push(soloDeletionRow(deletions[p])); + } + for (let p = pairCount; p < insertions.length; p++) { + rows.push(soloInsertionRow(insertions[p])); + } + + // Safety: skip unknown line types so the outer loop can't stall. + if ( + i < lines.length + && lines[i].type !== 'context' + && lines[i].type !== 'delete' + && lines[i].type !== 'insert' + ) { + i++; + } + } + } + + // ── Pass 2: group consecutive changed rows into navigation blocks ── + // Hunk headers and context rows each close the currently-active block. + const changeBlocks = []; + let currentBlock = null; + + rows.forEach((row, idx) => { + const isChanged = row.leftKind === 'del' || row.rightKind === 'ins'; + + if (row.leftKind === 'hunk' || !isChanged) { + currentBlock = null; + return; + } + + if (currentBlock) { + currentBlock.endIdx = idx; + } else { + currentBlock = { startIdx: idx, endIdx: idx }; + changeBlocks.push(currentBlock); + } + }); + + return { rows, changeBlocks }; +} + +// Wrap-around modulo so Prev at block 0 jumps to the last block. JS's +// native `%` returns -1 for `-1 % 5`; the double-mod gives 4. Clamp to 0 +// when there are no blocks at all. +export function wrapIndex(idx, length) { + if (length <= 0) return 0; + return ((idx % length) + length) % length; +} diff --git a/packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/highlightCache.js b/packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/highlightCache.js new file mode 100644 index 000000000..8f8959151 --- /dev/null +++ b/packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/highlightCache.js @@ -0,0 +1,55 @@ +import { escapeHtml } from 'utils/response'; + +// Skip word-level diff on lines longer than this (Diff2Html default is 10k). +const MAX_HIGHLIGHT_LENGTH = 5000; + +export function createHighlightCache() { + // Map of `${left}\x00${right}` → { left, right } HTML. The null byte separator safely delimits the pair. + const cache = new Map(); + + return { + // Word-level diff for a paired del+ins row. Returns { left, right } HTML + // with / around changed words. + getWordDiff(leftContent, rightContent) { + const key = `${leftContent}\x00${rightContent}`; + const hit = cache.get(key); + if (hit !== undefined) return hit; // cache hit → skip the ~1-3ms recomputation + + // Diff2Html ships as a global UMD bundle loaded from /public/static. + const D2H = typeof window !== 'undefined' && window.Diff2Html; + let result; + if (D2H && typeof D2H.diffHighlight === 'function') { + try { + // diffHighlight's internal parser expects each line to start with a + // prefix char (-, +, space) and strips it. We prepend '-' / '+' here + // purely to satisfy that input shape. + const out = D2H.diffHighlight( + `-${leftContent}`, + `+${rightContent}`, + false, // isCombined: standard two-way diff, not a git combined diff + { matching: 'words', maxLineLengthHighlight: MAX_HIGHLIGHT_LENGTH } + ); + // out.oldLine/newLine.content already has the / markup we want. + result = { + left: out?.oldLine?.content ?? escapeHtml(leftContent), + right: out?.newLine?.content ?? escapeHtml(rightContent) + }; + } catch { + // Malformed input or Diff2Html internal error — fall back so the row still renders. + result = { left: escapeHtml(leftContent), right: escapeHtml(rightContent) }; + } + } else { + // Diff2Html bundle hasn't loaded (test env, CSP, etc.) — escape only. + result = { left: escapeHtml(leftContent), right: escapeHtml(rightContent) }; + } + + cache.set(key, result); // stored so Virtuoso remounts of this same row hit cache + return result; + }, + + // Empties the cache when a fresh diff replaces the current one. + clear() { + cache.clear(); + } + }; +} diff --git a/packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/index.js b/packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/index.js index bf741d364..26d614dc3 100644 --- a/packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/index.js +++ b/packages/bruno-app/src/components/OpenAPISyncTab/SpecDiffModal/index.js @@ -1,13 +1,21 @@ import { useRef, useEffect, useState } from 'react'; -import { useTheme } from 'providers/Theme/index'; -import { IconLoader2 } from '@tabler/icons'; +import { Virtuoso } from 'react-virtuoso'; +import { IconLoader2, IconChevronUp, IconChevronDown } from '@tabler/icons'; import Modal from 'components/Modal'; import StatusBadge from 'ui/StatusBadge'; +import { buildRows, wrapIndex } from './buildRows'; +import { createHighlightCache } from './highlightCache'; +import DiffRow from './DiffRow'; const SpecDiffModal = ({ specDrift, onClose }) => { - const diffRef = useRef(null); - const { displayedTheme } = useTheme(); + const virtuosoRef = useRef(null); + + const [cache] = useState(createHighlightCache); const [isRendering, setIsRendering] = useState(true); + const [parseError, setParseError] = useState(false); + const [rows, setRows] = useState([]); + const [changeBlocks, setChangeBlocks] = useState([]); + const [currentIndex, setCurrentIndex] = useState(0); const addedCount = specDrift?.added?.length || 0; const modifiedCount = specDrift?.modified?.length || 0; @@ -17,54 +25,119 @@ const SpecDiffModal = ({ specDrift, onClose }) => { ? `v${specDrift.storedVersion || '?'} → v${specDrift.newVersion}` : null; + // Parse + build row list, deferred via setTimeout so the spinner paints first. useEffect(() => { const { Diff2Html } = window; - if (!diffRef?.current || !Diff2Html || !specDrift?.unifiedDiff) { + if (!Diff2Html || !specDrift?.unifiedDiff) { setIsRendering(false); return; } setIsRendering(true); - const diffHtml = Diff2Html.html(specDrift.unifiedDiff, { - drawFileList: false, - matching: 'lines', - outputFormat: 'side-by-side', - synchronisedScroll: true, - highlight: true, - renderNothingWhenEmpty: false, - colorScheme: displayedTheme + setParseError(false); + // setTimeout yields to the browser so the spinner paints before parse blocks. + const timer = setTimeout(() => { + try { + const parsed = Diff2Html.parse(specDrift.unifiedDiff, { + outputFormat: 'side-by-side', + matching: 'lines' + }); + const built = buildRows(parsed); + setRows(built.rows); + setChangeBlocks(built.changeBlocks); + setCurrentIndex(0); + cache.clear(); + } catch (err) { + console.error('SpecDiffModal: failed to parse unified diff', err); + setParseError(true); + } + setIsRendering(false); + }, 0); + + return () => clearTimeout(timer); + }, [specDrift?.unifiedDiff, cache]); + + const goToChange = (idx) => { + if (!changeBlocks.length) return; + const nextIndex = wrapIndex(idx, changeBlocks.length); + const targetBlock = changeBlocks[nextIndex]; + const fromBlock = changeBlocks[currentIndex]; + const gap = fromBlock ? Math.abs(targetBlock.startIdx - fromBlock.startIdx) : 0; + virtuosoRef.current?.scrollToIndex({ + index: targetBlock.startIdx, + align: 'center', + behavior: gap > 500 ? 'auto' : 'smooth' }); - // Safe: Diff2Html is loaded from a local static bundle (public/static/diff2Html.js) - diffRef.current.innerHTML = diffHtml; - setIsRendering(false); - }, [displayedTheme, specDrift?.unifiedDiff]); + setCurrentIndex(nextIndex); + }; + + const activeBlock = changeBlocks[currentIndex] || null; + const renderItem = (index) => ( + = activeBlock.startIdx && index <= activeBlock.endIdx} + cache={cache} + /> + ); + + const showNav = !!specDrift?.unifiedDiff && !parseError; + const changeCount = changeBlocks.length; + const counterLabel + = changeCount === 0 ? 'No changes' : `${currentIndex + 1} of ${changeCount} changes`; return ( - +
-
- {modifiedCount > 0 && Updated: {modifiedCount}} - {addedCount > 0 && Added: {addedCount}} - {removedCount > 0 && Removed: {removedCount}} - {versionLabel && {versionLabel}} -
+
+
+
+
Endpoint Changes:
+ {modifiedCount > 0 && Updated: {modifiedCount}} + {addedCount > 0 && Added: {addedCount}} + {removedCount > 0 && Removed: {removedCount}} + {versionLabel && {versionLabel}} +
-

- {specDrift?.storedSpecMissing - ? 'The current spec file is missing. The full remote spec is shown below.' - : 'Side-by-side diff of your current spec vs the updated spec from the spec URL.'} -

+

+ {specDrift?.storedSpecMissing + ? 'The current spec file is missing. The full remote spec is shown below.' + : 'Side-by-side diff of your current spec vs the updated spec from the spec URL.'} +

+
+ {showNav && ( +
+ {counterLabel} +
+ + +
+
+ )} +
{specDrift?.unifiedDiff ? ( <>
- {specDrift?.storedSpecMissing ? 'Current Spec (missing)' : 'Current Spec'} + + {specDrift?.storedSpecMissing ? 'Current Spec (missing)' : 'Current Spec'} + Updated Spec
{isRendering && ( @@ -73,7 +146,25 @@ const SpecDiffModal = ({ specDrift, onClose }) => { Loading diff...
)} -
+ {!isRendering && parseError && ( +
+ Diff couldn't be rendered. Please file an issue with the spec. +
+ )} + {!isRendering && !parseError && rows.length > 0 && ( + + )} + {!isRendering && !parseError && rows.length === 0 && ( +
No changes to display.
+ )} ) : (
No text diff available.
diff --git a/packages/bruno-app/src/components/OpenAPISyncTab/StyledWrapper.js b/packages/bruno-app/src/components/OpenAPISyncTab/StyledWrapper.js index 45027e37f..d21e67e27 100644 --- a/packages/bruno-app/src/components/OpenAPISyncTab/StyledWrapper.js +++ b/packages/bruno-app/src/components/OpenAPISyncTab/StyledWrapper.js @@ -1503,143 +1503,154 @@ const StyledWrapper = styled.div` .text-diff-container { border-radius: ${(props) => props.theme.border.radius.sm}; border: 1px solid ${(props) => props.theme.border.border1}; - overflow: auto; + overflow: hidden; + display: flex; + flex-direction: column; + background: ${(props) => props.theme.bg}; .diff-column-headers { - display: flex; + display: grid; + grid-template-columns: 9ch 1fr 9ch 1fr; border-bottom: 1px solid ${(props) => props.theme.border.border1}; - position: sticky; - top: 0; - z-index: 2; background: ${(props) => props.theme.bg}; + flex-shrink: 0; .diff-column-label { - flex: 1; padding: 6px 12px; font-size: 12px; font-weight: 600; color: ${(props) => props.theme.colors.text.muted}; + grid-column: span 2; - &:first-child { - border-right: 1px solid ${(props) => props.theme.border.border1}; + &:last-child { + border-left: 1px solid ${(props) => props.theme.border.border1}; } } } - .d2h-wrapper { - background-color: ${(props) => props.theme.bg} !important; + /* The Virtuoso scroll container fills the rest of the modal body. */ + > div[data-testid='virtuoso-scroller'], + > div:last-child { + flex: 1 1 auto; + min-height: 0; + } + + /* Active block gets a persistent 3px yellow bar down the left edge. */ + .diff-row { + display: grid; + grid-template-columns: 9ch 1fr 9ch 1fr; font-family: 'Fira Code', monospace; font-size: 12px; + line-height: 1.5; + /* Must match Virtuoso's fixedItemHeight in SpecDiffModal/index.js */ + min-height: 18px; + color: ${(props) => props.theme.text}; + font-variant-ligatures: none; + font-feature-settings: 'liga' 0, 'calt' 0; } - .d2h-file-wrapper { - border: none; - border-radius: 0; - margin-bottom: 0; + /* Vertical divider between the two side-by-side panels. Applied to the + third grid cell (right-side line number), aligned with the header's + existing border-right on the "Current Spec" label. */ + .diff-row > *:nth-child(3) { + border-left: 1px solid ${(props) => props.theme.border.border1}; } - .d2h-file-header { - display: none; + .diff-row.diff-row-focused > .diff-cell-num:first-child { + box-shadow: inset 3px 0 0 ${(props) => props.theme.colors.text.yellow}; } - .d2h-files-diff { - width: 100%; + .diff-row.diff-row-focused > .diff-cell-num { + color: ${(props) => props.theme.text}; + font-weight: 600; + } - .d2h-file-side-diff:first-child { - border-right: 1px solid ${(props) => props.theme.border.border1}; + .diff-cell-num { + padding: 0 0.5em; + text-align: right; + color: ${(props) => props.theme.colors.text.muted}; + user-select: none; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + + &.diff-kind-del { + background-color: color-mix(in srgb, ${(props) => props.theme.colors.text.danger} 22%, transparent); + } + + &.diff-kind-ins { + background-color: color-mix(in srgb, ${(props) => props.theme.colors.text.green} 15%, transparent); + } + + &.diff-kind-empty { + background-color: ${(props) => rgba(props.theme.colors.text.muted, 0.05)}; } } - .d2h-code-side-linenumber { - background: transparent !important; - position: static !important; + .diff-cell-code { + display: flex; + min-width: 0; + padding: 0 0.5em; + white-space: pre; + overflow: hidden; + + &.diff-kind-del { + background-color: color-mix(in srgb, ${(props) => props.theme.colors.text.danger} 22%, transparent); + } + + &.diff-kind-ins { + background-color: color-mix(in srgb, ${(props) => props.theme.colors.text.green} 15%, transparent); + } + + &.diff-kind-empty { + background-color: ${(props) => rgba(props.theme.colors.text.muted, 0.05)}; + } } - .d2h-diff-tbody { - tr td { border: none !important; } + .diff-prefix { + width: 1em; + flex-shrink: 0; + color: ${(props) => props.theme.colors.text.muted}; + user-select: none; } - .d2h-ins { - background-color: color-mix(in srgb, ${(props) => props.theme.colors.text.green} 15%, transparent) !important; - border-color: color-mix(in srgb, ${(props) => props.theme.colors.text.green} 40%, transparent) !important; + .diff-content { + flex: 1 1 auto; + min-width: 0; + overflow-x: auto; + scrollbar-width: thin; + + del { + background-color: color-mix(in srgb, ${(props) => props.theme.colors.text.danger} 40%, transparent); + text-decoration: none; + } + + ins { + background-color: color-mix(in srgb, ${(props) => props.theme.colors.text.green} 40%, transparent); + text-decoration: none; + } } - .d2h-del { - background-color: color-mix(in srgb, ${(props) => props.theme.colors.text.danger} 15%, transparent) !important; - border-color: color-mix(in srgb, ${(props) => props.theme.colors.text.danger} 40%, transparent) !important; - } + /* Hunk row must be exactly 18px so Virtuoso's fixedItemHeight is + accurate. Borders would add 2px; we use inset box-shadow to get the + visual top/bottom rule without consuming layout space. Vertical + padding removed for the same reason. */ + .diff-row-hunk { + grid-template-columns: 1fr; + background-color: ${(props) => rgba(props.theme.colors.text.muted, 0.08)}; + color: ${(props) => props.theme.colors.text.muted}; + box-shadow: + inset 0 1px 0 ${(props) => props.theme.border.border1}, + inset 0 -1px 0 ${(props) => props.theme.border.border1}; - .d2h-file-diff .d2h-ins.d2h-change { - background-color: color-mix(in srgb, ${(props) => props.theme.colors.text.green} 25%, transparent) !important; - } - - .d2h-file-diff .d2h-del.d2h-change { - background-color: color-mix(in srgb, ${(props) => props.theme.colors.text.warning} 20%, transparent) !important; - } - - .d2h-code-line ins, - .d2h-code-side-line ins { - background-color: color-mix(in srgb, ${(props) => props.theme.colors.text.green} 40%, transparent) !important; - text-decoration: none; - } - - .d2h-code-line del, - .d2h-code-side-line del { - background-color: color-mix(in srgb, ${(props) => props.theme.colors.text.danger} 40%, transparent) !important; - text-decoration: none; - } - - .d2h-code-line, - .d2h-code-side-line { - color: ${(props) => props.theme.text} !important; - word-break: break-all; - } - - .d2h-code-line-ctn { - word-break: break-all; - } - - .d2h-tag { - font-size: 9px; - font-weight: 500; - padding: 1px 5px; - border-radius: ${(props) => props.theme.border.radius.sm}; - text-transform: uppercase; - letter-spacing: 0.02em; - border: none; - } - - .d2h-changed-tag { - background-color: color-mix(in srgb, ${(props) => props.theme.colors.text.warning} 15%, transparent); - color: ${(props) => props.theme.colors.text.warning}; - } - - .d2h-added-tag { - background-color: color-mix(in srgb, ${(props) => props.theme.colors.text.green} 13%, transparent); - color: ${(props) => props.theme.colors.text.green}; - } - - .d2h-deleted-tag { - background-color: color-mix(in srgb, ${(props) => props.theme.colors.text.danger} 13%, transparent); - color: ${(props) => props.theme.colors.text.danger}; - } - - .d2h-renamed-tag, - .d2h-moved-tag { - display: none; - } - - .d2h-file-wrapper, - .d2h-file-diff, - .d2h-code-wrapper, - .d2h-diff-table, - .d2h-code-line, - .d2h-code-side-line, - .d2h-code-line-ctn, - .d2h-code-linenumber, - .d2h-code-side-linenumber { - font-family: 'Fira Code', monospace !important; - font-size: 12px !important; + .diff-cell-hunk { + padding: 0 0.75em; + font-family: 'Fira Code', monospace; + font-size: 11px; + white-space: pre; + overflow: hidden; + text-overflow: ellipsis; + } } } @@ -1661,6 +1672,15 @@ const StyledWrapper = styled.div` } .spec-diff-modal { + + .spec-diff-header { + display: flex; + align-items: flex-end; + justify-content: space-between; + gap: 0.5rem; + margin-bottom: 1rem; + } + .spec-diff-badges { display: flex; gap: 0.5rem; @@ -1671,12 +1691,50 @@ const StyledWrapper = styled.div` .spec-diff-subtitle { font-size: ${(props) => props.theme.font.size.sm}; color: ${(props) => props.theme.colors.text.muted}; - margin: 0 0 0.75rem 0; + } + + .spec-diff-nav { + display: flex; + align-items: center; + justify-content: space-between; + gap: 1rem; + + .spec-diff-nav-counter { + font-size: ${(props) => props.theme.font.size.sm}; + color: ${(props) => props.theme.colors.text.muted}; + } + + .spec-diff-nav-buttons { + display: flex; + gap: 0.5rem; + } + + .spec-diff-nav-btn { + display: inline-flex; + align-items: center; + gap: 0.25rem; + padding: 0.25rem 0.5rem; + font-size: ${(props) => props.theme.font.size.xs}; + background: none; + border: 1px solid ${(props) => props.theme.border.border1}; + border-radius: ${(props) => props.theme.border.radius.sm}; + color: ${(props) => props.theme.text}; + cursor: pointer; + + &:hover:not(:disabled) { + background: ${(props) => props.theme.background.surface1}; + } + + &:disabled { + opacity: 0.5; + cursor: not-allowed; + } + } } .spec-diff-body { .text-diff-container { - max-height: calc(80vh - 140px); + height: calc(80vh - 140px); } } } diff --git a/packages/bruno-app/src/components/OpenAPISyncTab/SyncReviewPage/index.js b/packages/bruno-app/src/components/OpenAPISyncTab/SyncReviewPage/index.js index 07d25b8c9..5b2e34053 100644 --- a/packages/bruno-app/src/components/OpenAPISyncTab/SyncReviewPage/index.js +++ b/packages/bruno-app/src/components/OpenAPISyncTab/SyncReviewPage/index.js @@ -90,6 +90,17 @@ const SyncReviewPage = ({ const tabUiState = useSelector((state) => state.openapiSync?.tabUiState?.[collectionUid] || {}); const [showConfirmation, setShowConfirmation] = useState(false); const [showSpecDiffModal, setShowSpecDiffModal] = useState(false); + const [isOpeningSpecDiff, setIsOpeningSpecDiff] = useState(false); + + // setTimeout lets the button's spinner paint before the modal mounts — + // without it, React batches both state updates and the spinner never shows. + const handleOpenSpecDiff = () => { + setIsOpeningSpecDiff(true); + setTimeout(() => { + setShowSpecDiffModal(true); + setIsOpeningSpecDiff(false); + }, 0); + }; const { specAddedEndpoints, specUpdatedEndpoints, localUpdatedEndpoints, specRemovedEndpoints } = useMemo(() => { if (!remoteDrift) { @@ -228,8 +239,17 @@ const SyncReviewPage = ({ {(specDrift?.unifiedDiff || decidableEndpoints.length > 0) && (
{specDrift?.unifiedDiff && ( - )} {decidableEndpoints.length > 0 && (