Ty_Tech/SlabUploader
6
0
Fork 0
Code Issues 15 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(review): #90 clean cal + dimension-overlay are mandatory (checked + disabled)

Browse Source 
REVIEW-5 requires both the clean calibration photo and its dimension-overlay
copy to ship. Their keep checkbox is now checked AND disabled (aria-disabled,
a lock glyph, cursor:default, tap is a no-op) so the owner can't remove them.
Only batch photos are removable. The submission minimum can no longer be
broken by the owner, so publish is never lineup-blocked by a missing required
photo.
This commit is contained in:
Ty
2026-09-21 02:15:00 -07:00
parent 5a1800df91
commit 76fcb36a2b
2 changed files with 70 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/create/review/Review.svelte
+31 -4
View File
@@ -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 @@
<span class="ph-pri">PRIMARY</span>
{/if}
<span class="ph-seq" aria-hidden="true">{i + 1}</span>
<span class="ph-keep" aria-hidden="true"></span>
<!-- Keep checkbox: always checked for required photos; a disabled
glyph (no check) when a batch photo is removed. -->
<span class="ph-keep" class:required={p.required} aria-hidden="true"></span>
</div>
{/each}
</div>
@@ -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; }
</style>
frontend/tests/e2e/review-photo-strip.spec.ts
+39 -18
View File
@@ -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']);
});
});