feat(dashboard): Focal point editor in shared asset preview dialog

AGENTS.md

@@ -49,3 +49,19 @@ CI=true VITE_TEST_PORT=5176 bunx playwright test --config e2e/playwright.config.
 ## Gotchas
 
 - **Dashboard stale build**: `packages/dev-server/dist/` accumulates stale Vite build artifacts across branch switches. Vite doesn't clean old hashed files, so old chunks can interfere (e.g. overwriting `window.schemaInfo`). Always `rm -rf packages/dev-server/dist` before rebuilding. Build with `bunx vite build --base /dashboard/ --outDir ../dev-server/dist` from `packages/dashboard/`. Also check no stale Vite dev server is running on port 5173 — `DashboardPlugin` auto-proxies to it instead of serving static files.
+- **Don't take port 3010**: this repo's `dev-config.ts` uses `API_PORT = 3010` in the user's local working copy. Killing or starting another process on 3010 stomps the long-running dev backend. For ad-hoc test setups use a different port or rely on playwright's e2e env on 3050.
+- **Branch hygiene**: never run `git checkout HEAD -- file` on a branch that has uncommitted feature work without committing first — on a fresh branch HEAD == master, so the working copy is wiped.
+
+## Claude workflow for Vendure tickets
+
+Apply for every OSS/PDEV ticket that produces a code change. Skip the steps that don't apply (e.g. no UI to smoke), but never skip silently — say so.
+
+1. **Plan first.** For anything beyond a one-line fix, show the diff + test plan and wait for explicit OK before writing code.
+2. **Reviewers.** Spawn `nigel:nigel` and `vendurebot:vendurebot` in parallel on the proposed diff. Apply HIGH and MEDIUM feedback before commit; document why anything was skipped.
+3. **Tests are mandatory.** Every PR ships with at least one automated regression. Default location: `packages/dashboard/e2e/tests/regression/issue-<NNNN>-<slug>.spec.ts`. The test must pass on the branch and fail on master — verify both. For prod-only behaviour (e.g. lingui warnings) or component-level edge cases not exercised by e2e, document the gap and the manual repro steps.
+4. **Manual smoke for UI.** Drive the user's running dashboard on `localhost:5174` via `chrome-devtools` MCP (not a self-started backend). Clean up any test data through the same flow.
+5. **Commit only after explicit OK from the user.** A failing test, a passing test, a successful manual smoke — none of those imply permission to commit. Wait.
+6. **One commit per logical change.** Use the existing conventional-commit shape (`feat(dashboard):`, `fix(dashboard):`, `test(dashboard):`). Body explains *why*; ends with `Fixes #<gh-number>`.
+7. **Push + open PR** after commit. PR body lists: summary, root cause, change, test plan with what passed automatically vs manually, and any follow-ups out of scope.
+8. **Linear sync.** Move the ticket to `In Review`, post a comment with the PR link, a summary of the fix, and the verification (what tests, what smoke). Mirror any follow-ups as a separate comment.
+9. **Recover, don't paper over.** If something breaks (lost stash, killed port, polluted DB), say so up front and propose the recovery path before continuing.

packages/dashboard/e2e/global-setup.ts

@@ -1,3 +1,4 @@
+import type { AssetStorageStrategy } from '@vendure/core';
 import { mergeConfig } from '@vendure/core';
 import {
     createTestEnvironment,
@@ -7,6 +8,7 @@ import {
 } from '@vendure/testing';
 import { mkdirSync, writeFileSync } from 'node:fs';
 import path from 'node:path';
+import { Readable, Stream, Writable } from 'node:stream';
 import { fileURLToPath, pathToFileURL } from 'node:url';
 
 import { VENDURE_PORT } from './constants.js';
@@ -17,6 +19,49 @@ const __dirname = path.dirname(fileURLToPath(import.meta.url));
 
 registerInitializer('sqljs', new SqljsInitializer(path.join(__dirname, '__data__')));
 
+/**
+ * Minimal in-memory storage strategy used only by the dashboard e2e suite.
+ * Emits a parseable absolute URL so VendureImage's `new URL(asset.preview)`
+ * does not throw on assets created during tests. No bytes are persisted and
+ * no real HTTP server (and no AssetServerPlugin) is required.
+ *
+ * This duplicates a little of `TestingAssetStorageStrategy` rather than
+ * subclassing it because that class is not part of `@vendure/testing`'s public
+ * API — the only behaviour we need to change is `toAbsoluteUrl` returning a
+ * URL that `new URL(...)` can parse.
+ */
+class E2eAssetStorageStrategy implements AssetStorageStrategy {
+    toAbsoluteUrl(_req: unknown, identifier: string) {
+        return `http://test-asset.local/${identifier}`;
+    }
+    writeFileFromBuffer(fileName: string) {
+        return Promise.resolve(`test-assets/${fileName}`);
+    }
+    writeFileFromStream(fileName: string, data: Stream) {
+        return new Promise<string>((resolve, reject) => {
+            const w = new Writable({ write: (_c, _e, cb) => cb() });
+            data.on('error', reject);
+            data.pipe(w);
+            w.on('finish', () => resolve(`test-assets/${fileName}`));
+            w.on('error', reject);
+        });
+    }
+    readFileToBuffer() {
+        return Promise.resolve(Buffer.alloc(0));
+    }
+    readFileToStream() {
+        const s = new Readable();
+        s.push(null);
+        return Promise.resolve(s);
+    }
+    fileExists() {
+        return Promise.resolve(false);
+    }
+    deleteFile() {
+        return Promise.resolve();
+    }
+}
+
 /**
  * Compiles a TypeScript fixture with SWC so that NestJS parameter decorators
  * and emitDecoratorMetadata work correctly. Playwright's built-in transpiler
@@ -55,6 +100,22 @@ export default async function globalSetup() {
         paymentOptions: {
             paymentMethodHandlers: e2ePaymentMethodHandlers,
         },
+        // The default test-asset storage strategy emits a non-parseable
+        // `test-url/test-assets/...` placeholder that `VendureImage` cannot
+        // parse with `new URL(...)`, which crashes any page showing a real
+        // asset. The minimal strategy below emits a parseable absolute URL —
+        // all the asset-preview tests actually require — without pulling in
+        // AssetServerPlugin (not in the dashboard-e2e build scope on CI).
+        assetOptions: {
+            assetStorageStrategy: new E2eAssetStorageStrategy(),
+        },
+        // Point the CSV asset importer at the core e2e fixture images so the
+        // seeded products (e.g. "Laptop") get a real featured asset. This lets
+        // asset-dependent tests use a seeded product directly instead of
+        // uploading one at runtime.
+        importExportOptions: {
+            importAssetsDir: path.join(__dirname, '../../core/e2e/fixtures/assets'),
+        },
         plugins: [CustomHistoryEntryPlugin],
         customFields: e2eCustomFields,
     });

packages/dashboard/e2e/tests/catalog/custom-fields.spec.ts

@@ -108,7 +108,10 @@ test.describe('Custom Fields', () => {
 
         await dp.fillInput('Product name', 'Nullable Feature Type Product');
         await expect(dp.formItem('Slug').getByRole('textbox')).not.toHaveValue('', { timeout: 5_000 });
-        await expect(dp.formItem('Feature Type').getByRole('combobox')).toHaveAttribute('data-placeholder', '');
+        await expect(dp.formItem('Feature Type').getByRole('combobox')).toHaveAttribute(
+            'data-placeholder',
+            '',
+        );
         await expect(dp.createButton).toBeEnabled({ timeout: 10_000 });
 
         await dp.clickCreate();

packages/dashboard/e2e/tests/regression/issue-4722-asset-focal-point-in-preview-dialog.spec.ts

@@ -0,0 +1,117 @@
+import { expect, test } from '@playwright/test';
+
+import { VENDURE_PORT } from '../../constants.js';
+
+// #4722 — The focal-point editor was only available on the standalone Asset
+// detail route. The fix wires it through the shared `AssetPreview` (and
+// therefore the dialog used by Product / Variant detail pages), with a
+// callback up to `EntityAssets` so re-opening the dialog after a save shows
+// the new value rather than the stale one from the parent detail query.
+//
+// Setup: the seeded "Laptop" product already has a featured asset (imported
+// from the core e2e fixtures via `importAssetsDir` in global-setup), so the
+// test reads its id and drives the UI — no runtime asset upload or product
+// creation. `beforeEach` resets the asset's focal point to null so every run
+// (including a Playwright retry, which reuses the global-setup DB) starts from
+// the deterministic "Not set" state the assertions below rely on.
+//
+// Coverage note: this exercises the featured-asset path. The multi-asset
+// gallery / prev-next sync that `onAssetUpdated` also feeds is not covered here
+// because the seeded product has a single asset; adding it would require
+// re-introducing the runtime multi-asset seeding this rework removed.
+test.describe('Issue 4722 — focal point editor in shared asset preview dialog', () => {
+    let productId: string;
+
+    test.beforeEach(async ({ page }) => {
+        await page.goto('/');
+        productId = await page.evaluate(async vendurePort => {
+            const apiUrl = `http://localhost:${vendurePort}/admin-api`;
+            const sessionToken = localStorage.getItem('vendure-session-token');
+            if (!sessionToken) throw new Error('No vendure-session-token');
+            const post = async (query: string, variables?: Record<string, unknown>) => {
+                const res = await fetch(apiUrl, {
+                    method: 'POST',
+                    credentials: 'include',
+                    headers: {
+                        'content-type': 'application/json',
+                        authorization: `Bearer ${sessionToken}`,
+                    },
+                    body: JSON.stringify({ query, variables }),
+                });
+                const json = await res.json();
+                if (json.errors?.length) throw new Error(`Admin API: ${JSON.stringify(json.errors)}`);
+                return json.data;
+            };
+
+            const { product } = await post(`{ product(slug: "laptop") { id featuredAsset { id } } }`);
+            if (!product?.featuredAsset) {
+                // The featured asset comes from the "Laptop" row of
+                // core/e2e/fixtures/e2e-products-full.csv being imported via
+                // `importExportOptions.importAssetsDir` in global-setup. If this
+                // throws, that CSV row lost its asset, the fixture image is gone,
+                // or e2e/__data__ holds a stale pre-import DB (delete it to reseed).
+                throw new Error(
+                    'Seeded "laptop" product has no featured asset — check core e2e CSV / importAssetsDir / stale __data__ DB',
+                );
+            }
+
+            // Reset to a clean "Not set" state so the assertions start from a
+            // known baseline regardless of any previous (retried) run.
+            await post(`mutation($input: UpdateAssetInput!) { updateAsset(input: $input) { id } }`, {
+                input: { id: product.featuredAsset.id, focalPoint: null },
+            });
+
+            return product.id as string;
+        }, VENDURE_PORT);
+    });
+
+    test('should let the user set a focal point from the preview dialog and persist across re-open', async ({
+        page,
+    }) => {
+        test.setTimeout(45_000);
+
+        await page.goto(`/products/${productId}`);
+
+        // The Assets PageBlock in EntityAssets renders the featured asset
+        // inside a `<div data-testid="entity-assets-featured">` wrapper. Target
+        // its <img> directly so the test doesn't depend on the asset URL scheme.
+        const featuredImage = page.getByTestId('entity-assets-featured').locator('img');
+        await expect(featuredImage).toBeVisible({ timeout: 15_000 });
+        await featuredImage.click();
+
+        // The preview dialog opens with the new "Set focal point" button.
+        const setFocalPointTrigger = page.getByTestId('asset-preview-set-focal-point');
+        await expect(setFocalPointTrigger).toBeVisible({ timeout: 5_000 });
+
+        // Baseline: the asset has no focal point, so the readout shows "Not set".
+        // This makes the transition below a real state change rather than an
+        // assertion that could pass against a pre-existing value.
+        const focalPointValue = page.getByTestId('asset-preview-focal-point-value');
+        await expect(focalPointValue).toContainText('Not set');
+
+        // Activate the focal-point editor and confirm at the default centre
+        // position the editor renders for an asset without a saved focal point.
+        // (The exact dragged coordinate is not asserted: the editor's drag math
+        // is screen-pixels / natural-image-pixels, so a pixel-precise Playwright
+        // drag would be fragile. The Not-set → set → persist transition is the
+        // load-bearing regression signal.)
+        await setFocalPointTrigger.click();
+        await page.getByTestId('asset-focal-point-editor-confirm').click();
+
+        // After the mutation, the coords readout updates (the toast assertion
+        // is skipped — sonner auto-dismisses too quickly to race against
+        // reliably and the coords readout is the load-bearing signal).
+        await expect(focalPointValue).toContainText('0.50, 0.50', { timeout: 10_000 });
+
+        // Close the dialog (Escape) and re-open — the indicator must still
+        // show the saved coords, not regress to the stale parent value.
+        // Before the parent-sync fix, EntityAssets' local `assets` array would
+        // still hold the pre-save focal point, so the re-opened dialog would
+        // misreport "Not set".
+        await page.keyboard.press('Escape');
+        await expect(page.getByRole('dialog')).toBeHidden({ timeout: 5_000 });
+
+        await featuredImage.click();
+        await expect(focalPointValue).toContainText('0.50, 0.50', { timeout: 5_000 });
+    });
+});

packages/dashboard/src/i18n/locales/ar.po

@@ -164,7 +164,7 @@ msgstr "النوع"
 #. placeholder {0}: entityName.toLowerCase()
 #. placeholder {0}: group.name
 #: src/app/routes/_authenticated/_products/components/product-option-select.tsx:57
-#: src/lib/components/data-input/default-relation-input.tsx:616
+#: src/lib/components/data-input/default-relation-input.tsx:676
 msgid "Select {0}"
 msgstr "تحديد {0}"
 
@@ -348,6 +348,7 @@ msgstr "تم إنشاء موقع المخزون بنجاح"
 #: src/app/routes/_authenticated/_products/components/generate-variants-panel.tsx:185
 #: src/app/routes/_authenticated/_products/products_.$id_.variants.tsx:112
 #: src/app/routes/_authenticated/_products/products_.$id.tsx:204
+#: src/lib/components/shared/asset/asset-preview.tsx:142
 msgid "Unknown error"
 msgstr "خطأ غير معروف"
 
@@ -893,6 +894,7 @@ msgid "Shipping address set for order"
 msgstr "تم تعيين عنوان الشحن للطلب"
 
 #: src/app/routes/_authenticated/_assets/assets_.$id.tsx:173
+#: src/lib/components/shared/asset/asset-preview.tsx:168
 msgid "Focal Point"
 msgstr "نقطة التركيز"
 
@@ -1284,10 +1286,18 @@ msgstr "{buttonText}"
 msgid "Item added to order"
 msgstr "تمت إضافة عنصر إلى الطلب"
 
+#: src/lib/components/shared/asset/asset-preview.tsx:161
+msgid "Edit focal point"
+msgstr "تحرير نقطة التركيز"
+
 #: src/lib/components/shared/seller-selector.tsx:92
 #~ msgid "No sellers found"
 #~ msgstr "لم يتم العثور على بائعين"
 
+#: src/lib/components/shared/asset/asset-preview-dialog.tsx:34
+msgid "Asset"
+msgstr "ملف"
+
 #: src/app/routes/_authenticated/_products/components/shared-option-group-warning.tsx:13
 msgid "{productCount, plural, one {This option group is used by one other product. Changes will affect it too.} other {This option group is shared across # products. Changes will affect all of them.}}"
 msgstr "{productCount, plural, one {تُستخدم مجموعة الخيارات هذه بواسطة منتج آخر واحد. ستؤثر التغييرات عليه أيضًا.} other {تتم مشاركة مجموعة الخيارات هذه عبر # منتجات. ستؤثر التغييرات على جميعها.}}"
@@ -2750,7 +2760,7 @@ msgstr "حد الاستخدام لكل عميل"
 msgid "Billing address changed"
 msgstr "تم تغيير عنوان الفوترة"
 
-#: src/lib/components/data-input/select-with-options.tsx:101
+#: src/lib/components/data-input/select-with-options.tsx:107
 msgid "Select an option"
 msgstr "حدد خيارًا"
 
@@ -2812,6 +2822,10 @@ msgstr "من {0} إلى {1}"
 msgid "Failed to create customer"
 msgstr "فشل إنشاء العميل"
 
+#: src/lib/components/shared/asset/asset-preview.tsx:138
+msgid "Focal point updated"
+msgstr "تم تحديث نقطة التركيز"
+
 #: src/app/routes/_authenticated/_products/products_.$id_.variants.tsx:298
 msgid "Option values"
 msgstr "قيم الخيارات"
@@ -3054,6 +3068,10 @@ msgstr "هل هي الفئة الضريبية الافتراضية"
 msgid "Slug is set"
 msgstr "تم تعيين الرابط المختصر"
 
+#: src/lib/components/shared/asset/asset-preview.tsx:141
+msgid "Failed to update focal point"
+msgstr "فشل تحديث نقطة التركيز"
+
 #: src/app/routes/_authenticated/_zones/zones_.$id.tsx:61
 msgid "Successfully updated zone"
 msgstr "تم تحديث المنطقة بنجاح"
@@ -3315,7 +3333,7 @@ msgstr "نعم"
 msgid "Slug"
 msgstr "الرابط المختصر"
 
-#: src/lib/components/shared/asset/asset-focal-point-editor.tsx:97
+#: src/lib/components/shared/asset/asset-focal-point-editor.tsx:98
 msgid "Set Focal Point"
 msgstr "تعيين نقطة التركيز"
 
@@ -3570,6 +3588,7 @@ msgid "Cannot remove from active channel"
 msgstr "لا يمكن الإزالة من القناة النشطة"
 
 #: src/app/routes/_authenticated/_assets/assets_.$id.tsx:179
+#: src/lib/components/shared/asset/asset-preview.tsx:177
 msgid "Not set"
 msgstr "غير معين"
 
@@ -4983,6 +5002,11 @@ msgstr "جارٍ التنفيذ..."
 msgid "Add option group to product"
 msgstr "إضافة مجموعة خيارات إلى المنتج"
 
+#. placeholder {0}: asset.name
+#: src/lib/components/shared/asset/asset-preview-dialog.tsx:37
+msgid "Preview of {0}"
+msgstr "معاينة {0}"
+
 #: src/app/routes/_authenticated/_sellers/sellers_.$id.tsx:56
 msgid "Successfully updated seller"
 msgstr "تم تحديث البائع بنجاح"
@@ -5162,7 +5186,7 @@ msgid "Customer not found"
 msgstr "لم يتم العثور على العميل"
 
 #. placeholder {0}: entityName.toLowerCase()
-#: src/lib/components/data-input/default-relation-input.tsx:603
+#: src/lib/components/data-input/default-relation-input.tsx:663
 msgid "Select {0}s"
 msgstr "تحديد {0}"