Skip to content

Commit 18589e7

Browse files
balzssclaude
andcommitted
feat(ui-scripts): improve the visual-diff report UX and diff rendering
Land three report improvements: - Default the status filter to "Changed" (falling back to "All" when nothing changed) so reviewers see regressions first, and apply filters on load. - Close the lightbox on Escape and navigate with the arrow keys. - Rework the diff image to a Chromatic-style pixel highlight: dim and desaturate the actual screenshot into a faint backdrop and paint the exact changed pixels (dilated for visibility) in the accent color, replacing the coarse bounding boxes. Extracts a tested dilateMask helper. Also add a TEMP diff-demo fixture and a workflow step that injects it so the published report always has one "changed" row to preview the new rendering (this PR's -<theme> screenshot renames leave zero real "changed" rows). Both are marked for removal before merge. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent e1dcfd4 commit 18589e7

6 files changed

Lines changed: 176 additions & 167 deletions

File tree

.github/workflows/visual-regression.yml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,19 @@ jobs:
7777
mkdir -p regression-test/.actual
7878
find regression-test/cypress/screenshots -name '*.png' -exec cp {} regression-test/.actual/ \;
7979
80+
# TEMP (remove before merge): this PR renames every screenshot with a
81+
# `-<theme>` suffix, so nothing matches the old baselines and the report
82+
# has zero "changed" rows — leaving the new diff visualization impossible
83+
# to preview. Inject one synthetic baseline/actual pair that differs in a
84+
# few localized spots so the report always has a "changed" example.
85+
# Fixtures live in regression-test/cypress/diff-demo/. Delete this step
86+
# and that directory to revert.
87+
- name: TEMP inject diff-demo fixture
88+
run: |
89+
mkdir -p regression-test/.baselines regression-test/.actual
90+
cp regression-test/cypress/diff-demo/baseline.png regression-test/.baselines/diff-demo.png
91+
cp regression-test/cypress/diff-demo/actual.png regression-test/.actual/diff-demo.png
92+
8093
- name: Diff and generate report
8194
id: diff
8295
working-directory: regression-test

packages/ui-scripts/lib/__node_tests__/visual-diff.test.ts

Lines changed: 39 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import {
2929
indexByName,
3030
sourceLinkFor,
3131
appUrlFor,
32-
boxesFromMask
32+
dilateMask
3333
} from '../commands/visual-diff.ts'
3434

3535
// Build a w*h changed-mask with the given filled rectangles set to 1.
@@ -175,7 +175,10 @@ describe('sourceLinkFor', () => {
175175

176176
describe('appUrlFor', () => {
177177
const facets = ['canvas', 'light', 'dark']
178-
const meta = { 'button-dark': '/button', 'small-components-light': '/small-components' }
178+
const meta = {
179+
'button-dark': '/button',
180+
'small-components-light': '/small-components'
181+
}
179182

180183
it('returns an empty string when appPath is not provided', () => {
181184
expect(appUrlFor('button-dark.png', meta, facets)).toBe('')
@@ -212,54 +215,47 @@ describe('appUrlFor', () => {
212215
})
213216
})
214217

215-
describe('boxesFromMask', () => {
216-
it('returns no boxes for an unchanged mask', () => {
217-
expect(boxesFromMask(mask(32, 32, []), 32, 32)).toEqual([])
218-
})
218+
describe('dilateMask', () => {
219+
const countSet = (m: ArrayLike<number>) => {
220+
let n = 0
221+
for (let i = 0; i < m.length; i++) if (m[i]) n++
222+
return n
223+
}
219224

220-
it('wraps a single changed cluster in one box that contains it', () => {
221-
const boxes = boxesFromMask(
222-
mask(32, 32, [{ x: 4, y: 4, w: 10, h: 10 }]),
223-
32,
224-
32
225-
)
226-
expect(boxes).toHaveLength(1)
227-
const [b] = boxes
228-
// the box must fully contain the changed region
229-
expect(b.x).toBeLessThanOrEqual(4)
230-
expect(b.y).toBeLessThanOrEqual(4)
231-
expect(b.x + b.w).toBeGreaterThanOrEqual(14)
232-
expect(b.y + b.h).toBeGreaterThanOrEqual(14)
225+
it('leaves an empty mask empty', () => {
226+
expect(countSet(dilateMask(mask(16, 16, []), 16, 16))).toBe(0)
233227
})
234228

235-
it('separates two distant clusters into two boxes', () => {
236-
const boxes = boxesFromMask(
237-
mask(64, 64, [
238-
{ x: 0, y: 0, w: 6, h: 6 },
239-
{ x: 50, y: 50, w: 6, h: 6 }
240-
]),
241-
64,
242-
64
243-
)
244-
expect(boxes).toHaveLength(2)
229+
it('grows a single pixel into its 3x3 neighborhood by default', () => {
230+
const out = dilateMask(mask(16, 16, [{ x: 8, y: 8, w: 1, h: 1 }]), 16, 16)
231+
expect(countSet(out)).toBe(9)
232+
// the neighbors around the original pixel are all set
233+
for (let dy = -1; dy <= 1; dy++) {
234+
for (let dx = -1; dx <= 1; dx++) {
235+
expect(out[(8 + dy) * 16 + (8 + dx)]).toBe(1)
236+
}
237+
}
245238
})
246239

247-
it('drops specks smaller than minPixels', () => {
248-
// a single changed pixel is below the default minPixels threshold
249-
expect(
250-
boxesFromMask(mask(32, 32, [{ x: 5, y: 5, w: 1, h: 1 }]), 32, 32)
251-
).toEqual([])
240+
it('clamps the dilation at the image edges', () => {
241+
// a corner pixel only has 3 in-bounds neighbors besides itself
242+
const out = dilateMask(mask(16, 16, [{ x: 0, y: 0, w: 1, h: 1 }]), 16, 16)
243+
expect(countSet(out)).toBe(4)
252244
})
253245

254-
it('never lets a box exceed the image bounds', () => {
255-
const boxes = boxesFromMask(
256-
mask(20, 20, [{ x: 0, y: 0, w: 20, h: 20 }]),
257-
20,
258-
20
246+
it('honors a custom radius', () => {
247+
const out = dilateMask(
248+
mask(16, 16, [{ x: 8, y: 8, w: 1, h: 1 }]),
249+
16,
250+
16,
251+
2
259252
)
260-
expect(boxes).toHaveLength(1)
261-
const [b] = boxes
262-
expect(b.x + b.w).toBeLessThanOrEqual(20)
263-
expect(b.y + b.h).toBeLessThanOrEqual(20)
253+
expect(countSet(out)).toBe(25)
254+
})
255+
256+
it('passes the mask through unchanged for radius 0', () => {
257+
const src = mask(16, 16, [{ x: 3, y: 3, w: 2, h: 2 }])
258+
const out = dilateMask(src, 16, 16, 0)
259+
expect(countSet(out)).toBe(countSet(src))
264260
})
265261
})

0 commit comments

Comments
 (0)