From a76ad146c0cf958b7bd74dc88dd65609b35d6b4b Mon Sep 17 00:00:00 2001 From: marcelopenas Date: Mon, 9 Nov 2026 06:59:36 -0300 Subject: [PATCH 1/5] feat: manage product images through authorized upload contracts --- .env.example | 2 + .github/workflows/ci.yml | 5 +- Dockerfile | 2 + src/app/globals.css | 9 + src/components/admin-view.tsx | 23 +++ src/components/product-card.tsx | 4 +- src/components/product-image-dialog.tsx | 250 ++++++++++++++++++++++++ src/components/product-visual.tsx | 38 ++++ src/lib/contracts.test.ts | 26 +++ src/lib/contracts.ts | 19 ++ tests/images.spec.ts | 114 +++++++++++ 11 files changed, 489 insertions(+), 3 deletions(-) create mode 100644 src/components/product-image-dialog.tsx create mode 100644 src/components/product-visual.tsx create mode 100644 tests/images.spec.ts diff --git a/.env.example b/.env.example index 90c1753..ddeddb9 100644 --- a/.env.example +++ b/.env.example @@ -1,6 +1,8 @@ NEXT_PUBLIC_API_BASE_URL=/api NEXT_PUBLIC_DISPLAY_TIME_ZONE=UTC NEXT_PUBLIC_APP_ENV=development +NEXT_PUBLIC_ENABLE_PRODUCT_IMAGE_UPLOADS=false +NEXT_PUBLIC_IMAGE_UPLOAD_ORIGINS= NEXT_PUBLIC_APP_VERSION=local NEXT_PUBLIC_ENABLE_TELEMETRY=false NEXT_PUBLIC_FARO_URL= diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 10efcc6..0c2afc6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,7 +2,7 @@ name: Frontend checks on: pull_request: push: - branches: [main, feature/frontend] + branches: [main, feature/frontend, feature/product-images] permissions: contents: read jobs: @@ -20,6 +20,9 @@ jobs: - run: npm run typecheck - run: npm test - run: npm run build + env: + NEXT_PUBLIC_ENABLE_PRODUCT_IMAGE_UPLOADS: "true" + NEXT_PUBLIC_APP_ENV: fixture - run: npx playwright install --with-deps chromium - run: npm run test:e2e - uses: actions/upload-artifact@v4 diff --git a/Dockerfile b/Dockerfile index 47040d0..4f8eac0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -6,6 +6,8 @@ COPY . . ARG NEXT_PUBLIC_API_BASE_URL=/api ARG NEXT_PUBLIC_DISPLAY_TIME_ZONE=UTC ARG NEXT_PUBLIC_APP_ENV=development +ARG NEXT_PUBLIC_ENABLE_PRODUCT_IMAGE_UPLOADS=false +ARG NEXT_PUBLIC_IMAGE_UPLOAD_ORIGINS ARG NEXT_PUBLIC_APP_VERSION=local ARG NEXT_PUBLIC_ENABLE_TELEMETRY=false ARG NEXT_PUBLIC_FARO_URL diff --git a/src/app/globals.css b/src/app/globals.css index 2e0ddf6..546137d 100644 --- a/src/app/globals.css +++ b/src/app/globals.css @@ -221,6 +221,7 @@ min-width: 0; } .product-tile { + overflow: hidden; width: 100%; min-width: 0; aspect-ratio: 1.2; @@ -234,6 +235,14 @@ border-radius: 4px; transition: background 0.15s; } +input[type="file"] { + width: 100%; + min-height: 44px; +} +input[type="file"]::file-selector-button { + min-height: 44px; + cursor: pointer; +} .product-tile:hover { background: color-mix(in srgb, var(--tile), var(--accent) 12%); } diff --git a/src/components/admin-view.tsx b/src/components/admin-view.tsx index 9939113..a95214a 100644 --- a/src/components/admin-view.tsx +++ b/src/components/admin-view.tsx @@ -41,6 +41,7 @@ import { type Product, } from "@/lib/contracts"; import "./admin.css"; +import { ProductImageDialog } from "./product-image-dialog"; type Screen = "overview" | "products" | "accounts" | "observability"; const features = tableFeatures({ @@ -321,6 +322,7 @@ function Products() { const [search, setSearch] = useState(""); const [create, setCreate] = useState(false); const [remove, setRemove] = useState(null); + const [imageProduct, setImageProduct] = useState(null); const query = useQuery({ queryKey: ["products"], queryFn: ({ signal }) => api("/products", productsSchema, { signal }), @@ -351,6 +353,15 @@ function Products() { + + + + )} + + + + {product.image && ( + + )} + + + + + ); +} diff --git a/src/components/product-visual.tsx b/src/components/product-visual.tsx new file mode 100644 index 0000000..e5b85cd --- /dev/null +++ b/src/components/product-visual.tsx @@ -0,0 +1,38 @@ +"use client"; +import Image from "next/image"; +import { useState } from "react"; +import { type Product } from "@/lib/contracts"; + +export function ProductVisual({ + product, + priority = false, +}: { + product: Product; + priority?: boolean; +}) { + const [failed, setFailed] = useState(null); + const image = product.image; + return image && failed !== image.url ? ( + {image.alt} setFailed(image.url)} + style={{ + width: "100%", + height: "100%", + aspectRatio: "1.2", + objectFit: "cover", + borderRadius: 4, + }} + /> + ) : ( + <> + + Product image unavailable + + ); +} diff --git a/src/lib/contracts.test.ts b/src/lib/contracts.test.ts index 9e7552d..019a60d 100644 --- a/src/lib/contracts.test.ts +++ b/src/lib/contracts.test.ts @@ -6,6 +6,32 @@ import { z } from "zod"; afterEach(() => vi.unstubAllGlobals()); const id = "00000000-0000-4000-8000-000000000001"; describe("service contracts", () => { + it("accepts legacy products and validates optional image metadata", () => { + const product = { id, name: "Fixture", price: 1, unit: "item" }; + expect(productSchema.safeParse(product).success).toBe(true); + const image = { + url: "https://images.example.test/version.png", + alt: "Fixture image", + width: 320, + height: 240, + }; + expect(productSchema.safeParse({ ...product, image }).success).toBe(true); + for (const url of [ + "javascript:alert(1)", + "http://external.test/image", + "//external.test/image", + "/media/\\\\external.test", + ]) { + expect( + productSchema.safeParse({ ...product, image: { ...image, url } }) + .success, + ).toBe(false); + } + expect( + productSchema.safeParse({ ...product, image: { ...image, width: 0 } }) + .success, + ).toBe(false); + }); it("rejects invalid quantities recovered from browser storage", () => { for (const quantity of [0, -1, 1.5, 1000]) expect(cartSchema.safeParse([{ id, quantity }]).success).toBe(false); diff --git a/src/lib/contracts.ts b/src/lib/contracts.ts index ffcda70..79dbbc0 100644 --- a/src/lib/contracts.ts +++ b/src/lib/contracts.ts @@ -1,10 +1,29 @@ import { z } from "zod"; +export const imageSchema = z.object({ + url: z.string().refine((value) => { + if ( + /^\/(media|api\/products)\/[^\\\\]*$/.test(value) && + !value.includes("//") + ) + return true; + try { + return new URL(value).protocol === "https:"; + } catch { + return false; + } + }, "Use an HTTPS image URL or a /media/ path."), + alt: z.string().trim().min(1).max(200), + width: z.number().int().positive().max(4096), + height: z.number().int().positive().max(4096), +}); + export const productSchema = z.object({ id: z.uuid(), name: z.string().min(1), price: z.number().nonnegative(), unit: z.string().min(1), + image: imageSchema.nullish(), }); export const productsSchema = z.array(productSchema); export type Product = z.infer; diff --git a/tests/images.spec.ts b/tests/images.spec.ts new file mode 100644 index 0000000..3db20ca --- /dev/null +++ b/tests/images.spec.ts @@ -0,0 +1,114 @@ +import { test, expect } from "@playwright/test"; +import { readFile } from "node:fs/promises"; +import { mockApi, products } from "./fixtures"; +import type { Product } from "../src/lib/contracts"; + +test("admin uploads, replaces, edits alternative text and removes a product image", async ({ + page, +}) => { + await mockApi(page, "admin"); + let product: Product = { ...products[0] }; + let version = 0; + const png = await readFile("public/brand.png"); + const uploadId = "00000000-0000-4000-8000-000000000555"; + await page.route("**/api/**", async (route) => { + const request = route.request(); + const path = new URL(request.url()).pathname; + if (path === "/api/products" && request.method() === "GET") + return route.fulfill({ json: [product, products[1]] }); + if (path.includes("/fixture-media-upload/")) { + expect(request.method()).toBe("PUT"); + expect(request.headers()["content-type"]).toBe("image/png"); + expect(request.headers()["if-none-match"]).toBe("*"); + return route.fulfill({ status: 204 }); + } + if (path.endsWith("/image/uploads")) + return route.fulfill({ + json: { + uploadId, + uploadHeaders: { "If-None-Match": "*" }, + uploadUrl: new URL( + "/api/fixture-media-upload/" + uploadId, + request.url(), + ).href, + }, + }); + if (path.endsWith("/image/content")) + return route.fulfill({ body: png, contentType: "image/png" }); + if (path.endsWith("/image")) { + if (request.method() === "DELETE") product = { ...product, image: null }; + else if (request.method() === "PATCH") + product = { + ...product, + image: { ...product.image!, alt: request.postDataJSON().alt }, + }; + else + product = { + ...product, + image: { + url: + "/api/products/" + product.id + "/image/content?v=" + ++version, + alt: request.postDataJSON().alt, + width: 697, + height: 345, + }, + }; + return route.fulfill({ json: product }); + } + return route.fallback(); + }); + await page.goto("/admin/products/"); + const manage = () => + page + .getByRole("row") + .filter({ hasText: "Fixture Tea" }) + .getByRole("button", { name: "Manage image" }); + await manage().click(); + await expect(page.getByRole("button", { name: "Save image" })).toBeDisabled(); + await page.getByLabel("Image file", { exact: true }).setInputFiles({ + name: "invalid.txt", + mimeType: "text/plain", + buffer: Buffer.from("not an image"), + }); + await expect( + page.getByText("Choose a JPEG or PNG file up to 5 MB."), + ).toBeVisible(); + for (const alt of [ + "Project logo upload fixture", + "Replacement logo fixture", + ]) { + await page + .getByLabel("Image file", { exact: true }) + .setInputFiles("public/brand.png"); + await page.getByLabel("Alternative text", { exact: true }).fill(alt); + await page.getByRole("button", { name: "Save image", exact: true }).click(); + await expect(page.getByRole("dialog")).toHaveCount(0); + await manage().click(); + await expect( + page.getByRole("img", { name: alt, exact: true }), + ).toBeVisible(); + } + await page + .getByLabel("Alternative text", { exact: true }) + .fill("Updated alternative text"); + await page.getByRole("button", { name: "Save image", exact: true }).click(); + await expect(page.getByRole("dialog")).toHaveCount(0); + await manage().click(); + await expect( + page.getByRole("img", { name: "Updated alternative text", exact: true }), + ).toBeVisible(); + await page.getByRole("button", { name: "Remove image", exact: true }).click(); + await page.getByRole("button", { name: "Keep image", exact: true }).click(); + await expect( + page.getByRole("button", { name: "Confirm removal" }), + ).toHaveCount(0); + await page.getByRole("button", { name: "Remove image", exact: true }).click(); + await page + .getByRole("button", { name: "Confirm removal", exact: true }) + .click(); + await expect(page.getByRole("dialog")).toHaveCount(0); + await manage().click(); + await expect( + page.getByRole("img", { name: "Updated alternative text", exact: true }), + ).toHaveCount(0); +}); From e5303d6b63ec87d700a606e927c14a4d39af5ec4 Mon Sep 17 00:00:00 2001 From: marcelopenas Date: Mon, 9 Nov 2026 07:01:57 -0300 Subject: [PATCH 2/5] test: retain product image upload screenshots --- .github/workflows/ci.yml | 6 ++++-- tests/images.spec.ts | 17 ++++++++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0c2afc6..6407506 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,7 +26,9 @@ jobs: - run: npx playwright install --with-deps chromium - run: npm run test:e2e - uses: actions/upload-artifact@v4 - if: failure() + if: always() with: name: browser-test-report - path: playwright-report + path: | + playwright-report + test-results diff --git a/tests/images.spec.ts b/tests/images.spec.ts index 3db20ca..1eafe57 100644 --- a/tests/images.spec.ts +++ b/tests/images.spec.ts @@ -5,7 +5,7 @@ import type { Product } from "../src/lib/contracts"; test("admin uploads, replaces, edits alternative text and removes a product image", async ({ page, -}) => { +}, testInfo) => { await mockApi(page, "admin"); let product: Product = { ...products[0] }; let version = 0; @@ -87,6 +87,21 @@ test("admin uploads, replaces, edits alternative text and removes a product imag await expect( page.getByRole("img", { name: alt, exact: true }), ).toBeVisible(); + await expect + .poll(() => + page + .getByRole("img", { name: alt, exact: true }) + .evaluate((element) => (element as HTMLImageElement).naturalWidth), + ) + .toBeGreaterThan(0); + if (alt === "Project logo upload fixture") { + const screenshot = testInfo.outputPath("image-dialog.png"); + await page.screenshot({ path: screenshot }); + await testInfo.attach("Image dialog", { + path: screenshot, + contentType: "image/png", + }); + } } await page .getByLabel("Alternative text", { exact: true }) From 45a1cea203eb21cc92094439661d271d62756440 Mon Sep 17 00:00:00 2001 From: marcelopenas Date: Mon, 9 Nov 2026 07:03:27 -0300 Subject: [PATCH 3/5] test: check image flows for uncaught browser errors --- tests/images.spec.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/images.spec.ts b/tests/images.spec.ts index 1eafe57..c2cb206 100644 --- a/tests/images.spec.ts +++ b/tests/images.spec.ts @@ -7,6 +7,8 @@ test("admin uploads, replaces, edits alternative text and removes a product imag page, }, testInfo) => { await mockApi(page, "admin"); + const pageErrors: string[] = []; + page.on("pageerror", (error) => pageErrors.push(error.message)); let product: Product = { ...products[0] }; let version = 0; const png = await readFile("public/brand.png"); @@ -126,4 +128,10 @@ test("admin uploads, replaces, edits alternative text and removes a product imag await expect( page.getByRole("img", { name: "Updated alternative text", exact: true }), ).toHaveCount(0); + await page.getByRole("button", { name: "Cancel", exact: true }).click(); + await expect(page.getByRole("dialog")).toHaveCount(0); + await manage().click(); + await page.getByRole("dialog").press("Escape"); + await expect(page.getByRole("dialog")).toHaveCount(0); + expect(pageErrors).toEqual([]); }); From 44ef0ca616a3da48f9167108581e1f1d76061039 Mon Sep 17 00:00:00 2001 From: marcelopenas Date: Mon, 9 Nov 2026 07:43:03 -0300 Subject: [PATCH 4/5] fix: keep transparent product images visible in both themes --- src/components/product-image-dialog.tsx | 7 ++++++- src/components/product-visual.tsx | 3 ++- tests/images.spec.ts | 27 ++++++++++++++++++++++++- 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/components/product-image-dialog.tsx b/src/components/product-image-dialog.tsx index 4f4bbe1..96a523d 100644 --- a/src/components/product-image-dialog.tsx +++ b/src/components/product-image-dialog.tsx @@ -177,7 +177,12 @@ export function ProductImageDialog({ width={480} height={320} unoptimized - style={{ width: "100%", height: 240, objectFit: "contain" }} + style={{ + width: "100%", + height: 240, + objectFit: "contain", + backgroundColor: "#6b7065", + }} /> )} diff --git a/src/components/product-visual.tsx b/src/components/product-visual.tsx index e5b85cd..0623ca0 100644 --- a/src/components/product-visual.tsx +++ b/src/components/product-visual.tsx @@ -25,7 +25,8 @@ export function ProductVisual({ width: "100%", height: "100%", aspectRatio: "1.2", - objectFit: "cover", + objectFit: priority ? "contain" : "cover", + backgroundColor: "#6b7065", borderRadius: 4, }} /> diff --git a/tests/images.spec.ts b/tests/images.spec.ts index c2cb206..4d79eb8 100644 --- a/tests/images.spec.ts +++ b/tests/images.spec.ts @@ -98,7 +98,7 @@ test("admin uploads, replaces, edits alternative text and removes a product imag .toBeGreaterThan(0); if (alt === "Project logo upload fixture") { const screenshot = testInfo.outputPath("image-dialog.png"); - await page.screenshot({ path: screenshot }); + await page.screenshot({ path: screenshot, animations: "disabled" }); await testInfo.attach("Image dialog", { path: screenshot, contentType: "image/png", @@ -133,5 +133,30 @@ test("admin uploads, replaces, edits alternative text and removes a product imag await manage().click(); await page.getByRole("dialog").press("Escape"); await expect(page.getByRole("dialog")).toHaveCount(0); + await page.getByRole("combobox", { name: "Color theme" }).click(); + await page.getByRole("option", { name: "Dark", exact: true }).click(); + await manage().click(); + await page + .getByLabel("Image file", { exact: true }) + .setInputFiles("public/brand.png"); + await page + .getByLabel("Alternative text", { exact: true }) + .fill("Dark theme upload fixture"); + await expect( + page.getByRole("img", { name: "Dark theme upload fixture", exact: true }), + ).toBeVisible(); + const darkScreenshot = testInfo.outputPath("image-dialog-dark.png"); + await page.screenshot({ path: darkScreenshot, animations: "disabled" }); + await testInfo.attach("Dark image dialog", { + path: darkScreenshot, + contentType: "image/png", + }); + expect( + await page.evaluate( + () => document.documentElement.scrollWidth <= window.innerWidth, + ), + ).toBe(true); + await page.getByRole("button", { name: "Cancel", exact: true }).click(); + await expect(page.getByRole("dialog")).toHaveCount(0); expect(pageErrors).toEqual([]); }); From 0453c698e282ac1fa72c3eed6ef5f9f5ace20e71 Mon Sep 17 00:00:00 2001 From: marcelopenas Date: Mon, 9 Nov 2026 07:48:37 -0300 Subject: [PATCH 5/5] fix: clarify the native image picker action --- src/app/globals.css | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/app/globals.css b/src/app/globals.css index 546137d..cab844a 100644 --- a/src/app/globals.css +++ b/src/app/globals.css @@ -242,6 +242,19 @@ input[type="file"] { input[type="file"]::file-selector-button { min-height: 44px; cursor: pointer; + border: 1px solid var(--line); + border-radius: 6px; + padding: 8px 12px; + margin-right: 12px; + background: var(--paper); + color: var(--ink); + font-weight: 600; +} +input[type="file"]::file-selector-button:hover { + background: var(--tile); +} +input[type="file"]::file-selector-button:active { + transform: translateY(1px); } .product-tile:hover { background: color-mix(in srgb, var(--tile), var(--accent) 12%);