diff --git a/frontend/src/create/review/Review.svelte b/frontend/src/create/review/Review.svelte index 5df62c0..081f9fb 100644 --- a/frontend/src/create/review/Review.svelte +++ b/frontend/src/create/review/Review.svelte @@ -147,6 +147,10 @@ role: 'calibration' | 'overlay' | 'batch'; isCalibration: boolean; kept: boolean; + /** True for the mandatory photos (clean calibration + dimension-overlay + copy). Their keep checkbox is checked and disabled — the owner cannot + remove them (REVIEW-5 requires both). */ + required: boolean; original: Blob; processed: Blob | null; url: string; @@ -197,6 +201,10 @@ id: r.id, role, isCalibration: r.is_calibration === 1, + // Mandatory: the clean calibration photo and the dimension-overlay + // copy. REVIEW-5 requires both to ship, so their keep control is + // checked + disabled (the owner can't remove them). + required: role === 'calibration' || role === 'overlay', kept: true, original: blob ?? new Blob(), processed: blob ?? null, @@ -219,7 +227,12 @@ // Whole-card tap = keep/remove toggle. A real drag (>6px) reorders and does NOT // toggle keep/remove — the threshold lives in the pointer handlers below. function toggleKeep(id: string) { - photos = photos.map((p) => (p.id === id ? { ...p, kept: !p.kept } : p)); + photos = photos.map((p) => { + if (p.id !== id) return p; + // Mandatory photos (clean cal + dimension-overlay) can't be removed. + if (p.required) return p; + return { ...p, kept: !p.kept }; + }); lineupDirty = true; } @@ -524,12 +537,15 @@ class="pcard" class:kept={p.kept} class:primary={p.id === primaryId} + class:required={p.required} data-testid="photo-card" data-primary={p.id === primaryId ? 'true' : ''} + data-required={p.required ? 'true' : ''} role="button" tabindex="0" - aria-pressed={p.kept} - aria-label="{roleLabel(p.role)}{p.kept ? ', kept' : ', removed'}" + aria-pressed={p.required ? true : p.kept} + aria-disabled={p.required ? 'true' : 'false'} + aria-label="{p.required ? 'Required: ' : ''}{roleLabel(p.role)}{p.required ? ' (cannot be removed)' : (p.kept ? ', kept' : ', removed')}" draggable="true" on:click={() => toggleKeep(p.id)} on:keydown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); toggleKeep(p.id); } }} @@ -542,7 +558,9 @@ PRIMARY {/if} - + + {/each} @@ -810,5 +828,14 @@ .ph-keep { position: absolute; top: 6px; left: 6px; width: 16px; height: 16px; border: 2px solid #fff; border-radius: 3px; background: rgba(0,0,0,.5); pointer-events: none; } .pcard.kept .ph-keep { background: #00e5ff; border-color: #00e5ff; } .pcard.kept .ph-keep::after { content: ''; position: absolute; left: 4px; top: 1px; width: 4px; height: 8px; border: solid #002; border-width: 0 2px 2px 0; transform: rotate(45deg); } + /* Mandatory photos (clean cal + dimension-overlay): the keep checkbox is + checked AND disabled — it can't be unchecked, and a small lock glyph + signals "required, cannot be removed". The card keeps full opacity even + though it's always kept, and the cursor shows it's not a remove target. */ + .pcard.required { border-color: #2a2a2a; } + .pcard.required .ph-keep { background: #00e5ff; border-color: #00e5ff; } + .pcard.required .ph-keep::after { content: ''; position: absolute; left: 4px; top: 1px; width: 4px; height: 8px; border: solid #002; border-width: 0 2px 2px 0; transform: rotate(45deg); } + .pcard.required .ph-keep::before { content: ''; position: absolute; left: 2.5px; top: 5px; width: 7px; height: 5px; border: 1.5px solid #002; border-radius: 1px; } + .pcard.required { cursor: default; } .ph-nudge { color: #e8a; font-size: .74rem; margin: 8px 0 0; } diff --git a/frontend/tests/e2e/review-photo-strip.spec.ts b/frontend/tests/e2e/review-photo-strip.spec.ts index 1473875..6f375d1 100644 --- a/frontend/tests/e2e/review-photo-strip.spec.ts +++ b/frontend/tests/e2e/review-photo-strip.spec.ts @@ -212,18 +212,30 @@ test.describe('M3 Review photo strip (#90)', () => { const bgImage = await cards.nth(0).evaluate((el) => getComputedStyle(el).backgroundImage); expect(bgImage).toContain('45deg'); - // Remove the primary (cal) → primary auto-promotes to the next leftmost (ov). - await cards.nth(0).click(); - await expect(cards.nth(0)).toHaveAttribute('aria-pressed', 'false'); - await expect(cards.nth(1)).toHaveAttribute('data-primary', 'true'); - // Minimum nudge: cal + batch kept, but overlay still present → only cal missing. - await expect(page.getByTestId('lineup-nudge')).toBeVisible(); - await expect(page.getByTestId('lineup-nudge')).toContainText('clean photo'); + // Mandatory photos: the clean calibration (seq 1) and the dimension-overlay + // copy (seq 2) are required — their keep checkbox is checked AND disabled. + await expect(cards.nth(0)).toHaveAttribute('data-required', 'true'); + await expect(cards.nth(1)).toHaveAttribute('data-required', 'true'); + await expect(cards.nth(2)).toHaveAttribute('data-required', ''); + await expect(cards.nth(0)).toHaveAttribute('aria-pressed', 'true'); + await expect(cards.nth(1)).toHaveAttribute('aria-pressed', 'true'); - // Re-add the cal (the original re-check bug) → it's back to the left = primary. - await cards.nth(0).click(); + // Tapping a mandatory photo is a no-op: it stays checked + primary (can't remove). + // The card is aria-disabled, so force the click to exercise the handler directly. + await cards.nth(0).click({ force: true }); await expect(cards.nth(0)).toHaveAttribute('aria-pressed', 'true'); await expect(cards.nth(0)).toHaveAttribute('data-primary', 'true'); + // No minimum nudge — both required photos are kept by construction. + await expect(page.getByTestId('lineup-nudge')).toHaveCount(0); + + // Remove the OPTIONAL batch photo (index 2) → it un-keeps; required stay kept. + await cards.nth(2).click(); + await expect(cards.nth(2)).toHaveAttribute('aria-pressed', 'false'); + // Primary is still the leftmost required (cal, index 0). + await expect(cards.nth(0)).toHaveAttribute('data-primary', 'true'); + // Re-add the batch photo. + await cards.nth(2).click(); + await expect(cards.nth(2)).toHaveAttribute('aria-pressed', 'true'); await expect(page.getByTestId('lineup-nudge')).toHaveCount(0); // Drag the batch photo (index 2) to the front → it becomes the primary. @@ -259,20 +271,29 @@ test.describe('M3 Review photo strip (#90)', () => { expect(seqs).toEqual([1, 2, 3]); }); - test('removing a photo persists a DELETE and blocks publish when the minimum is broken', async ({ page }) => { + test('required photos guarantee the minimum; removing an OPTIONAL batch persists a DELETE', async ({ page }) => { const mock = await mockApi(page); await reachReview(page); const cards = page.getByTestId('photo-card'); - // Remove the overlay (seq 2) → minimum broken (no overlay). - await cards.nth(1).click(); - await expect(cards.nth(1)).toHaveAttribute('aria-pressed', 'false'); - await expect(page.getByTestId('lineup-nudge')).toContainText('overlay'); + // The clean calibration + dimension-overlay are mandatory and always kept, + // so the submission minimum can never be broken — publish is never lineup-blocked. + await expect(page.getByTestId('lineup-nudge')).toHaveCount(0); - // Complete the rest of the form; the slab fields are ready but the lineup is not. + // Removing an OPTIONAL batch photo is allowed and persists a DELETE. + await cards.nth(2).click(); + await expect(cards.nth(2)).toHaveAttribute('aria-pressed', 'false'); + // Still no nudge: both required photos remain kept. + await expect(page.getByTestId('lineup-nudge')).toHaveCount(0); + + // Complete the rest of the form; the lineup is valid (required photos kept). await completeForReady(page); - await expect(page.getByTestId('ready-status')).toContainText(/Not ready|remaining/); - // Continue is blocked by the lineup minimum even though the fields are set. - await expect(page.getByTestId('continue-publish')).toBeDisabled(); + await expect(page.getByTestId('ready-status')).toContainText('Ready to publish', { timeout: 15000 }); + await expect(page.getByTestId('continue-publish')).toBeEnabled(); + await page.getByTestId('continue-publish').click(); + await expect(page.getByTestId('publish-screen')).toBeVisible(); + + // The removed batch photo persisted as a DELETE; the required photos re-uploaded. + expect(mock.deletes).toEqual(['ph-b1']); }); });