diff --git a/nginx/default.conf b/nginx/default.conf index 9d010353..58f2bedb 100644 --- a/nginx/default.conf +++ b/nginx/default.conf @@ -5,6 +5,19 @@ server { return 403; } + # Screenshots, baselines and diffs. Their names are unique per upload and the + # bytes behind a name are never rewritten, so the browser may keep them: going + # back to a screen already reviewed then costs no request at all. + # + # This must not fall through to index.html the way the SPA route below does — + # a missing image would be answered with the app's HTML and then cached under + # the image's name for a year. + location /static/imageUploads/ { + root /usr/share/nginx/html; + add_header Cache-Control "public, max-age=31536000, immutable"; + try_files $uri =404; + } + location / { root /usr/share/nginx/html; index index.html index.htm; diff --git a/src/_helpers/imagePrefetch.helper.test.ts b/src/_helpers/imagePrefetch.helper.test.ts new file mode 100644 index 00000000..42191bfc --- /dev/null +++ b/src/_helpers/imagePrefetch.helper.test.ts @@ -0,0 +1,118 @@ +import { run } from "./testRun.fixture"; +import { neighbourImageNames, prefetchImages } from "./imagePrefetch.helper"; +import { staticService } from "../services"; + +const at = (index: number) => + run({ + id: `run-${index}`, + baselineName: `baseline-${index}.png`, + imageName: `image-${index}.png`, + diffName: `diff-${index}.png`, + }); + +const list = (length: number) => + Array.from({ length }, (_unused, index) => at(index)); + +describe("neighbourImageNames", () => { + it("asks for the run ahead before the one behind, since review moves forward", () => { + const names = neighbourImageNames(list(5), 2); + + expect(names.indexOf("image-3.png")).toBeLessThan( + names.indexOf("image-1.png"), + ); + }); + + it("leaves out the run being looked at — it is already loading", () => { + const names = neighbourImageNames(list(5), 2); + + expect(names).not.toContain("image-2.png"); + expect(names).not.toContain("baseline-2.png"); + expect(names).not.toContain("diff-2.png"); + }); + + it("takes all three pictures of a neighbour", () => { + const names = neighbourImageNames(list(3), 0); + + expect(names).toEqual( + expect.arrayContaining(["baseline-1.png", "image-1.png", "diff-1.png"]), + ); + }); + + it("stops at the start of the list", () => { + const names = neighbourImageNames(list(5), 0); + + expect(names).not.toContain("image-4.png"); + expect(names).toContain("image-1.png"); + }); + + it("stops at the end of the list", () => { + const names = neighbourImageNames(list(3), 2); + + expect(names).toEqual( + expect.arrayContaining(["image-1.png", "baseline-1.png"]), + ); + expect(names).toHaveLength(3); + }); + + it("skips a run that has no diff yet", () => { + const runs = [at(0), run({ id: "run-1", diffName: null as never })]; + + expect(neighbourImageNames(runs, 0)).not.toContain(null); + }); + + it("asks for a name shared by two runs only once", () => { + const shared = run({ id: "shared", baselineName: "same-baseline.png" }); + const runs = [at(0), { ...shared, id: "a" }, { ...shared, id: "b" }]; + + const names = neighbourImageNames(runs, 0); + + expect(names.filter((name) => name === "same-baseline.png")).toHaveLength( + 1, + ); + }); + + it("has nothing to fetch when the run is not in the list", () => { + expect(neighbourImageNames(list(3), -1)).toEqual([]); + }); +}); + +describe("prefetchImages", () => { + let requestedSrcs: string[]; + let originalImage: typeof window.Image; + + beforeEach(() => { + requestedSrcs = []; + originalImage = window.Image; + // jsdom will not load anything, but it does record what was asked for + window.Image = class { + set src(value: string) { + requestedSrcs.push(value); + } + } as unknown as typeof window.Image; + }); + + afterEach(() => { + window.Image = originalImage; + }); + + it("requests every name through the same URL the dialog will use", () => { + prefetchImages(["a.png", "b.png"], new Set()); + + expect(requestedSrcs).toEqual([ + staticService.getImage("a.png"), + staticService.getImage("b.png"), + ]); + }); + + it("does not ask twice for a name it already requested", () => { + const alreadyRequested = new Set(); + + prefetchImages(["a.png"], alreadyRequested); + prefetchImages(["a.png", "b.png"], alreadyRequested); + + expect(requestedSrcs).toEqual([ + staticService.getImage("a.png"), + staticService.getImage("b.png"), + ]); + }); +}); diff --git a/src/_helpers/imagePrefetch.helper.ts b/src/_helpers/imagePrefetch.helper.ts new file mode 100644 index 00000000..73e5c681 --- /dev/null +++ b/src/_helpers/imagePrefetch.helper.ts @@ -0,0 +1,62 @@ +import { TestRun } from "../types"; +import { staticService } from "../services"; + +// How far around the reviewed run the dialog warms the browser cache. Review +// moves forward, so it reaches further ahead than behind; each run costs three +// requests, which is why the window is small. +export const PREFETCH_AHEAD = 2; +export const PREFETCH_BEHIND = 1; + +/** + * The pictures of the runs on either side of the one being reviewed, nearest + * first and ahead before behind, so the run the arrow key lands on next is + * requested first. Deduped, and empty names (a run compared to no baseline, or + * not compared yet) are left out. + */ +export const neighbourImageNames = ( + testRuns: TestRun[], + index: number, +): string[] => { + if (index < 0 || index >= testRuns.length) { + return []; + } + + const offsets: number[] = []; + for ( + let step = 1; + step <= Math.max(PREFETCH_AHEAD, PREFETCH_BEHIND); + step++ + ) { + if (step <= PREFETCH_AHEAD) offsets.push(step); + if (step <= PREFETCH_BEHIND) offsets.push(-step); + } + + const names = new Set(); + offsets.forEach((offset) => { + const neighbour = testRuns[index + offset]; + if (!neighbour) return; + [neighbour.baselineName, neighbour.imageName, neighbour.diffName] + .filter((name): name is string => !!name) + .forEach((name) => names.add(name)); + }); + + return [...names]; +}; + +/** + * Warms the browser cache for the given image names, through the very URL the + * dialog will ask for, so arrow navigation shows a picture instead of a + * spinner. `alreadyRequested` is carried by the caller across renders — a + * second request for the same name would be wasted work. + */ +export const prefetchImages = ( + names: string[], + alreadyRequested: Set, +): void => { + names.forEach((name) => { + if (alreadyRequested.has(name)) return; + alreadyRequested.add(name); + const image = new Image(); + image.src = staticService.getImage(name); + }); +}; diff --git a/src/components/TestDetailsDialog/index.tsx b/src/components/TestDetailsDialog/index.tsx index 078a7d64..9037fad6 100644 --- a/src/components/TestDetailsDialog/index.tsx +++ b/src/components/TestDetailsDialog/index.tsx @@ -7,6 +7,10 @@ import { BaseModal } from "../BaseModal"; import TestDetailsModal from "./TestDetailsModal"; import { TestRun } from "../../types"; import { makeStyles } from "@mui/styles"; +import { + neighbourImageNames, + prefetchImages, +} from "../../_helpers/imagePrefetch.helper"; const useStyles = makeStyles(() => ({ modal: { @@ -48,6 +52,17 @@ export const TestDetailsDialog: React.FunctionComponent = () => { [testRuns, selectedTestRun?.id], ); + // Screenshots are megapixels each, so stepping to the next run used to leave + // the pane blank under a "Loading..." while the browser fetched and decoded + // them. Fetch the neighbours' pictures while the reviewer looks at this one. + const prefetched = React.useRef>(new Set()); + React.useEffect(() => { + prefetchImages( + neighbourImageNames(testRuns, selectedTestRunIndex), + prefetched.current, + ); + }, [testRuns, selectedTestRunIndex]); + const navigateById = React.useCallback( (id?: string) => { if (touched) {