diff --git a/cli/src/diff.ts b/cli/src/diff.ts index 2d32145..a54c1d5 100644 --- a/cli/src/diff.ts +++ b/cli/src/diff.ts @@ -1,5 +1,6 @@ import type { RawImage } from "selfie-store"; import type { BoundingBox, ChangedHunk, PixelDiff, HunkKind } from "selfie-ui/app"; +import { fade, seamOf } from "selfie-ui/diff-image"; import { explainHunks, forEachUnexplained } from "./explain.ts"; import { encodePng } from "./png.ts"; import { findHunks } from "./hunks.ts"; @@ -9,18 +10,6 @@ export type { BoundingBox, PixelDiff } from "selfie-ui/app"; type Color = readonly [number, number, number]; const COLORS: Readonly> = { changed: [255, 0, 80], moved: [9, 105, 218], spacing: [255, 214, 102] }; -// An unchanged pixel fades into gray from this level to white, so the page stays readable -// behind the highlight and no color in it can be mistaken for one. -const FADED_FLOOR = 190; - -function fade(from: Uint8Array, out: Uint8Array, at: number): void { - const luma = (from[at]! * 77 + from[at + 1]! * 150 + from[at + 2]! * 29) >> 8; - const faded = FADED_FLOOR + ((luma * (255 - FADED_FLOOR)) >> 8); - out[at] = faded; - out[at + 1] = faded; - out[at + 2] = faded; -} - function paint(out: Uint8Array, at: number, [red, green, blue]: Color): void { out[at] = red; out[at + 1] = green; @@ -94,12 +83,10 @@ export function diffImages(current: RawImage, golden: RawImage): { readonly diff changedPixels += explanation.changedPixels; if (explanation.changedPixels === goldenOnly && moved.length === 0 && (kind !== "spacing" || goldenSpacingRows > 0)) { // Nothing here is different, since something was only taken away, so mark the seam it left. - const seam = Math.min(here.y, height - 1); - const from = here.width === 0 ? 0 : here.x; - const to = here.width === 0 ? width : here.x + here.width; - for (let x = from; x < to; x++) { - if (kind === "changed") mark(x, seam); - paint(out, (seam * width + x) * 3, COLORS[kind]); + const seam = seamOf(here, current); + for (let x = seam.x; x < seam.x + seam.width; x++) { + if (kind === "changed") mark(x, seam.y); + paint(out, (seam.y * width + x) * 3, COLORS[kind]); } } return { ...hunk, kind, changedPixels: explanation.changedPixels, moved }; diff --git a/cli/test/deltas.test.ts b/cli/test/deltas.test.ts new file mode 100644 index 0000000..6b24588 --- /dev/null +++ b/cli/test/deltas.test.ts @@ -0,0 +1,88 @@ +// What the page relies on: the differ's hunks account for everything, so redrawing them onto the +// golden gives exactly what's left, with no need to compare any pixels again. +import type { RawImage } from "selfie-store"; +import { canApply, patchGolden } from "selfie-ui/deltas"; +import { describe, expect, test } from "vitest"; +import { diffImages } from "../src/diff.ts"; + +const WIDTH = 120; + +/** Every row its own, as with real text, so rows line up across images. */ +function page(rows: readonly (readonly [number, number, number])[]): RawImage { + const pixels = new Uint8Array(WIDTH * rows.length * 3); + rows.forEach((row, y) => { + for (let x = 0; x < WIDTH; x++) pixels.set([(row[0] + x * (y % 5)) & 255, row[1], row[2]], (y * WIDTH + x) * 3); + }); + return { width: WIDTH, height: rows.length, pixels }; +} + +const rowsOf = (from: number, count: number) => Array.from({ length: count }, (_, at): readonly [number, number, number] => [(from + at) & 255, ((from + at) * 7) & 255, ((from + at) >> 8) & 255]); + +const blank = (count: number) => Array.from({ length: count }, (): readonly [number, number, number] => [255, 255, 255]); + +function redrawn(current: RawImage, golden: RawImage, accepted: readonly number[]) { + const { diff } = diffImages(current, golden); + const patch = patchGolden(diff, accepted); + const { width, height } = patch.diff.golden; + const pixels = new Uint8Array(width * height * 3); + for (const { from, box, toX, toY } of patch.blits) { + const source = from === "golden" ? golden : current; + for (let row = 0; row < box.height; row++) { + const start = ((box.y + row) * source.width + box.x) * 3; + pixels.set(source.pixels.subarray(start, start + box.width * 3), ((toY + row) * width + toX) * 3); + } + } + return { diff, patch, image: { width, height, pixels } satisfies RawImage }; +} + +/** A header that grows, a change further down, and a band below that closes up, as one real change did. */ +const golden = page([...rowsOf(0, 8), ...rowsOf(1000, 20), ...rowsOf(100, 60), ...rowsOf(200, 4), ...rowsOf(300, 60), ...blank(15), ...rowsOf(400, 40)]); +const current = page([...rowsOf(0, 8), ...rowsOf(2000, 35), ...rowsOf(100, 60), ...rowsOf(600, 4), ...rowsOf(300, 60), ...rowsOf(400, 40)]); + +function narrowed(image: RawImage): RawImage { + const pixels = image.pixels.slice(); + for (let y = 30; y < 34; y++) for (let x = 40; x < 44; x++) pixels.set([1, 2, 3], (y * image.width + x) * 3); + return { ...image, pixels }; +} + +describe("redrawing hunks onto the golden", () => { + test("every hunk here can be applied, and there's more than one", () => { + const { diff } = diffImages(current, golden); + expect(diff.hunks.length).toBeGreaterThan(1); + expect(diff.hunks.every((hunk) => canApply(hunk, diff))).toBe(true); + }); + + test("all of them give the current image, pixel for pixel", () => { + const hunks = diffImages(current, golden).diff.hunks.map((_, index) => index); + const { image, patch } = redrawn(current, golden, hunks); + expect(patch.diff.hunks).toEqual([]); + expect({ width: image.width, height: image.height }).toEqual({ width: current.width, height: current.height }); + expect(Buffer.compare(image.pixels, current.pixels)).toBe(0); + }); + + test("any of them, in any combination, leave exactly the others to find", () => { + const { hunks } = diffImages(current, golden).diff; + for (let mask = 0; mask < 1 << hunks.length; mask++) { + const accepted = hunks.flatMap((_, index) => ((mask >> index) & 1 ? [index] : [])); + const { image, patch } = redrawn(current, golden, accepted); + const again = diffImages(current, image).diff; + expect(again.hunks.map(({ current: box }) => box)).toEqual(patch.diff.hunks.map(({ current: box }) => box)); + expect(again.hunks.map(({ golden: box }) => box)).toEqual(patch.diff.hunks.map(({ golden: box }) => box)); + } + }); + + test("a narrow hunk of one size goes where the golden has it, below one that grew", () => { + const changed = narrowed(current); + const { hunks } = diffImages(changed, golden).diff; + const { image } = redrawn(changed, golden, hunks.map((_, index) => index)); + expect(Buffer.compare(image.pixels, changed.pixels)).toBe(0); + }); + + test("a change in width replaces the whole image", () => { + const wider = { width: 100, height: 4, pixels: new Uint8Array(100 * 4 * 3).fill(9) }; + const narrower = { width: 80, height: 4, pixels: new Uint8Array(80 * 4 * 3).fill(200) }; + const { image } = redrawn(narrower, wider, [0]); + expect(Buffer.compare(image.pixels, narrower.pixels)).toBe(0); + expect(image.width).toBe(80); + }); +}); diff --git a/cli/test/ui-access.test.ts b/cli/test/ui-access.test.ts index 6e16892..59cba52 100644 --- a/cli/test/ui-access.test.ts +++ b/cli/test/ui-access.test.ts @@ -84,7 +84,7 @@ describe("who may ask", () => { const refused = await get("/api/stories", { host: "evil.example" }); for (const response of [await get("/"), await get("/api/stories"), refused]) { expect(response.headers.get("content-security-policy")).toBe( - "default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; frame-ancestors 'none'; base-uri 'none'; form-action 'none'", + "default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data: blob:; frame-ancestors 'none'; base-uri 'none'; form-action 'none'", ); expect(response.headers.get("x-frame-options")).toBe("DENY"); expect(response.headers.get("cross-origin-resource-policy")).toBe("same-origin"); diff --git a/docs/specs/access.md b/docs/specs/access.md index 5cc6ff1..3213d3a 100644 --- a/docs/specs/access.md +++ b/docs/specs/access.md @@ -14,7 +14,7 @@ There's no login on this server, since anyone who can reach loopback on the mach - **Must listen only on loopback and check the `Host` header**, so a website the user visits can't read their renders through it, by pointing its own DNS name at `127.0.0.1` for one (rationale). The header must be `127.0.0.1:` or `localhost:`, with the port it is listening on, and anything else gets 403. - **Must refuse a request whose `Origin` isn't this server's**, whatever its method. - **Must require `Content-Type: application/json` and the header `X-Selfie-Request: review-ui` on anything but GET and HEAD**, so a form or a page on another site can't approve anything, and must send no CORS headers, so a page's own attempt fails its preflight (rationale). -- **Must refuse to be framed and to be embedded**, with `frame-ancestors 'none'`, `X-Frame-Options: DENY`, and `Cross-Origin-Resource-Policy: same-origin`, so another site can't steer a click on Approve (rationale), on every response, refusals included. `Content-Security-Policy` is exactly `default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:; frame-ancestors 'none'; base-uri 'none'; form-action 'none'`, with `frame-src` naming the hosted Storybook's origin if there is one, through `hono/secure-headers`. +- **Must refuse to be framed and to be embedded**, with `frame-ancestors 'none'`, `X-Frame-Options: DENY`, and `Cross-Origin-Resource-Policy: same-origin`, so another site can't steer a click on Approve (rationale), on every response, refusals included. `Content-Security-Policy` is exactly `default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data: blob:; frame-ancestors 'none'; base-uri 'none'; form-action 'none'`, with `frame-src` naming the hosted Storybook's origin if there is one, through `hono/secure-headers`. - **Must host a Storybook on a loopback port of its own, read only, never on the review server's origin**, since its code is whatever branch was built, and on that origin it could approve its own images (rationale). The `Host` check applies to it too. It may be framed only by itself, since Storybook frames its own preview, and by the review page, and it serves nothing from a directory with no `iframe.html`. Kept built Storybooks share that origin (`docs/specs/ui.md` → Routes). - **Must let the review page frame only the hosted Storybook's origin, and sandbox every frame, without `allow-top-navigation`**, so a story can't steer the review page, and can't read it, being another origin (rationale). - **Must serve an image only through the app, by a validated hash, with `Cache-Control: private`**, and never name a file path in a response. Reserved: [Serving images](#serving-images) holds the same for the cloud. diff --git a/docs/specs/glossary.md b/docs/specs/glossary.md index bc6a538..8bdc136 100644 --- a/docs/specs/glossary.md +++ b/docs/specs/glossary.md @@ -11,6 +11,7 @@ | **Environment** | What decides the pixels apart from the project's own input: the runner, its architecture, the browser and its flags, the viewport and scale, and the render recipe (`docs/specs/system.md`). Its ID is part of a render's name. | | **Render batch** | The record of the process that rendered a set of images, against a Git sha. `batch` for short. | | **Reference** | What a story is compared with at a commit: the image of the newest approval that commit's history reaches (`docs/specs/renders.md` → Approvals). A story with none has no reference. Code and the page still say "golden" for it, as in `goldenHash`, until that's renamed. | +| **Delta** | One change repeated across stories: hunks whose two sides are the same size and hold the same pixels (`docs/specs/ui.md` → Screens). | | **Baseline** | What `selfie review --baseline` compares with, for one review: a batch, or a commit's history. | | **Note** | What a person says about a story, for an agent to act on: on an image, and maybe a box of it (`docs/specs/feedback.md`). | | **Session** | Whose notes these are: a local Git worktree's top folder (`docs/specs/feedback.md`). | diff --git a/docs/specs/ui.md b/docs/specs/ui.md index 8d135de..04ca052 100644 --- a/docs/specs/ui.md +++ b/docs/specs/ui.md @@ -13,6 +13,8 @@ Source of truth: `ui/`. |---|---| | `ui/src/shared/api.ts` | The JSON on the wire, and `ReviewSource` | | `ui/src/shared/hunks.ts`, exported as `selfie-ui/hunks` | Grouping a diff's hunks for showing, for the client and the CLI's document | +| `ui/src/shared/diff-image.ts`, exported as `selfie-ui/diff-image` | How a diff image draws what didn't change and where something was taken away, for the differ and a page redrawing one | +| `ui/src/shared/deltas.ts`, exported as `selfie-ui/deltas` | What a [delta](#screens) is, which to offer, and what redrawing one leaves; tests in `ui/test/deltas.test.ts`, and in `cli/test/deltas.test.ts` against the real differ | | `ui/src/shared/notes.ts`, exported as `selfie-ui/notes` | Shared note validation and open-note state (`docs/specs/feedback.md`) | | `ui/src/shared/renderProgress.ts`, exported as `selfie-ui/render-progress` | A render's estimate and how it's said, for the client and `selfie flakes`; tests in `ui/test/renderProgress.test.ts` | | `selfie-ui/sync`, `selfie-ui/sync-api` | `docs/specs/sync.md` → Routes | @@ -126,7 +128,7 @@ Source of truth: `ui/src/client/`. - **Must say a story is renamed if it's added and a removed story's reference is its very image**, one to one, and the closest names first where several share an image (rationale). The list has it once, under its new name. This is only to look at: to decisions they're still one added and one removed. -**The batch** shows, in order: its commit, batch ID, whether its pixels were rendered earlier and by which batch, dirty state, environment, and decision actor; a warning if it was recorded dirty; the counts; blockers; and the differences. +**The batch** shows, in order: any [repeated changes](#screens); its commit, batch ID, whether its pixels were rendered earlier and by which batch, dirty state, environment, and decision actor; a warning if it was recorded dirty; the counts; blockers; and the differences. - **Must group blockers by reason, in words, and offer each as a filter of the list.** - **Must keep the filter, text and state, in the URL, and nowhere else**, so back, reload, and next and previous all follow it. @@ -137,6 +139,20 @@ Source of truth: `ui/src/client/`. - **Must confirm every bulk decision in a dialog, with the count, how many are changed and added, how many are already decided, and which stories.** Focus starts on Cancel. - **Must hold the rows in the dialog from when it opens, and send those**, so what it listed is what's sent, whatever a filter or a read does meanwhile. +**Repeated changes:** a delta is one change repeated across stories (`docs/specs/glossary.md`). Source of truth: `ui/src/shared/deltas.ts`, `ui/src/client/deltas.ts`, `ui/src/client/DeltaOffers.tsx`. + +- **Must keep all of this in the browser.** It adds no route and no table, and sends only approvals, through the same route any decision takes. Which deltas are accepted is held in `localStorage`, by batch, so a reload keeps them, and another browser, batch, or person doesn't have them (rationale). +- **Must look for deltas only for someone who [may decide](#screens)**, since nothing else can come of it, and looking decodes every changed image. +- **Must find a delta from a pair's own diff, fetched by both hashes, and never a review's stats** ([Screens](#screens)), since those are by story, and could be of another pair. +- **Must group raw hunks, not the groups a list shows, by the size of each side, and then by the SHA-256 of each side's pixels**, wherever in an image a hunk sits (rationale). It decodes only images with a size at least 3 stories share. +- **Must only group a hunk that can be redrawn on its own:** one with the same size on both sides, or one the whole width of both images. A narrower hunk that changes height only balances out with others in its column, so it stays a hunk to look at. +- **Must offer each delta at least 3 undecided changed stories have**, most stories first, once all of them have been looked through, each with how many stories and a crop of it before and after. An image with a decision isn't affected. +- **Must let someone go through a delta's stories before accepting it**, one delta at a time, starting with the first and moving on once it's accepted: previous and next through its stories, wrapping around, naming each with a link to it, and showing that story's crop and all of its image on each side as it is on record, with the delta outlined where that story has it (rationale). +- **Must say what's accepted, and let each be undone**, which brings its hunks back and takes back no approval. +- **Must approve an image once every one of its hunks is an accepted delta, and only an undecided one**, so a rejection is never overturned, and confirm that as any [bulk decision](#screens) is, with accepting nothing if it's cancelled. A delta that finishes no image needs no confirming, since it decides nothing. +- **Must redraw only on a batch's own pages.** One render's golden and diff are drawn from a pair's images and the server's diff image, with its pixel stats, hunk list, and outlines all from the hunks left, and it says that it's redrawn, that the golden on record hasn't changed, and offers that golden instead (rationale). +- **Must keep what's on the golden to a note on all of it while it's redrawn**, since a box on it is in pixels the golden on record doesn't have in that place. Its existing boxes aren't shown then. + **One render** shows its state and decision, pixel stats, and images. - **Must offer the images side by side and stacked whenever a story has more than one to look at**, as an added story does with a note on an older image. Stacked, switching layers costs no scroll, and the zoom starts at actual size: the image's width divided by `batch.scale`, in CSS pixels, so it's the size the page really was on every display. diff --git a/docs/specs/ui.rationale.md b/docs/specs/ui.rationale.md index bda955f..d379e6a 100644 --- a/docs/specs/ui.rationale.md +++ b/docs/specs/ui.rationale.md @@ -28,6 +28,14 @@ Evidence and reasoning for `docs/specs/ui.md`, keyed by its headings. ## Screens +**Repeated changes, in the browser.** A header that grew 15 rows made all 124 stories of one batch change, and each had to be looked at for whatever else was in it. Of 146 distinct hunks there, 28 were in at least 3 stories, and those accounted for all of 83 images. Whether accepting one should outlive a batch, or be something a team shares, is what **Scope: hunks** in `docs/specs/renders.md` is for. Nothing here narrows that, since a store only ever hears of an approval of a whole image. + +**Why raw hunks, and by pixels.** A group is only a choice of what to show together, and its box holds whatever's between its hunks, so the same change could group differently beside something else. Position isn't compared, since a band that closes up sits at a different height on each page. A hash and not pixels held in memory, since a whole page's change on each of 568 stories would hold gigabytes. SHA-256, since this approves images, and a weaker hash would let a crafted image pass as a change someone accepted. + +**Why whole images to go through a delta's stories.** A delta's pixels are the same in every story by definition, so its crop can't tell one from another. What can be wrong is where it landed and what's around it, as when a change that's right on most pages is on top of something on one. Only one is open at a time, since 28 pairs of whole images would be a very long page. + +**Why it needs no second comparison.** Everything outside a hunk matches, which is what makes hunks correct at all, so with some redrawn, what's different is exactly the rest. `cli/test/deltas.test.ts` shows it on the real differ, by comparing again for every combination. It was also compared pixel by pixel in a browser, on all 83 of that batch's finished images, through canvas and PNG. + - **Why moves and spacing are a filter, not a pass.** On one real layout shift, 59 of 134 stories had nothing else in them. Looking through them one at a time is the cost the label should save, and `kind` must not decide for anyone (`cli.rationale.md`), so the filter lets a person see just those, glance over the thumbnails, and approve what's shown. - **Why a list even for one hunk.** A whole-image approval is the case of one hunk, so the screen shouldn't have two shapes, and a person learns one way of looking. - **Why a flip holds the hunk still.** Once something above a hunk has changed height, the hunk is at a different place in each image, and a flip shows the page jump and nothing about the hunk. The 9-slice already knows which boxes correspond, so the scroll follows them. It's only while looking at a hunk, since flipping with none costs no scroll. diff --git a/ui/package.json b/ui/package.json index 92f80bc..741dcd3 100644 --- a/ui/package.json +++ b/ui/package.json @@ -7,6 +7,8 @@ "exports": { "./api": "./src/shared/api.ts", "./batch-view": "./src/server/batch-view.ts", + "./deltas": "./src/shared/deltas.ts", + "./diff-image": "./src/shared/diff-image.ts", "./app": "./src/server/app.ts", "./hunks": "./src/shared/hunks.ts", "./notes": "./src/shared/notes.ts", diff --git a/ui/src/client/App.tsx b/ui/src/client/App.tsx index f2354f2..4948065 100644 --- a/ui/src/client/App.tsx +++ b/ui/src/client/App.tsx @@ -2,8 +2,10 @@ import { Heading } from "@primer/react"; import logo from "../../../branding/logo-icon.svg"; import { useLayoutEffect, useRef, type ReactNode } from "react"; import { fetchBatchContext } from "./api.ts"; +import { canDecide } from "./base.ts"; import { DetailScreen } from "./DetailScreen.tsx"; import { FeedbackBar, NewerBatchBanner } from "./Feedback.tsx"; +import { useDeltas } from "./deltas.ts"; import { useFeedback, useFetched, useBatch } from "./hooks.ts"; import { StoriesRoutes } from "./StoriesScreen.tsx"; import { AppLink, parseRoute, storyPath, useLocation, type Route, storiesPath } from "./router.tsx"; @@ -18,6 +20,7 @@ function BatchRoutes({ batchId, route, search }: { batchId: string; route: Route const feedback = useFeedback(); const { data: context } = useFetched(batchId, (signal) => fetchBatchContext(batchId, signal)); const tree = useBatchTree(state.view, state.unchanged, search, route.kind === "story" ? route.story : null); + const deltas = useDeltas(batchId, state.view?.rows, route.kind === "batch" && state.view !== null && canDecide(state.view)); // Where to put focus on coming back to the list. const lastStory = useRef(null); if (state.view === null) { @@ -32,9 +35,9 @@ function BatchRoutes({ batchId, route, search }: { batchId: string; route: Route )} storyPath(batchId, story, "")} showList={route.kind === "batch"} /> {route.kind === "story" && tree.navigation !== null ? ( - + ) : ( - + )} ); diff --git a/ui/src/client/BatchScreen.stories.tsx b/ui/src/client/BatchScreen.stories.tsx index 9fcb071..22e5670 100644 --- a/ui/src/client/BatchScreen.stories.tsx +++ b/ui/src/client/BatchScreen.stories.tsx @@ -1,24 +1,26 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; import { useRef } from "react"; -import { userEvent, within } from "storybook/test"; +import { expect, userEvent, within } from "storybook/test"; import type { BatchView } from "../shared/api.ts"; import type { BatchState } from "./hooks.ts"; import { BatchFrame } from "./stories/BatchFrame.tsx"; import { BatchScreen } from "./BatchScreen.tsx"; -import { approved, failure, OTHER_BATCH_ID, rejected, rows, batchOf, stateOf, viewOf } from "./stories/fixtures.ts"; +import type { DeltaState } from "./deltas.ts"; +import { approved, deltaFrom, deltasOf, failure, OTHER_BATCH_ID, rejected, rows, batchOf, stateOf, viewOf } from "./stories/fixtures.ts"; interface Args { readonly view: BatchView; readonly state?: Partial; readonly search?: string; + readonly deltas?: DeltaState; } -function Screen({ view, state, search = "" }: Args) { +function Screen({ view, state, search = "", deltas = deltasOf() }: Args) { const returningFrom = useRef(null); const shown = stateOf(view, state); return ( - {() => } + {() => } ); } @@ -116,3 +118,29 @@ export const ConfirmApproveAll: Story = { await userEvent.click(within(canvasElement).getByRole("button", { name: /^Approve all/ })); }, }; + +const AFFECTED = ["components-button--primary", "pages-settings--account", "components-list--with-new-row"]; +const OFFERS = [deltaFrom("taller-golden", [...AFFECTED, "pages-home--rearranged"]), deltaFrom("changed-golden", AFFECTED), deltaFrom("spaced-golden", AFFECTED)]; +export const LookingForRepeatedChanges: Story = { args: { view: viewOf(SOME_CHANGES), deltas: deltasOf({ progress: 40 }) } }; +export const RepeatedChanges: Story = { args: { view: viewOf(SOME_CHANGES), deltas: deltasOf({ offers: OFFERS }) } }; +export const LookingThroughADelta: Story = { + args: { view: viewOf(SOME_CHANGES), deltas: deltasOf({ offers: OFFERS }) }, + play: async ({ canvasElement }) => { + await userEvent.click(within(canvasElement).getAllByRole("button", { name: "Look through 3 snapshots" })[0]!); + await userEvent.click(within(canvasElement).getByRole("button", { name: "Next" })); + await expect(within(canvasElement).getByText("2 of 3")).toBeVisible(); + }, +}; +export const ConfirmAcceptingADelta: Story = { + args: { view: viewOf(SOME_CHANGES), deltas: deltasOf({ offers: OFFERS, finishes: () => [rows.changed(AFFECTED[0]!), rows.changed(AFFECTED[1]!)] }) }, + play: async ({ canvasElement }) => { + const [first] = within(canvasElement).getAllByRole("button", { name: "Accept this delta to all affected snapshots" }); + await userEvent.click(first!); + }, +}; +export const AcceptedDeltas: Story = { + args: { view: viewOf(SOME_CHANGES), deltas: deltasOf({ offers: OFFERS.slice(1), taken: OFFERS.slice(0, 1) }) }, + play: async ({ canvasElement }) => { + await userEvent.click(within(canvasElement).getByText(/1 delta accepted in this browser/)); + }, +}; diff --git a/ui/src/client/BatchScreen.tsx b/ui/src/client/BatchScreen.tsx index 1896676..f8b022b 100644 --- a/ui/src/client/BatchScreen.tsx +++ b/ui/src/client/BatchScreen.tsx @@ -1,9 +1,8 @@ -import { isHosted, signInHref } from "./base.ts"; +import { canDecide, isHosted, signInHref } from "./base.ts"; // The batch: its facts, what blocks it, what to look at, and bulk decisions on what differs. import { Banner, Button, - ConfirmationDialog, CounterLabel, Details, FormControl, @@ -15,9 +14,12 @@ import { Text, useDetails, } from "@primer/react"; -import { Blankslate, ScrollableRegion } from "@primer/react/experimental"; +import { Blankslate } from "@primer/react/experimental"; import { useEffect, useMemo, useState, type ComponentProps, type RefObject } from "react"; import type { BlockerReason, DecisionKind, BatchView, RenderRow } from "../shared/api.ts"; +import { BulkDialog } from "./BulkDialog.tsx"; +import { DeltaOffers } from "./DeltaOffers.tsx"; +import type { DeltaState } from "./deltas.ts"; import { applyFilter, decidableIn, filterQuery, NO_FILTER, parseFilter, showQuery, SHOW_OPTIONS, targetsOf, type RowFilter, type Show } from "./filter.ts"; import { useStoryFocusKeys, useTitle, type BatchState } from "./hooks.ts"; import { Keys } from "./Keys.tsx"; @@ -176,56 +178,11 @@ function Blockers({ view, onShow }: { view: BatchView; onShow: (show: Show) => v interface PendingBulk { readonly decision: DecisionKind; - /** Each decidable, as they were when the dialog opened, which is exactly what gets sent. */ readonly rows: readonly RenderRow[]; /** All of the batch, as opposed to what a filter is showing. */ readonly scope: "all" | "shown"; } -function BulkDialog({ pending, onClose }: { pending: PendingBulk; onClose: (confirmed: boolean) => void }) { - const { decision, rows, scope } = pending; - const verb = decision === "approved" ? "Approve" : "Reject"; - const count = (test: (row: RenderRow) => boolean) => rows.filter(test).length; - const facts = [ - [count(({ state }) => state === "changed"), "changed"], - [count(({ state }) => state === "added"), "added"], - [count(({ decision: made }) => made?.decision === "approved"), "already approved"], - [count(({ decision: made }) => made?.decision === "rejected"), "already rejected"], - ] as const; - return ( - onClose(gesture === "confirm")} - > - - - {scope === "all" ? "This decides on every image in this batch that has one to decide on." : "This decides on exactly the images the current filter shows."}{" "} - A decision on an image holds in later batches too, until it's replaced. - -
    - {facts - .filter(([value]) => value > 0) - .map(([value, words]) => ( -
  • - {value.toLocaleString()} {words} -
    • - ))} -
- - {rows.slice(0, 200).map(({ story }) => ( -
{story}
- ))} - {rows.length > 200 &&
and {rows.length - 200} more
} - - - - ); -} - function BulkButton({ decision, scope, @@ -308,7 +265,7 @@ function Differences({ ))} - {(!isHosted || view.actor !== null) && ( + {canDecide(view) && ( {isFiltered && ( <> @@ -340,7 +297,14 @@ function Differences({ )} )} - {pending !== null && void confirm(confirmed)} />} + {pending !== null && ( + void confirm(confirmed)} + /> + )} ); } @@ -350,11 +314,13 @@ export function BatchScreen({ view, search, returningFrom, + deltas, }: { state: BatchState; view: BatchView; search: string; returningFrom: RefObject; + deltas: DeltaState; }) { const { batch, counts, outsideFilter } = view; const filter = useMemo(() => parseFilter(search), [search]); @@ -385,6 +351,7 @@ export function BatchScreen({ Every story + {canDecide(view) && } {newest !== undefined && newest.id !== batch.id && ( void; +}) { + const verb = decision === "approved" ? "Approve" : "Reject"; + const count = (test: (row: RenderRow) => boolean) => rows.filter(test).length; + const facts = [ + [count(({ state }) => state === "changed"), "changed"], + [count(({ state }) => state === "added"), "added"], + [count(({ decision: made }) => made?.decision === "approved"), "already approved"], + [count(({ decision: made }) => made?.decision === "rejected"), "already rejected"], + ] as const; + return ( + onClose(gesture === "confirm")} + > + + + {why}{" "} + A decision on an image holds in later batches too, until it's replaced. + +
    + {facts + .filter(([value]) => value > 0) + .map(([value, words]) => ( +
  • + {value.toLocaleString()} {words} +
    • + ))} +
+ + {rows.slice(0, 200).map(({ story }) => ( +
{story}
+ ))} + {rows.length > 200 &&
and {rows.length - 200} more
} + + + + ); +} diff --git a/ui/src/client/DeltaOffers.tsx b/ui/src/client/DeltaOffers.tsx new file mode 100644 index 0000000..8a4cab4 --- /dev/null +++ b/ui/src/client/DeltaOffers.tsx @@ -0,0 +1,256 @@ +// The changes that repeat across stories, to accept once instead of looking at each. +import { Button, Details, Heading, ProgressBar, Stack, Text, useDetails } from "@primer/react"; +import { ScrollableRegion } from "@primer/react/experimental"; +import { useState, type ReactNode } from "react"; +import type { RenderRow } from "../shared/api.ts"; +import type { Delta, DeltaExample, DeltaSide } from "../shared/deltas.ts"; +import { describeBox, isEmptyBox } from "../shared/hunks.ts"; +import { plural } from "../shared/plural.ts"; +import { imageUrl } from "./api.ts"; +import { BulkDialog } from "./BulkDialog.tsx"; +import type { DeltaState } from "./deltas.ts"; +import { showQuery, targetsOf } from "./filter.ts"; +import type { BatchState } from "./hooks.ts"; +import { Pane, placed, share } from "./ImageCompare.tsx"; +import { AppLink, navigate, storyPath } from "./router.tsx"; +import { stepped } from "./selection.ts"; +import { KindLabel, sizeChangeWords } from "./words.tsx"; + +/** How much around a change to show, since a few pixels mean nothing by themselves. */ +const CONTEXT = 12; +const MAX_WIDTH = 880; +const MAX_HEIGHT = 96; +/** As close as is any use, and past this a crop of a few pixels would be blocks. */ +const MAX_SCALE = 4; + +/** Where a crop looks: the first place, since a delta's pixels are the same in all of them. */ +const firstBox = ({ boxes }: DeltaSide) => boxes[0]!; + +function regionOf(side: DeltaSide) { + const { image } = side; + const box = firstBox(side); + const x = Math.max(box.x - CONTEXT, 0); + const y = Math.max(box.y - CONTEXT, 0); + return { x, y, width: Math.min(box.x + box.width + CONTEXT, image.width) - x, height: Math.min(box.y + box.height + CONTEXT, image.height) - y }; +} + +/** One scale for both sides, so they compare. A whole number when enlarging, so pixels stay even. */ +function scaleOf({ golden, current }: DeltaExample): number { + const sides = [golden, current].map(regionOf); + const fit = Math.min(MAX_WIDTH / Math.max(...sides.map(({ width }) => width)), MAX_HEIGHT / Math.max(...sides.map(({ height }) => height)), MAX_SCALE); + return fit < 1 ? fit : Math.floor(fit); +} + +/** As shares of what's shown, so a crop too wide for the page can narrow with it. */ +function Crop({ label, side, scale }: { label: string; side: DeltaSide; scale: number }) { + const { image, hash } = side; + const box = firstBox(side); + const region = regionOf(side); + return ( +
+
{label}
+ {isEmptyBox(box) ? ( + Nothing + ) : ( +
+ {`${label}, 1 ? "pixelated" : "auto", + }} + /> +
+
+ )} +
+ ); +} + +function DeltaCrops({ example }: { example: DeltaExample }) { + const scale = scaleOf(example); + return ( +
+ + +
+ ); +} + +/** Through the stories a delta would touch, as they really look, before saying yes to all of them. */ +function DeltaExamples({ delta, batchId }: { delta: Delta; batchId: string }) { + const { examples, kind } = delta; + const [wanted, setWanted] = useState(null); + // Deciding on an image takes it out of here, so this may not find it any more. + const at = Math.max(examples.findIndex(({ story }) => story === wanted), 0); + const example = examples[at]!; + const href = storyPath(batchId, example.story, showQuery("changed")); + const step = (by: 1 | -1) => setWanted(examples[stepped(at, examples.length, by)!]!.story); + + return ( + + + + + + {at + 1} of {examples.length} + + {example.story} + + +
+ {( + [ + ["Before", example.golden], + ["After", example.current], + ] as const + ).map(([label, side]) => ( + navigate(href)}> + + + ); +} + +/** A delta that isn't the one open, with a way to open it. */ +function ClosedDelta({ delta, onOpen }: { delta: Delta; onOpen: () => void }) { + return ( + + +
+ +
+ + ); +} + +/** A delta, and what can be done with it. */ +function DeltaRow({ delta, children, more }: { delta: Delta; children: ReactNode; more: ReactNode }) { + const { golden, current } = delta.examples[0]!; + return ( +
  • + + + + {plural(delta.examples.length, "snapshot")} + + {sizeChangeWords(firstBox(golden), firstBox(current))} + + {children} + + {more} + +
    • + ); +} + +function AcceptedDeltas({ deltas }: { deltas: DeltaState }) { + const { getDetailsProps } = useDetails({ closeOnOutsideClick: false }); + return ( +
    + {plural(deltas.taken.length, "delta")} accepted in this browser + + Undoing one brings its hunks back. Any approval it led to stays, and rejecting that image is how to take one back. + +
      + {deltas.taken.map((delta) => ( + }> + + + ))} +
    +
    + ); +} + +export function DeltaOffers({ deltas, state, batchId }: { deltas: DeltaState; state: BatchState; batchId: string }) { + const [pending, setPending] = useState<{ readonly delta: Delta; readonly rows: readonly RenderRow[] } | null>(null); + const [openKey, setOpenKey] = useState(null); + const { progress, offers, taken } = deltas; + // One's always open, so accepting one moves on to the next to look at. + const open = offers.find(({ key }) => key === openKey) ?? offers[0]; + + if (progress !== null) { + return ( + + Looking for changes that repeat across stories. + + + ); + } + if (offers.length === 0 && taken.length === 0) return null; + + const pick = (delta: Delta) => { + const rows = deltas.finishes(delta); + if (rows.length === 0) deltas.accept(delta); + else setPending({ delta, rows }); + }; + + const confirm = async (confirmed: boolean) => { + const chosen = pending; + setPending(null); + if (!confirmed || chosen === null) return; + deltas.accept(chosen.delta); + await state.decide("approved", targetsOf(chosen.rows)); + }; + + return ( + + + Repeated changes + + {offers.length > 0 && ( + <> + + These are the same pixels, changed the same way, in several stories. Accepting one draws it onto each golden in this browser, so what's left is less to look at, and approves any image with nothing left. + + +
      + {offers.map((delta) => ( + : setOpenKey(delta.key)} />}> + + + ))} +
    +     + + )} + {taken.length > 0 && } + {pending !== null && ( + + Accepting this delta leaves nothing different in these images, which approves them. + {pending.delta.examples.length > pending.rows.length && ` The other ${plural(pending.delta.examples.length - pending.rows.length, "snapshot")} will just show less.`} + + } + onClose={(confirmed) => void confirm(confirmed)} + /> + )} +     + ); +} diff --git a/ui/src/client/DetailScreen.deltas.stories.tsx b/ui/src/client/DetailScreen.deltas.stories.tsx new file mode 100644 index 0000000..2af9bed --- /dev/null +++ b/ui/src/client/DetailScreen.deltas.stories.tsx @@ -0,0 +1,54 @@ +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { expect, userEvent, waitFor, within } from "storybook/test"; +import type { RenderRow } from "../shared/api.ts"; +import { deltaKeysOf } from "./deltas.ts"; +import { useFetched } from "./hooks.ts"; +import { rows } from "./stories/fixtures.ts"; +import { press, StoryPage } from "./stories/storyPage.tsx"; + +/** A story with these of its hunks accepted, whose keys are what its images really hold. */ +function Accepted({ row, hunks }: { readonly row: RenderRow; readonly hunks: readonly number[] }) { + const { data: keys } = useFetched(row.story, () => deltaKeysOf(row)); + if (keys === null) return null; + const accepted = new Set(hunks.flatMap((hunk) => keys[hunk] ?? [])); + return ; +} + +const meta = { title: "Pages/Story/Deltas", component: Accepted } satisfies Meta; +export default meta; +type Story = StoryObj; + +/** Stacked or not, a redrawn golden is only there once it's been drawn and loaded. */ +const drawn = async (canvasElement: HTMLElement) => { + await within(canvasElement).findByText(/^Accepted deltas account for/); + await waitFor(() => { + const images = [...canvasElement.querySelectorAll('img[src^="blob:"]')]; + expect(images.length).toBeGreaterThan(0); + for (const image of images) expect(image.naturalWidth).toBeGreaterThan(0); + }); +}; + +const SEVERAL = rows.several("pages-home--rearranged"); +const TALLER = rows.taller("components-list--with-new-row"); + +export const PartlyAccepted: Story = { args: { row: SEVERAL, hunks: [0] }, play: async ({ canvasElement }) => drawn(canvasElement) }; +export const AllAccepted: Story = { + args: { row: SEVERAL, hunks: SEVERAL.pixelDiff!.hunks.map((_, index) => index) }, + play: async ({ canvasElement }) => drawn(canvasElement), +}; +export const GrewAndAccepted: Story = { args: { row: TALLER, hunks: [0] }, play: async ({ canvasElement }) => drawn(canvasElement) }; +export const DiffOfWhatsLeft: Story = { + args: { row: SEVERAL, hunks: [0] }, + play: async ({ canvasElement }) => { + await drawn(canvasElement); + await press("d")(); + }, +}; +export const GoldenOnRecord: Story = { + args: { row: SEVERAL, hunks: [0] }, + play: async ({ canvasElement }) => { + await drawn(canvasElement); + await userEvent.click(within(canvasElement).getByRole("button", { name: "Show the golden on record" })); + await expect(within(canvasElement).getByRole("button", { name: "Show with deltas" })).toBeVisible(); + }, +}; diff --git a/ui/src/client/DetailScreen.tsx b/ui/src/client/DetailScreen.tsx index 03c858d..9a37011 100644 --- a/ui/src/client/DetailScreen.tsx +++ b/ui/src/client/DetailScreen.tsx @@ -7,7 +7,8 @@ import { findRenames } from "../shared/renames.ts"; import { isDecidable } from "selfie-store"; import type { DecisionKind, PixelDiff, BatchView, RenderRow } from "../shared/api.ts"; import { diffImageUrl, fetchDiff, imageUrl } from "./api.ts"; -import { isHosted, signInHref } from "./base.ts"; +import { prepareNext, usePatchedGolden, type PatchedGolden } from "./deltas.ts"; +import { canDecide, signInHref } from "./base.ts"; import { parseFilter, targetsOf, type StoryNavigation } from "./filter.ts"; import { useFetched, useHotkeys, useHunkSelection, useTitle, type FeedbackState, type BatchState } from "./hooks.ts"; import { imageCount, ImageCompare } from "./ImageCompare.tsx"; @@ -67,7 +68,7 @@ function PixelStats({ stats }: { stats: PixelDiff }) { return ( {describePixels(stats)} - {!isLayoutOnly(stats) && ` of ${stats.totalPixels.toLocaleString()} changed`} + {stats.changedPixels > 0 && ` of ${stats.totalPixels.toLocaleString()} changed`} {box === null ? "" : `, within ${describeBox(box)}`}. {account.length > 0 && ` ${account.join(", ")} pixels.`} {stats.maxChannelDelta > 0 && ` The largest difference in a channel is ${stats.maxChannelDelta} of 255.`} @@ -107,6 +108,20 @@ function Notes({ row, diff, renamedFrom, renamedTo }: { row: RenderRow; diff: Pi ); } +const NOTHING_ACCEPTED: ReadonlySet = new Set(); + +/** Says the golden on screen isn't the one on record, and how to see that one. */ +function PatchedBanner({ patched, shown, onShown }: { patched: PatchedGolden; shown: boolean; onShown: (shown: boolean) => void }) { + return ( + onShown(!shown)}>{shown ? "Show the golden on record" : "Show with deltas"}} + /> + ); +} + /** Where this story is, and the way to the ones either side of it. */ export function StoryPager({ previous, next, position }: Pick) { const go = (href: string | null) => { @@ -134,6 +149,7 @@ export function DetailScreen({ navigation, liveStories, startNote = false, + accepted = NOTHING_ACCEPTED, }: { state: BatchState; view: BatchView; @@ -145,6 +161,8 @@ export function DetailScreen({ liveStories?: readonly [boolean, (open: boolean) => void]; /** Someone asked to write a note on this story, and it's just been rendered so they can. */ startNote?: boolean; + /** The deltas accepted on this batch, which only its own pages have. */ + accepted?: ReadonlySet; }) { const { unchanged, loadUnchanged } = state; const filter = useMemo(() => parseFilter(search), [search]); @@ -176,7 +194,12 @@ export function DetailScreen({ const { data: fetched, error: diffError } = useFetched(changedPair && row.pixelDiff === null ? `${row.imageHash}/${compared.hash}` : null, (signal) => fetchDiff(row!.imageHash!, compared!.hash, signal), ); - const diff = row?.pixelDiff ?? fetched; + const recordedDiff = row?.pixelDiff ?? fetched; + const patched = usePatchedGolden(row, accepted); + // Here, so that it stays as it is from story to story, like how someone's looking at the images. + const [deltasShown, setDeltasShown] = useState(true); + const applied = deltasShown ? patched : null; + const diff = applied?.diff ?? recordedDiff; const mergeDistance = view.hunkMergeDistance * view.batch.scale; const groups = useMemo(() => (diff === null ? [] : groupHunks(diff.hunks, mergeDistance)), [diff, mergeDistance]); const selection = useHunkSelection(story); @@ -194,12 +217,13 @@ export function DetailScreen({ for (const url of urls) { if (url !== null) new Image().src = url; } - }, [preloading]); + prepareNext(preloading, accepted); + }, [preloading, accepted]); - const canDecide = !isHosted || view.actor !== null; + const mayDecide = canDecide(view); const decide = async (decision: DecisionKind) => { if (row === null || state.busy) return; - if (!canDecide) { + if (!mayDecide) { setMessage("Sign in to approve or reject."); return; } @@ -255,8 +279,8 @@ export function DetailScreen({ - {decidable && !canDecide && Sign in to approve} - {decidable && canDecide && ( + {decidable && !mayDecide && Sign in to approve} + {decidable && mayDecide && ( {children}
    @@ -151,6 +157,8 @@ function Pane({ interface ImageCompareProps { readonly row: RenderRow; readonly diff: PixelDiff | null; + /** The golden and diff to show instead, which have accepted deltas drawn on them, as `diff` already says. */ + readonly patched: PatchedImages | null; readonly scale: number; readonly groups: readonly HunkGroup[]; readonly selection: HunkSelection; @@ -160,14 +168,14 @@ interface ImageCompareProps { readonly startNote?: boolean; } -export function ImageCompare({ row, diff, scale, groups, selection, batchId, feedback, startNote = false }: ImageCompareProps) { +export function ImageCompare({ row, diff, patched, scale, groups, selection, batchId, feedback, startNote = false }: ImageCompareProps) { const { story, imageHash, goldenHash } = row; const allNotes = feedback.view?.notes ?? []; const storyNotes = allNotes.filter((note) => note.story === story); const openNotes = storyNotes.filter(isOpenNote); const notedImage = newestNotedImage(allNotes, story); const compared = comparedImage(row, notedImage); - const sources = sourcesOf(row, diff !== null, notedImage, compared?.hash ?? null); + const sources = sourcesOf(row, diff !== null, notedImage, compared?.hash ?? null, patched); const oneImage = sources.length === 1; const canFlip = sources.length > 1 && compared !== null; // A failed story's one image may be its render or its golden, so it has to say which it is. @@ -182,6 +190,7 @@ export function ImageCompare({ row, diff, scale, groups, selection, batchId, fee // The layer someone asked for, or, in a story without it, the one it has. const shown = sources.find(({ name }) => name === wantedLayer) ?? sources.find(({ name }) => name === "current") ?? sources[0]; const layer = shown?.name ?? "current"; + const shownWords = shown?.words ?? LAYER_WORDS.current; const [zoom, setZoom] = useState("size"); const zoomOptions: readonly ZoomOption[] = oneImage ? ["fit", ...ZOOMS] : ZOOMS; const zoomShown: ZoomOption = mode === "side" ? "fit" : zoom; @@ -268,20 +277,23 @@ export function ImageCompare({ row, diff, scale, groups, selection, batchId, fee } else section.current?.querySelector(`[data-hunk="${index}"]`)?.scrollIntoView({ block: "center" }); }; - const hashOn = (on: Layer) => sources.find(({ name }) => name === on)?.hash ?? null; + const sourceOn = (on: Layer) => sources.find(({ name }) => name === on); + const hashOn = (on: Layer) => sourceOn(on)?.hash ?? null; const outlinesOn = (on: Layer, size: Size | undefined) => size !== undefined && ( <> {outlined && } - noted === hashOn(on))} drawn={mode === "flip" ? drawn : null} size={size} /> + noted === hashOn(on))} drawn={mode === "flip" ? drawn : null} size={size} /> ); - const selectedBoxes = [...selected].flatMap((index) => boxOn(groups[index]!, hunkLayer(layer)) ?? []); - const noteBox = drawn ?? (selectedBoxes.length === 0 ? null : selectedBoxes.reduce(hull)); + const boxable = shown?.boxable ?? true; + const drawnBox = boxable ? drawn : null; + const selectedBoxes = boxable ? [...selected].flatMap((index) => boxOn(groups[index]!, hunkLayer(layer)) ?? []) : []; + const noteBox = drawnBox ?? (selectedBoxes.length === 0 ? null : selectedBoxes.reduce(hull)); const imageWords = hashOn(layer) === null ? "this story" : LAYER_IMAGE_WORDS[layer]; - const noteTarget = drawn !== null ? `on the box you drew on ${imageWords}` : selectedBoxes.length > 0 ? `on ${plural(selectedBoxes.length, "selected hunk")} of ${imageWords}` : `on all of ${imageWords}`; + const noteTarget = drawnBox !== null ? `on the box you drew on ${imageWords}` : selectedBoxes.length > 0 ? `on ${plural(selectedBoxes.length, "selected hunk")} of ${imageWords}` : `on all of ${imageWords}`; const addNote = async (text: string) => { const saved = await feedback.add({ story, batchId, imageHash: hashOn(layer), box: noteBox, text }); @@ -365,9 +377,9 @@ export function ImageCompare({ row, diff, scale, groups, selection, batchId, fee <> {sources.length > 1 && ( setLayer(sources[index]!.name)}> - {sources.map(({ name }) => ( + {sources.map(({ name, words }) => ( - {LAYER_WORDS[name]} + {words} ))} @@ -390,12 +402,12 @@ export function ImageCompare({ row, diff, scale, groups, selection, batchId, fee {mode === "flip" ? ( <> - Showing {captioned ? {LAYER_WORDS[layer].toLowerCase()} : "the image"} + Showing {captioned ? {shownWords.toLowerCase()} : "the image"} {shownSize === undefined ? "" : `, ${shownSize.width}×${shownSize.height}`} rendered at {scale}×,{" "} {zoom === "size" ? "at its actual size." : `with one image pixel ${zoom} device ${zoom === 1 ? "pixel" : "pixels"} wide.`}{" "} {canFlip && ( <> - flips current and {LAYER_WORDS[compared!.layer].toLowerCase()}.{" "} + flips current and {sourceOn(compared!.layer)?.words.toLowerCase()}.{" "} )} {hasDiffLayer && ( @@ -429,10 +441,10 @@ export function ImageCompare({ row, diff, scale, groups, selection, batchId, fee {mode === "side" ? (
    {/* Keyed by image, so an added story never shows the last one's image while its own loads. */} - {sources.map(({ name, src }) => ( + {sources.map(({ name, src, words }) => ( ) : ( -
    -
    +
    +
    {sources.map(({ name, src }) => { const size = sizes[src]; return ( {name `/batches/${encodeURIComponent(batchId)} export const fetchBatch = (batchId: string, signal: AbortSignal) => request(batchRoute(batchId), { signal }); export const fetchBatchContext = (batchId: string, signal: AbortSignal) => request(`/batches/${encodeURIComponent(batchId)}/context`, { signal }); export const fetchUnchanged = async (batchId: string) => (await request<{ rows: readonly RenderRow[] }>(`${batchRoute(batchId)}/unchanged`)).rows; -export const fetchDiff = (current: string, golden: string, signal: AbortSignal) => request(`/diffs/${current}/${golden}`, { signal }); +export const fetchDiff = (current: string, golden: string, signal: AbortSignal | null = null) => request(`/diffs/${current}/${golden}`, { signal }); export const postDecision = (batchId: string, decision: DecisionKind, renders: readonly DecisionTarget[]) => post(`${batchRoute(batchId)}/decisions`, { decision, renders }); diff --git a/ui/src/client/base.ts b/ui/src/client/base.ts index ab6b339..2001740 100644 --- a/ui/src/client/base.ts +++ b/ui/src/client/base.ts @@ -1,3 +1,5 @@ +import type { BatchView } from "../shared/api.ts"; + // Where this page's project is. Locally there's only one, at the root, and in the cloud the Worker says, // since a project's pages are under its name. export const BASE = typeof document === "undefined" ? "" : (document.querySelector('meta[name="selfie-base"]')?.getAttribute("content") ?? ""); @@ -5,5 +7,8 @@ export const BASE = typeof document === "undefined" ? "" : (document.querySelect /** In the cloud: anyone can look, only the writer can decide, and notes and rendering stay on a person's own machine. */ export const isHosted = BASE !== ""; +/** Anyone can on someone's own machine, and in the cloud, only whoever's signed in. */ +export const canDecide = ({ actor }: Pick) => !isHosted || actor !== null; + /** Comes back to this page once signed in. */ export const signInHref = () => `/sign-in?next=${encodeURIComponent(`${window.location.pathname}${window.location.search}`)}`; diff --git a/ui/src/client/deltas.ts b/ui/src/client/deltas.ts new file mode 100644 index 0000000..a059634 --- /dev/null +++ b/ui/src/client/deltas.ts @@ -0,0 +1,283 @@ +// Finding deltas, and drawing what accepting them leaves. All of it happens here: which are accepted +// is this browser's, for this batch, and what reaches the server is only an approval of a whole image. +import { useEffect, useMemo, useState } from "react"; +import { mapConcurrently, sha256, toHex } from "selfie-store"; +import type { BoundingBox, ChangedHunk, PixelDiff, RenderRow } from "../shared/api.ts"; +import { acceptedDeltas, canApply, finishedBy, hunksAccepted, isComparable, offeredDeltas, patchGolden, repeatedSizes, sizeKey, type Comparable, type Delta, type GoldenPatch, type ScannedRow } from "../shared/deltas.ts"; +import { fade } from "../shared/diff-image.ts"; +import { isEmptyBox } from "../shared/hunks.ts"; +import { diffImageUrl, fetchDiff, imageUrl } from "./api.ts"; +import { useFetched } from "./hooks.ts"; + +/** Enough to go quickly, and few enough to leave room for whatever else the page is asking for. */ +const AT_ONCE = 4; +/** Each holds two images, and this is plenty for what's on screen, what's next, and going back a little. */ +const PATCHES_KEPT = 16; + +const pairOf = ({ imageHash, goldenHash }: Comparable) => `${imageHash}/${goldenHash}`; + +/** + * What's been worked out, by content, so it never changes. At most one of each is on its way, and + * it's never called off, since whoever asks next wants the same answer. + */ +class Known { + private readonly held = new Map(); + private readonly coming = new Map>(); + private readonly limit: number; + private readonly dropped: (value: Value) => void; + + constructor(limit = Infinity, dropped: (value: Value) => void = () => undefined) { + this.limit = limit; + this.dropped = dropped; + } + + /** Counts as using it, so what's on screen is never the oldest. */ + peek(id: string): Value | undefined { + const value = this.held.get(id); + if (value !== undefined) { + this.held.delete(id); + this.held.set(id, value); + } + return value; + } + + private hold(id: string, value: Value): Value { + this.held.set(id, value); + if (this.held.size > this.limit) { + const [oldest, old] = this.held.entries().next().value as [string, Value]; + this.held.delete(oldest); + this.dropped(old); + } + return value; + } + + of(id: string, work: () => Promise): Promise { + const value = this.peek(id); + if (value !== undefined) return Promise.resolve(value); + const coming = this.coming.get(id) ?? work().then((done) => this.hold(id, done)).finally(() => this.coming.delete(id)); + this.coming.set(id, coming); + return coming; + } +} + +export interface PatchedImages { + readonly golden: string; + readonly diff: string; +} + +export interface PatchedGolden { + readonly images: PatchedImages; + /** What's left, against `images.golden`. */ + readonly diff: PixelDiff; +} + +const knownDiffs = new Known(); +const knownKeys = new Known(); +const knownPatches = new Known(PATCHES_KEPT, ({ images }) => { + URL.revokeObjectURL(images.golden); + URL.revokeObjectURL(images.diff); +}); + +async function decoded(url: string): Promise { + const response = await fetch(url); + if (!response.ok) throw new Error(`The server answered ${response.status} for ${url}.`); + return createImageBitmap(await response.blob(), { colorSpaceConversion: "none", premultiplyAlpha: "none" }); +} + +async function withImages({ imageHash, goldenHash }: Comparable, work: (current: ImageBitmap, golden: ImageBitmap) => Promise): Promise { + const [current, golden] = await Promise.all([decoded(imageUrl(imageHash)), decoded(imageUrl(goldenHash))]); + try { + return await work(current, golden); + } finally { + current.close(); + golden.close(); + } +} + +function pixelsIn(image: ImageBitmap, { x, y, width, height }: BoundingBox): ImageData { + const context = new OffscreenCanvas(width, height).getContext("2d", { willReadFrequently: true })!; + context.drawImage(image, x, y, width, height, 0, 0, width, height); + return context.getImageData(0, 0, width, height); +} + +const digest = async (image: ImageBitmap, box: BoundingBox) => (isEmptyBox(box) ? "" : toHex(await sha256(new Uint8Array(pixelsIn(image, box).data.buffer)))); + +async function keyOf(hunk: ChangedHunk, current: ImageBitmap, golden: ImageBitmap): Promise { + const [before, after] = await Promise.all([digest(golden, hunk.golden), digest(current, hunk.current)]); + return `${sizeKey(hunk)}:${before}:${after}`; +} + +/** Always of these two images. A review's stats are by story, and a later approval can change what it's compared with, so they could be another pair's. */ +const diffOf = (row: Comparable) => knownDiffs.of(pairOf(row), () => fetchDiff(row.imageHash, row.goldenHash)); + +const hunkKeys = (row: Comparable) => + knownKeys.of(pairOf(row), async () => { + const diff = await diffOf(row); + return withImages(row, (current, golden) => Promise.all(diff.hunks.map((hunk) => (canApply(hunk, diff) ? keyOf(hunk, current, golden) : null)))); + }); + +/** What each of a story's hunks holds, for saying which are accepted. */ +export const deltaKeysOf = (row: RenderRow) => (isComparable(row) ? hunkKeys(row) : Promise.resolve([])); + +/** The diff image with what's settled drawn as what didn't change, which the server's can't know. */ +async function settledDiffImage({ imageHash, goldenHash }: Comparable, { settled }: GoldenPatch, current: ImageBitmap): Promise { + const image = await decoded(diffImageUrl(imageHash, goldenHash)); + const canvas = new OffscreenCanvas(image.width, image.height); + const context = canvas.getContext("2d")!; + context.drawImage(image, 0, 0); + image.close(); + for (const box of settled) { + const pixels = pixelsIn(current, box); + for (let at = 0; at < pixels.data.length; at += 4) fade(pixels.data, pixels.data, at); + context.putImageData(pixels, box.x, box.y); + } + return canvas.convertToBlob({ type: "image/png" }); +} + +const drawPatch = (row: Comparable, patch: GoldenPatch): Promise => + withImages(row, async (current, golden) => { + const canvas = new OffscreenCanvas(patch.diff.golden.width, patch.diff.golden.height); + const context = canvas.getContext("2d")!; + context.imageSmoothingEnabled = false; + for (const { from, box, toX, toY } of patch.blits) { + context.drawImage(from === "golden" ? golden : current, box.x, box.y, box.width, box.height, toX, toY, box.width, box.height); + } + const [goldenBlob, diffBlob] = await Promise.all([canvas.convertToBlob({ type: "image/png" }), settledDiffImage(row, patch, current)]); + return { golden: URL.createObjectURL(goldenBlob), diff: URL.createObjectURL(diffBlob) }; + }); + +const patchId = (row: Comparable, hunks: readonly number[]) => `${pairOf(row)}:${hunks.join()}`; + +async function patchFor(row: Comparable, accepted: ReadonlySet): Promise { + const [diff, keys] = await Promise.all([diffOf(row), hunkKeys(row)]); + const hunks = hunksAccepted(keys, accepted); + if (hunks.length === 0) return null; + return knownPatches.of(patchId(row, hunks), async () => { + const patch = patchGolden(diff, hunks); + return { images: await drawPatch(row, patch), diff: patch.diff }; + }); +} + +/** Already drawn, so moving to a story that has been shown or got ready never shows what it looked like before. */ +function patchKnown(row: Comparable, accepted: ReadonlySet): PatchedGolden | null { + const keys = knownKeys.peek(pairOf(row)); + return keys === undefined ? null : (knownPatches.peek(patchId(row, hunksAccepted(keys, accepted))) ?? null); +} + +/** A story's golden with what's been accepted drawn onto it, or null with nothing to draw, or until it's drawn. */ +export function usePatchedGolden(row: RenderRow | null, accepted: ReadonlySet): PatchedGolden | null { + const comparable = row !== null && accepted.size > 0 && isComparable(row) ? row : null; + const id = comparable === null ? null : `${pairOf(comparable)}:${[...accepted].sort().join()}`; + const { data } = useFetched(id, () => patchFor(comparable!, accepted)); + return useMemo(() => data ?? (comparable === null ? null : patchKnown(comparable, accepted)), [data, id]); +} + +/** Has the story after this one drawn before someone gets to it. */ +export function prepareNext(row: RenderRow, accepted: ReadonlySet): void { + if (accepted.size > 0 && isComparable(row)) patchFor(row, accepted).catch(() => undefined); +} + +export interface DeltaState { + /** How far through looking for them, in percent, or null once that's done. */ + readonly progress: number | null; + readonly accepted: ReadonlySet; + readonly offers: readonly Delta[]; + /** What's been accepted, whichever stories still have it to show. */ + readonly taken: readonly Delta[]; + /** The undecided stories that accepting this leaves nothing different in. */ + readonly finishes: (delta: Delta) => readonly RenderRow[]; + readonly accept: (delta: Delta) => void; + readonly undo: (delta: Delta) => void; +} + +const storageKey = (batchId: string) => `selfie:accepted-deltas:${batchId}`; + +function readAccepted(batchId: string): ReadonlySet { + try { + const held: unknown = JSON.parse(localStorage.getItem(storageKey(batchId)) ?? "[]"); + return new Set(Array.isArray(held) ? held.filter((key) => typeof key === "string") : []); + } catch { + return new Set(); + } +} + +/** Which deltas are accepted on this batch, kept in this browser, and the same in all its tabs. */ +function useAcceptedKeys(batchId: string) { + const [accepted, setAccepted] = useState(() => readAccepted(batchId)); + useEffect(() => { + const onStorage = ({ key }: StorageEvent) => { + if (key === storageKey(batchId)) setAccepted(readAccepted(batchId)); + }; + window.addEventListener("storage", onStorage); + return () => window.removeEventListener("storage", onStorage); + }, [batchId]); + const save = (next: ReadonlySet) => { + localStorage.setItem(storageKey(batchId), JSON.stringify([...next])); + setAccepted(next); + }; + return [accepted, save] as const; +} + +/** + * Every changed story's hunks and what's in them. `looking` is whether to go on finding that, since + * only the batch's own screen shows what it finds, and a story's images come first. What's found stays. + */ +export function useDeltas(batchId: string, rows: readonly RenderRow[] | undefined, looking: boolean): DeltaState { + const [accepted, setAccepted] = useAcceptedKeys(batchId); + // How many times it's finished looking, since what it found is in what's known. + const [looked, setLooked] = useState(0); + const [progress, setProgress] = useState(null); + const pairs = useMemo(() => (rows ?? []).filter(isComparable), [rows]); + const pairsId = useMemo(() => pairs.map(pairOf).join(), [pairs]); + + useEffect(() => { + if (!looking || pairs.length === 0) { + setProgress(null); + return; + } + let stopped = false; + /** One half of the way there. A whole percent, so the page only draws again when that changes. One that can't be compared just can't be part of a delta. */ + const halfway = async (items: readonly Comparable[], work: (row: Comparable) => Promise, from: number) => { + let done = 0; + await mapConcurrently(items, AT_ONCE, async (row) => { + if (stopped) return; + await work(row).catch(() => undefined); + if (!stopped) setProgress(from + Math.round((++done / items.length) * 50)); + }); + }; + void (async () => { + setProgress(0); + await halfway(pairs, diffOf, 0); + const diffOfRow = (row: Comparable) => knownDiffs.peek(pairOf(row)); + const sizes = repeatedSizes(pairs.flatMap((row) => diffOfRow(row) ?? [])); + await halfway(pairs.filter((row) => diffOfRow(row)?.hunks.some((hunk) => sizes.has(sizeKey(hunk)))), hunkKeys, 50); + if (stopped) return; + setLooked((times) => times + 1); + setProgress(null); + })(); + return () => { + stopped = true; + }; + }, [looking, pairsId]); + + const scans = useMemo( + () => + pairs.flatMap((row): ScannedRow[] => { + const diff = knownDiffs.peek(pairOf(row)); + const keys = knownKeys.peek(pairOf(row)); + return diff === undefined || keys === undefined ? [] : [{ row, diff, keys }]; + }), + [pairs, looked], + ); + const offers = useMemo(() => offeredDeltas(scans, accepted), [scans, accepted]); + const taken = useMemo(() => acceptedDeltas(scans, accepted), [scans, accepted]); + return { + progress, + accepted, + offers, + taken, + finishes: ({ key }) => finishedBy(scans, new Set([...accepted, key])), + accept: ({ key }) => setAccepted(new Set([...accepted, key])), + undo: ({ key }) => setAccepted(new Set([...accepted].filter((held) => held !== key))), + }; +} diff --git a/ui/src/client/stories/fixtures.ts b/ui/src/client/stories/fixtures.ts index 35ddd7f..756a3e9 100644 --- a/ui/src/client/stories/fixtures.ts +++ b/ui/src/client/stories/fixtures.ts @@ -11,6 +11,8 @@ import type { BatchView, RenderRow, } from "../../shared/api.ts"; +import type { Delta } from "../../shared/deltas.ts"; +import type { DeltaState } from "../deltas.ts"; import type { FeedbackState, BatchState } from "../hooks.ts"; import { RequestError } from "../api.ts"; import { HASHES, PIXEL_DIFFS } from "./images.generated.ts"; @@ -181,4 +183,22 @@ export function stateOf(view: BatchView, overrides: Partial = {}): B }; } +/** A delta from the first hunk of a generated diff, in these stories. */ +export function deltaFrom(pair: keyof typeof PIXEL_DIFFS, stories: readonly string[]): Delta { + const [current, golden] = pair.split("-") as [keyof typeof HASHES, keyof typeof HASHES]; + const diff = PIXEL_DIFFS[pair]; + const { kind, golden: goldenBox, current: currentBox } = diff.hunks[0]!; + const examples = stories.map((story) => ({ + story, + golden: { hash: HASHES[golden], boxes: [goldenBox], image: diff.golden }, + current: { hash: HASHES[current], boxes: [currentBox], image: diff.current }, + })); + return { key: pair, kind, examples }; +} + +/** Nothing found, with actions that are recorded but change nothing. */ +export function deltasOf(overrides: Partial = {}): DeltaState { + return { progress: null, accepted: new Set(), offers: [], taken: [], finishes: () => [], accept: fn(), undo: fn(), ...overrides }; +} + export const failure = (code: string, message: string) => new RequestError(code, message); diff --git a/ui/src/client/stories/storyPage.tsx b/ui/src/client/stories/storyPage.tsx index a492de6..a1036cc 100644 --- a/ui/src/client/stories/storyPage.tsx +++ b/ui/src/client/stories/storyPage.tsx @@ -10,14 +10,15 @@ export interface StoryPageArgs { readonly others?: readonly RenderRow[]; readonly view?: BatchView; readonly notes?: readonly Note[]; + readonly accepted?: ReadonlySet; } -export function StoryPage({ row, others = [], view, notes = [] }: StoryPageArgs) { +export function StoryPage({ row, others = [], view, notes = [], accepted }: StoryPageArgs) { const shown = view ?? viewOf([row, ...others]); const state = stateOf(shown); return ( - {(navigation) => navigation !== null && } + {(navigation) => navigation !== null && } ); } diff --git a/ui/src/client/styles.css b/ui/src/client/styles.css index a2003bd..fd55428 100644 --- a/ui/src/client/styles.css +++ b/ui/src/client/styles.css @@ -780,3 +780,46 @@ code { gap: var(--base-size-8); } } + +.selfie-deltas { + max-height: 75vh; + overflow: auto; +} + +.selfie-delta-whole { + display: grid; + grid-template-columns: repeat(2, minmax(0, 1fr)); + gap: var(--base-size-12); + align-items: start; +} + +.selfie-delta-crops { + display: flex; + flex-wrap: wrap; + gap: var(--base-size-8) var(--base-size-16); +} + +.selfie-delta-crop { + min-width: 0; + max-width: 100%; + margin: 0; +} + +.selfie-delta-window { + position: relative; + max-width: 100%; + overflow: hidden; + border: var(--borderWidth-thin) solid var(--borderColor-default); +} + +.selfie-delta-window img { + position: absolute; + max-width: none; +} + +.selfie-delta-box { + position: absolute; + box-sizing: border-box; + border: var(--borderWidth-thin) solid var(--borderColor-accent-emphasis); + pointer-events: none; +} diff --git a/ui/src/client/words.tsx b/ui/src/client/words.tsx index 8b4dc65..2fb6fcc 100644 --- a/ui/src/client/words.tsx +++ b/ui/src/client/words.tsx @@ -137,11 +137,16 @@ export function DecisionLabel({ row }: { readonly row: Pick current.width !== golden.width || current.height !== golden.height; +export const sizeChangeWords = (from: Size, to: Size) => `${from.width}×${from.height} to ${to.width}×${to.height}`; + export function describePixels(diff: PixelDiff | null): string { if (diff === null) return ""; - const size = sizeChanged(diff) ? `, ${diff.golden.width}×${diff.golden.height} to ${diff.current.width}×${diff.current.height}` : ""; + if (diff.hunks.length === 0) return "Nothing left to look at"; + const size = sizeChanged(diff) ? `, ${sizeChangeWords(diff.golden, diff.current)}` : ""; if (isLayoutOnly(diff)) return `Moved and spacing only${size}`; const percent = (diff.changedPixels / diff.totalPixels) * 100; return `${plural(diff.changedPixels, "pixel")} (${percent < 0.01 ? "<0.01" : percent.toFixed(2)}%)${size}`; diff --git a/ui/src/server/app.ts b/ui/src/server/app.ts index 4c5e736..b0ba238 100644 --- a/ui/src/server/app.ts +++ b/ui/src/server/app.ts @@ -50,7 +50,7 @@ export const pageHeaders = (storybookOrigin?: string): MiddlewareHandler => contentSecurityPolicy: { defaultSrc: ["'self'"], styleSrc: ["'self'", "'unsafe-inline'"], - imgSrc: ["'self'", "data:"], + imgSrc: ["'self'", "data:", "blob:"], ...(storybookOrigin === undefined ? {} : { frameSrc: [storybookOrigin] }), frameAncestors: ["'none'"], baseUri: ["'none'"], diff --git a/ui/src/shared/deltas.ts b/ui/src/shared/deltas.ts new file mode 100644 index 0000000..b708b6a --- /dev/null +++ b/ui/src/shared/deltas.ts @@ -0,0 +1,183 @@ +// A delta is one change repeated across stories: hunks whose two sides are the same size and hold the +// same pixels, wherever they sit. Accepting one redraws its hunks onto the golden, in the browser and +// nowhere else, so what's left to look at is smaller. Pure, so a test needs no browser. +import type { BoundingBox, ChangedHunk, PixelDiff, RenderRow } from "./api.ts"; +import { seamOf } from "./diff-image.ts"; +import { hull, isEmptyBox } from "./hunks.ts"; + +/** Fewer than this isn't worth stopping for, since looking at each is about as quick. */ +export const MIN_AFFECTED = 3; + +/** A changed render with both its images, which is all a delta can be between. */ +export type Comparable = RenderRow & { readonly imageHash: string; readonly goldenHash: string }; +export const isComparable = (row: RenderRow): row is Comparable => row.state === "changed" && row.imageHash !== null && row.goldenHash !== null; + +/** One story's hunks. A key says what a hunk's pixels are, or is null if it can't be redrawn on its own. */ +export interface ScannedRow { + readonly row: Comparable; + readonly diff: PixelDiff; + /** By hunk. */ + readonly keys: readonly (string | null)[]; +} + +/** Where one side of a delta is, in one story's image: every place it has it, which accepting covers, in the order of its hunks. */ +export interface DeltaSide { + readonly hash: string; + readonly boxes: readonly BoundingBox[]; + /** The whole image's, to crop it. */ + readonly image: { readonly width: number; readonly height: number }; +} + +/** One story a delta is in. */ +export interface DeltaExample { + readonly story: string; + readonly golden: DeltaSide; + readonly current: DeltaSide; +} + +export interface Delta { + readonly key: string; + readonly kind: ChangedHunk["kind"]; + /** Each story once, in the order they're listed. */ + readonly examples: readonly DeltaExample[]; +} + +export const sizeKey = ({ golden, current }: ChangedHunk) => `${golden.width}x${golden.height}>${current.width}x${current.height}`; + +const sameSize = ({ golden, current }: ChangedHunk) => golden.width === current.width && golden.height === current.height; + +/** + * A hunk of one size stays in its place, and one that changes height has to move all that's below it, + * which takes the image's whole width. A narrower one only balances out with others in its column. + */ +export const canApply = (hunk: ChangedHunk, { golden, current }: Pick) => + sameSize(hunk) || (hunk.golden.x === 0 && hunk.golden.width === golden.width && hunk.current.x === 0 && hunk.current.width === current.width); + +/** Only sizes enough stories share are worth decoding any images for. */ +export function repeatedSizes(diffs: readonly PixelDiff[]): ReadonlySet { + const counts = new Map(); + for (const diff of diffs) { + const sizes = new Set(diff.hunks.filter((hunk) => canApply(hunk, diff)).map(sizeKey)); + for (const size of sizes) counts.set(size, (counts.get(size) ?? 0) + 1); + } + return new Set([...counts].filter(([, count]) => count >= MIN_AFFECTED).map(([size]) => size)); +} + +const areaOf = ({ width, height }: BoundingBox) => width * height; + +const sideWith = (box: BoundingBox, hash: string, image: DeltaSide["image"], known: DeltaSide | undefined): DeltaSide => ({ hash, image, boxes: [...(known?.boxes ?? []), box] }); + +/** Whichever `wanted` picks, most stories first, and then the largest, so the order never depends on the list's. */ +function deltasIn(scans: readonly ScannedRow[], wanted: (key: string) => boolean): Delta[] { + const found = new Map }>(); + for (const { row, diff, keys } of scans) { + diff.hunks.forEach((hunk, index) => { + const key = keys[index]; + if (key === null || key === undefined || !wanted(key)) return; + const { examples } = found.get(key) ?? found.set(key, { kind: hunk.kind, examples: new Map() }).get(key)!; + const known = examples.get(row.story); + examples.set(row.story, { + story: row.story, + golden: sideWith(hunk.golden, row.goldenHash, diff.golden, known?.golden), + current: sideWith(hunk.current, row.imageHash, diff.current, known?.current), + }); + }); + } + const size = ({ examples }: Delta) => areaOf(examples[0]!.golden.boxes[0]!) + areaOf(examples[0]!.current.boxes[0]!); + return [...found] + .map(([key, { kind, examples }]): Delta => ({ key, kind, examples: [...examples.values()] })) + .sort((p, q) => q.examples.length - p.examples.length || size(q) - size(p) || (p.key < q.key ? -1 : 1)); +} + +const isUndecided = ({ row }: ScannedRow) => row.decision === null; + +/** What to offer: only undecided stories are affected, since an image with a decision has had its look. */ +export const offeredDeltas = (scans: readonly ScannedRow[], accepted: ReadonlySet): readonly Delta[] => + deltasIn(scans.filter(isUndecided), (key) => !accepted.has(key)).filter(({ examples }) => examples.length >= MIN_AFFECTED); + +export const acceptedDeltas = (scans: readonly ScannedRow[], accepted: ReadonlySet): readonly Delta[] => deltasIn(scans, (key) => accepted.has(key)); + +/** The undecided stories with nothing left once these are accepted, which is what an approval is for. */ +export const finishedBy = (scans: readonly ScannedRow[], accepted: ReadonlySet): readonly RenderRow[] => + scans.filter((scan) => isUndecided(scan) && scan.keys.length > 0 && scan.keys.every((key) => key !== null && accepted.has(key))).map(({ row }) => row); + +/** The hunks of a story that these say to accept. */ +export const hunksAccepted = (keys: readonly (string | null)[], accepted: ReadonlySet): readonly number[] => + keys.flatMap((key, index) => (key !== null && accepted.has(key) ? [index] : [])); + +/** Pixels to copy from one image onto the patched golden, at their own size. */ +export interface Blit { + readonly from: "golden" | "current"; + readonly box: BoundingBox; + readonly toX: number; + readonly toY: number; +} + +export interface GoldenPatch { + /** In order, each over the last. */ + readonly blits: readonly Blit[]; + /** The hunks left, against the patched golden, which is `diff.golden`'s size. */ + readonly diff: PixelDiff; + /** What the diff image marks that's no longer a difference, in the current image. */ + readonly settled: readonly BoundingBox[]; +} + +/** + * The golden with these hunks redrawn from the current image. Everything outside a hunk matches, so + * what differs afterward is exactly the other hunks, with no need to compare any pixels again, and + * accepting every one gives the current image itself. + */ +export function patchGolden(diff: PixelDiff, accepted: readonly number[]): GoldenPatch { + const taken = new Set(accepted); + const hunks = diff.hunks.filter((_, index) => taken.has(index)); + const splices = hunks.filter((hunk) => !sameSize(hunk)).sort((p, q) => p.golden.y - q.golden.y); + const width = splices.some(({ golden, current }) => golden.width !== current.width) ? diff.current.width : diff.golden.width; + + const blits: Blit[] = []; + let from = 0; + let to = 0; + const copyGolden = (height: number) => { + blits.push({ from: "golden", box: { x: 0, y: from, width: diff.golden.width, height }, toX: 0, toY: to }); + to += height; + }; + for (const { golden, current } of splices) { + copyGolden(golden.y - from); + blits.push({ from: "current", box: current, toX: 0, toY: to }); + to += current.height; + from = golden.y + golden.height; + } + copyGolden(diff.golden.height - from); + + // Where a row of the golden ends up, which only what's wholly above it can change. + const moved = (y: number) => splices.reduce((sum, { golden, current }) => (golden.y + golden.height <= y ? sum + current.height - golden.height : sum), y); + for (const hunk of hunks) { + if (sameSize(hunk)) blits.push({ from: "current", box: hunk.current, toX: hunk.golden.x, toY: moved(hunk.golden.y) }); + } + + const left = diff.hunks + .filter((_, index) => !taken.has(index)) + .map((hunk) => ({ + ...hunk, + golden: { ...hunk.golden, y: moved(hunk.golden.y) }, + moved: hunk.moved.map((pair) => ({ ...pair, golden: { ...pair.golden, y: moved(pair.golden.y) } })), + })); + const first = left[0]?.current; + return { + blits: blits.filter(({ box }) => !isEmptyBox(box)), + settled: hunks.map(({ current }) => (isEmptyBox(current) ? seamOf(current, diff.current) : current)), + diff: { + current: diff.current, + golden: { width, height: to }, + hunks: left, + changedPixels: left.reduce((sum, { changedPixels }) => sum + changedPixels, 0), + movedPixels: left.reduce((sum, hunk) => sum + hunk.moved.reduce((blocks, { current }) => blocks + areaOf(current), 0), 0), + // Only a hunk that's all spacing says how much it holds, so this leaves out any inside a changed hunk. + spacingPixels: left.reduce((sum, { kind, golden, current }) => (kind === "spacing" ? sum + Math.abs(areaOf(golden) - areaOf(current)) : sum), 0), + totalPixels: Math.max(diff.current.width * diff.current.height, width * to), + boundingBox: left.length === 0 ? null : left.map(({ current }) => current).reduce(hull), + firstChanged: first === undefined ? null : { x: first.x, y: first.y }, + // Which hunk had the largest isn't known, and saying nothing beats overstating it. + maxChannelDelta: 0, + }, + }; +} diff --git a/ui/src/shared/diff-image.ts b/ui/src/shared/diff-image.ts new file mode 100644 index 0000000..52b2941 --- /dev/null +++ b/ui/src/shared/diff-image.ts @@ -0,0 +1,23 @@ +// How a diff image is drawn, for the differ that draws one, and a page that redraws part of one. +import type { BoundingBox } from "./api.ts"; + +// An unchanged pixel fades into gray from this level to white, so the page stays readable +// behind the highlight and no color in it can be mistaken for one. +const FADED_FLOOR = 190; + +/** What didn't change, at any stride. */ +export function fade(from: ArrayLike, out: { [index: number]: number }, at: number): void { + const luma = (from[at]! * 77 + from[at + 1]! * 150 + from[at + 2]! * 29) >> 8; + const faded = FADED_FLOOR + ((luma * (255 - FADED_FLOOR)) >> 8); + out[at] = faded; + out[at + 1] = faded; + out[at + 2] = faded; +} + +/** The row marking where something was only taken away, for a hunk at this box of the current image. */ +export const seamOf = ({ x, y, width }: BoundingBox, image: { readonly width: number; readonly height: number }): BoundingBox => ({ + x: width === 0 ? 0 : x, + y: Math.min(y, image.height - 1), + width: width === 0 ? image.width : width, + height: 1, +}); diff --git a/ui/src/shared/hunks.ts b/ui/src/shared/hunks.ts index 14f397c..c3f5da1 100644 --- a/ui/src/shared/hunks.ts +++ b/ui/src/shared/hunks.ts @@ -10,7 +10,7 @@ export interface HunkGroup extends BoxPair { } /** Nothing changed but where things are, which a person still decides is right. */ -export const isLayoutOnly = ({ changedPixels }: PixelDiff) => changedPixels === 0; +export const isLayoutOnly = ({ changedPixels, hunks }: PixelDiff) => changedPixels === 0 && hunks.length > 0; const strongest = (p: HunkKind, q: HunkKind): HunkKind => (p === "changed" || q === "changed" ? "changed" : p === "moved" || q === "moved" ? "moved" : "spacing"); diff --git a/ui/test/deltas.test.ts b/ui/test/deltas.test.ts new file mode 100644 index 0000000..104a3c3 --- /dev/null +++ b/ui/test/deltas.test.ts @@ -0,0 +1,174 @@ +import { describe, expect, test } from "vitest"; +import type { ChangedHunk, PixelDiff, RenderRow } from "../src/shared/api.ts"; +import { type Comparable, type Delta, acceptedDeltas, canApply, finishedBy, hunksAccepted, MIN_AFFECTED, offeredDeltas, patchGolden, repeatedSizes, type ScannedRow } from "../src/shared/deltas.ts"; + +const box = (x: number, y: number, width: number, height: number) => ({ x, y, width, height }); + +const hunk = (golden: ChangedHunk["golden"], current: ChangedHunk["current"], changedPixels = 1): ChangedHunk => ({ golden, current, kind: "changed", changedPixels, moved: [] }); + +const diffOf = (hunks: readonly ChangedHunk[], golden = { width: 100, height: 200 }, current = golden): PixelDiff => ({ + golden, + current, + hunks, + changedPixels: hunks.reduce((sum, { changedPixels }) => sum + changedPixels, 0), + movedPixels: 0, + spacingPixels: 0, + totalPixels: 20_000, + boundingBox: null, + firstChanged: null, + maxChannelDelta: 9, +}); + +const scan = (story: string, keys: readonly (string | null)[], decision: RenderRow["decision"] = null): ScannedRow => ({ + row: { story, state: "changed", imageHash: `${story}-now`, goldenHash: `${story}-was`, failure: null, decision, pixelDiff: null, goldenStorybookUrl: null, goldenViewport: null } as Comparable, + diff: diffOf(keys.map((_, index) => hunk(box(0, index * 10, 4, 4), box(0, index * 10, 4, 4)))), + keys, +}); + +const storiesOf = (examples: Delta["examples"]) => examples.map(({ story }) => story); + +const approved = { decision: "approved", actor: "a", at: 1 } as const; +const rejected = { decision: "rejected", actor: "a", at: 1 } as const; + +describe("canApply", () => { + test("takes a hunk of one size anywhere, and one that changes height only across the whole image", () => { + const diff = diffOf([]); + expect(canApply(hunk(box(5, 5, 4, 4), box(5, 20, 4, 4)), diff)).toBe(true); + expect(canApply(hunk(box(0, 5, 100, 4), box(0, 5, 100, 9)), diff)).toBe(true); + expect(canApply(hunk(box(0, 5, 100, 15), box(0, 20, 100, 0)), diff)).toBe(true); + expect(canApply(hunk(box(5, 5, 4, 4), box(5, 5, 4, 9)), diff)).toBe(false); + }); +}); + +describe("repeatedSizes", () => { + test("counts a story once, however often it has that size", () => { + const twice = diffOf([hunk(box(0, 0, 4, 4), box(0, 0, 4, 4)), hunk(box(0, 9, 4, 4), box(0, 9, 4, 4))]); + expect(repeatedSizes([twice, twice])).toEqual(new Set()); + expect(repeatedSizes(Array.from({ length: MIN_AFFECTED }, () => twice))).toEqual(new Set(["4x4>4x4"])); + }); + + test("leaves out what can't be applied", () => { + const narrow = diffOf([hunk(box(5, 5, 4, 4), box(5, 5, 4, 9))]); + expect(repeatedSizes([narrow, narrow, narrow])).toEqual(new Set()); + }); +}); + +describe("offeredDeltas", () => { + test("offers only what enough stories share, most stories first", () => { + const scans = [scan("a", ["x", "y"]), scan("b", ["x", "y"]), scan("c", ["x", "y"]), scan("d", ["x"]), scan("e", ["z"]), scan("f", ["z"])]; + expect(offeredDeltas(scans, new Set()).map(({ key, examples }) => [key, storiesOf(examples)])).toEqual([ + ["x", ["a", "b", "c", "d"]], + ["y", ["a", "b", "c"]], + ]); + }); + + test("counts a story once when it has a delta twice", () => { + expect(offeredDeltas([scan("a", ["x", "x"]), scan("b", ["x"])], new Set())).toEqual([]); + }); + + test("never counts an image that has a decision, or offers what's accepted", () => { + const scans = [scan("a", ["x"]), scan("b", ["x"]), scan("c", ["x"], approved), scan("d", ["x"], rejected)]; + expect(offeredDeltas(scans, new Set())).toEqual([]); + expect(offeredDeltas([...scans, scan("e", ["x"])], new Set(["x"]))).toEqual([]); + expect(offeredDeltas([...scans, scan("e", ["x"])], new Set()).map(({ examples }) => storiesOf(examples))).toEqual([["a", "b", "e"]]); + }); + + test("says where each story has it, which may be different in each", () => { + const at = (story: string, y: number): ScannedRow => { + const placed = scan(story, ["x"]); + return { ...placed, diff: diffOf([hunk(box(0, y, 4, 4), box(0, y + 15, 4, 4))]) }; + }; + const [delta] = offeredDeltas([at("a", 10), at("b", 40), at("c", 70)], new Set()); + expect(delta?.examples.map(({ story, golden, current }) => [story, golden.hash, golden.boxes[0]?.y, current.hash, current.boxes[0]?.y])).toEqual([ + ["a", "a-was", 10, "a-now", 25], + ["b", "b-was", 40, "b-now", 55], + ["c", "c-was", 70, "c-now", 85], + ]); + }); + + test("gives every place a story has it, and counts that story once", () => { + const twice: ScannedRow = { ...scan("a", ["x", "x"]), diff: diffOf([hunk(box(0, 10, 4, 4), box(0, 10, 4, 4)), hunk(box(0, 90, 4, 4), box(0, 90, 4, 4))]) }; + const [delta] = offeredDeltas([twice, scan("b", ["x"]), scan("c", ["x"])], new Set()); + expect(delta?.examples[0]?.golden.boxes.map(({ y }) => y)).toEqual([10, 90]); + expect(delta?.examples).toHaveLength(3); + }); + + test("never offers a hunk that can't be applied", () => { + expect(offeredDeltas([scan("a", [null]), scan("b", [null]), scan("c", [null])], new Set())).toEqual([]); + }); +}); + +test("acceptedDeltas lists every story, whatever's been decided since", () => { + const scans = [scan("a", ["x"], approved), scan("b", ["x"]), scan("c", ["y"])]; + expect(acceptedDeltas(scans, new Set(["x"])).map(({ examples }) => storiesOf(examples))).toEqual([["a", "b"]]); +}); + +describe("finishedBy", () => { + test("is only the undecided images with nothing left", () => { + const scans = [scan("a", ["x"]), scan("b", ["x", "y"]), scan("c", ["x", null]), scan("d", ["x"], rejected), scan("e", ["x"], approved)]; + expect(finishedBy(scans, new Set(["x"])).map(({ story }) => story)).toEqual(["a"]); + expect(finishedBy(scans, new Set(["x", "y"])).map(({ story }) => story)).toEqual(["a", "b"]); + }); +}); + +test("hunksAccepted says which of a story's hunks", () => { + expect(hunksAccepted(["x", null, "y", "x"], new Set(["x"]))).toEqual([0, 3]); +}); + +describe("patchGolden", () => { + const header = hunk(box(0, 8, 100, 20), box(0, 8, 100, 35), 500); + const narrow = hunk(box(40, 100, 4, 4), box(40, 115, 4, 4), 3); + const gap = { ...hunk(box(0, 150, 100, 15), box(0, 165, 100, 0), 0), kind: "spacing" as const }; + const diff = diffOf([header, narrow, gap]); + + test("accepting nothing is the golden", () => { + const { blits, diff: left } = patchGolden(diff, []); + expect(left.golden).toEqual(diff.golden); + expect(blits).toEqual([{ from: "golden", box: box(0, 0, 100, 200), toX: 0, toY: 0 }]); + expect(left.hunks).toEqual(diff.hunks); + }); + + test("makes room for what grew, and moves what's left down with it", () => { + const { blits, diff: left, settled } = patchGolden(diff, [0]); + expect(blits).toEqual([ + { from: "golden", box: box(0, 0, 100, 8), toX: 0, toY: 0 }, + { from: "current", box: box(0, 8, 100, 35), toX: 0, toY: 8 }, + { from: "golden", box: box(0, 28, 100, 172), toX: 0, toY: 43 }, + ]); + expect(left.hunks.map(({ golden }) => golden)).toEqual([box(40, 115, 4, 4), box(0, 165, 100, 15)]); + expect(left.hunks.map(({ current }) => current)).toEqual([narrow.current, gap.current]); + expect(left.golden).toEqual({ width: 100, height: 215 }); + expect(left.changedPixels).toBe(3); + expect(left.spacingPixels).toBe(1500); + expect(left.firstChanged).toEqual({ x: 40, y: 115 }); + expect(settled).toEqual([header.current]); + }); + + test("puts a hunk of one size where the golden has it, as moved by what's above it", () => { + const { blits } = patchGolden(diff, [0, 1]); + expect(blits.at(-1)).toEqual({ from: "current", box: narrow.current, toX: 40, toY: 115 }); + }); + + test("takes out what's gone, and marks the seam it left as settled", () => { + const { blits, settled, diff: left } = patchGolden(diff, [2]); + expect(left.golden.height).toBe(185); + expect(blits).toEqual([ + { from: "golden", box: box(0, 0, 100, 150), toX: 0, toY: 0 }, + { from: "golden", box: box(0, 165, 100, 35), toX: 0, toY: 150 }, + ]); + expect(settled).toEqual([box(0, 165, 100, 1)]); + }); + + test("gives no bounding box or first change with nothing left", () => { + const { diff: left } = patchGolden(diff, [0, 1, 2]); + expect(left).toMatchObject({ hunks: [], changedPixels: 0, movedPixels: 0, spacingPixels: 0, boundingBox: null, firstChanged: null, maxChannelDelta: 0 }); + expect(left.golden).toEqual(left.current); + }); + + test("takes on the current image's width when a hunk is the whole of two widths", () => { + const whole = diffOf([hunk(box(0, 0, 100, 200), box(0, 0, 80, 200))], { width: 100, height: 200 }, { width: 80, height: 200 }); + const { blits, diff: left } = patchGolden(whole, [0]); + expect(left.golden).toEqual({ width: 80, height: 200 }); + expect(blits).toEqual([{ from: "current", box: box(0, 0, 80, 200), toX: 0, toY: 0 }]); + }); +}); diff --git a/ui/test/filter.test.ts b/ui/test/filter.test.ts index 7ddece7..7c157ff 100644 --- a/ui/test/filter.test.ts +++ b/ui/test/filter.test.ts @@ -73,7 +73,7 @@ test("a filter survives the URL, and a bad one falls back", () => { }); test("moved and spacing only is a changed row with nothing changed in it", () => { - const stats = (changedPixels: number) => ({ changedPixels }) as RenderRow["pixelDiff"]; + const stats = (changedPixels: number) => ({ changedPixels, hunks: [{ changedPixels }] }) as unknown as RenderRow["pixelDiff"]; const candidates = [ row("only-moved", { pixelDiff: stats(0) }), row("really-changed", { pixelDiff: stats(1) }),