fix(products-ui): review pass — show-more race, keyboard history access, a11y glyphs/aria-live, label consistency
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -42,6 +42,11 @@ export interface ProductsSummary {
|
|||||||
|
|
||||||
export type Result<T> = { ok: true; value: T } | { ok: false; error: ApiError; status: number };
|
export type Result<T> = { ok: true; value: T } | { ok: false; error: ApiError; status: number };
|
||||||
|
|
||||||
|
// One label rule for CSV dialects, shared by Products history / preview / run detail.
|
||||||
|
export function dialectLabel(d: string): string {
|
||||||
|
return d === "canonical" ? "Canonical format" : d;
|
||||||
|
}
|
||||||
|
|
||||||
async function request<T>(path: string, init?: RequestInit): Promise<Result<T>> {
|
async function request<T>(path: string, init?: RequestInit): Promise<Result<T>> {
|
||||||
const resp = await fetch(path, { credentials: "include", ...init });
|
const resp = await fetch(path, { credentials: "include", ...init });
|
||||||
if (!resp.ok) return { ok: false, error: await errorOf(resp), status: resp.status };
|
if (!resp.ok) return { ok: false, error: await errorOf(resp), status: resp.status };
|
||||||
|
|||||||
@@ -1,10 +1,11 @@
|
|||||||
// Import preview (SD-0002 §5.4) — the consent gate. Summary tiles filter a
|
// Import preview (SD-0002 §5.4) — the consent gate. Summary tiles filter a
|
||||||
// drill-in diff list; the sticky footer carries confirm (PUC-3) / cancel (PUC-3a).
|
// drill-in diff list; the sticky footer carries confirm (PUC-3) / cancel (PUC-3a).
|
||||||
// Diff glyphs pair with color, never color alone (§6.6).
|
// Diff glyphs pair with color, never color alone (§6.6).
|
||||||
import { useEffect, useState } from "react";
|
import { useEffect, useRef, useState } from "react";
|
||||||
import {
|
import {
|
||||||
cancelDraft,
|
cancelDraft,
|
||||||
confirmDraft,
|
confirmDraft,
|
||||||
|
dialectLabel,
|
||||||
getDraft,
|
getDraft,
|
||||||
getDraftRecords,
|
getDraftRecords,
|
||||||
type Draft,
|
type Draft,
|
||||||
@@ -40,8 +41,8 @@ function SetLine({ field, value }: { field: string; value: unknown }) {
|
|||||||
function ChangeRow({ change }: { change: FieldChange }) {
|
function ChangeRow({ change }: { change: FieldChange }) {
|
||||||
return (
|
return (
|
||||||
<div className="diffchange">
|
<div className="diffchange">
|
||||||
{change.field}: <span className="diffchange__glyph--del">{fmt(change.before)}</span> →{" "}
|
{change.field}: <span className="diffchange__glyph--del">− {fmt(change.before)}</span> →{" "}
|
||||||
<span className="diffchange__glyph--add">{fmt(change.after)}</span>
|
<span className="diffchange__glyph--add">+ {fmt(change.after)}</span>
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
@@ -137,6 +138,7 @@ export default function ImportPreview({ draftId }: { draftId: number }) {
|
|||||||
const [loadFail, setLoadFail] = useState<"gone" | "expired" | "failed" | null>(null);
|
const [loadFail, setLoadFail] = useState<"gone" | "expired" | "failed" | null>(null);
|
||||||
const [filter, setFilter] = useState<RecordKind | null>(null);
|
const [filter, setFilter] = useState<RecordKind | null>(null);
|
||||||
const [records, setRecords] = useState<DraftRecord[] | null>(null);
|
const [records, setRecords] = useState<DraftRecord[] | null>(null);
|
||||||
|
const [recordsError, setRecordsError] = useState<"load" | "more" | null>(null);
|
||||||
const [hasMore, setHasMore] = useState(false);
|
const [hasMore, setHasMore] = useState(false);
|
||||||
const [moreBusy, setMoreBusy] = useState(false);
|
const [moreBusy, setMoreBusy] = useState(false);
|
||||||
const [confirming, setConfirming] = useState(false);
|
const [confirming, setConfirming] = useState(false);
|
||||||
@@ -144,6 +146,10 @@ export default function ImportPreview({ draftId }: { draftId: number }) {
|
|||||||
const [stale, setStale] = useState(false);
|
const [stale, setStale] = useState(false);
|
||||||
const [nothingNote, setNothingNote] = useState(false);
|
const [nothingNote, setNothingNote] = useState(false);
|
||||||
const [confirmError, setConfirmError] = useState<string | null>(null);
|
const [confirmError, setConfirmError] = useState<string | null>(null);
|
||||||
|
// Generation counter for the records list: bumped on every page-0 (re)load, so a
|
||||||
|
// page-0 or show-more response that resolves after a tile/filter (or draft) switch
|
||||||
|
// is recognized as stale and dropped instead of clobbering/appending to the new list.
|
||||||
|
const recordsGen = useRef(0);
|
||||||
|
|
||||||
async function loadDraft() {
|
async function loadDraft() {
|
||||||
setLoadFail(null);
|
setLoadFail(null);
|
||||||
@@ -155,32 +161,54 @@ export default function ImportPreview({ draftId }: { draftId: number }) {
|
|||||||
setDraft(resp.value);
|
setDraft(resp.value);
|
||||||
}
|
}
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
|
// A new draft means a fresh consent gate — reset everything the old one set.
|
||||||
|
setFilter(null);
|
||||||
|
setStale(false);
|
||||||
|
setConfirmError(null);
|
||||||
|
setNothingNote(false);
|
||||||
|
setRecords(null);
|
||||||
|
setRecordsError(null);
|
||||||
|
setHasMore(false);
|
||||||
void loadDraft();
|
void loadDraft();
|
||||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||||
}, [draftId]);
|
}, [draftId]);
|
||||||
|
|
||||||
useEffect(() => {
|
function loadRecords(kind: RecordKind | null) {
|
||||||
if (!draft) return;
|
recordsGen.current += 1;
|
||||||
let gone = false;
|
const gen = recordsGen.current;
|
||||||
setRecords(null);
|
setRecords(null);
|
||||||
|
setRecordsError(null);
|
||||||
setHasMore(false);
|
setHasMore(false);
|
||||||
void getDraftRecords(draftId, filter ?? undefined, PAGE, 0).then((resp) => {
|
void getDraftRecords(draftId, kind ?? undefined, PAGE, 0).then((resp) => {
|
||||||
if (gone || !resp.ok) return;
|
if (gen !== recordsGen.current) return;
|
||||||
|
if (!resp.ok) {
|
||||||
|
setRecordsError("load");
|
||||||
|
return;
|
||||||
|
}
|
||||||
setRecords(resp.value);
|
setRecords(resp.value);
|
||||||
setHasMore(resp.value.length === PAGE);
|
setHasMore(resp.value.length === PAGE);
|
||||||
});
|
});
|
||||||
return () => {
|
}
|
||||||
gone = true;
|
useEffect(() => {
|
||||||
};
|
if (!draft) return;
|
||||||
|
loadRecords(filter);
|
||||||
|
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||||
}, [draft, draftId, filter]);
|
}, [draft, draftId, filter]);
|
||||||
|
|
||||||
async function showMore() {
|
async function showMore() {
|
||||||
if (!records) return;
|
if (!records) return;
|
||||||
|
const gen = recordsGen.current;
|
||||||
setMoreBusy(true);
|
setMoreBusy(true);
|
||||||
|
setRecordsError(null);
|
||||||
const resp = await getDraftRecords(draftId, filter ?? undefined, PAGE, records.length);
|
const resp = await getDraftRecords(draftId, filter ?? undefined, PAGE, records.length);
|
||||||
setMoreBusy(false);
|
setMoreBusy(false);
|
||||||
if (!resp.ok) return;
|
// Filter/draft switched while this page was in flight — drop the stale page.
|
||||||
setRecords([...records, ...resp.value]);
|
if (gen !== recordsGen.current) return;
|
||||||
|
if (!resp.ok) {
|
||||||
|
setRecordsError("more");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
setRecords((prev) => [...(prev ?? []), ...resp.value]);
|
||||||
setHasMore(resp.value.length === PAGE);
|
setHasMore(resp.value.length === PAGE);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -233,7 +261,13 @@ export default function ImportPreview({ draftId }: { draftId: number }) {
|
|||||||
</Banner>
|
</Banner>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
if (!draft) return <p className="note">Loading…</p>;
|
if (!draft) {
|
||||||
|
return (
|
||||||
|
<p className="note" role="status">
|
||||||
|
Loading…
|
||||||
|
</p>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
const { summary } = draft;
|
const { summary } = draft;
|
||||||
const tiles: { kind: RecordKind; num: number; label: string }[] = [
|
const tiles: { kind: RecordKind; num: number; label: string }[] = [
|
||||||
@@ -250,10 +284,17 @@ export default function ImportPreview({ draftId }: { draftId: number }) {
|
|||||||
<a href="#/products">← Products</a>
|
<a href="#/products">← Products</a>
|
||||||
</p>
|
</p>
|
||||||
<h1>Import preview — {draft.file_name}</h1>
|
<h1>Import preview — {draft.file_name}</h1>
|
||||||
<p className="note">{draft.dialect === "canonical" ? "Canonical format" : draft.dialect}</p>
|
<p className="note">{dialectLabel(draft.dialect)}</p>
|
||||||
{draft.unknown_columns.length > 0 && (
|
{draft.unknown_columns.length > 0 && (
|
||||||
<Banner tone="info" title="Columns not imported">
|
<Banner tone="info" title="Columns not imported">
|
||||||
{draft.unknown_columns.join(", ")}
|
{draft.unknown_columns.length > 8 ? (
|
||||||
|
<details>
|
||||||
|
<summary>{draft.unknown_columns.length} columns not imported</summary>
|
||||||
|
{draft.unknown_columns.join(", ")}
|
||||||
|
</details>
|
||||||
|
) : (
|
||||||
|
draft.unknown_columns.join(", ")
|
||||||
|
)}
|
||||||
</Banner>
|
</Banner>
|
||||||
)}
|
)}
|
||||||
<div className="tiles">
|
<div className="tiles">
|
||||||
@@ -271,8 +312,17 @@ export default function ImportPreview({ draftId }: { draftId: number }) {
|
|||||||
))}
|
))}
|
||||||
</div>
|
</div>
|
||||||
{filter === "error" && records && <ErrorTable records={records} />}
|
{filter === "error" && records && <ErrorTable records={records} />}
|
||||||
{records === null ? (
|
{recordsError === "load" ? (
|
||||||
<p className="note">Loading…</p>
|
<p className="note note--attn" role="alert">
|
||||||
|
Couldn't load these records.{" "}
|
||||||
|
<button type="button" className="linklike" onClick={() => loadRecords(filter)}>
|
||||||
|
Retry
|
||||||
|
</button>
|
||||||
|
</p>
|
||||||
|
) : records === null ? (
|
||||||
|
<p className="note" role="status">
|
||||||
|
Loading…
|
||||||
|
</p>
|
||||||
) : records.length === 0 ? (
|
) : records.length === 0 ? (
|
||||||
<p className="note">Nothing to show here.</p>
|
<p className="note">Nothing to show here.</p>
|
||||||
) : (
|
) : (
|
||||||
@@ -294,9 +344,17 @@ export default function ImportPreview({ draftId }: { draftId: number }) {
|
|||||||
<button type="button" className="btn-secondary" disabled={moreBusy} onClick={() => void showMore()}>
|
<button type="button" className="btn-secondary" disabled={moreBusy} onClick={() => void showMore()}>
|
||||||
{moreBusy ? "Loading…" : "Show more"}
|
{moreBusy ? "Loading…" : "Show more"}
|
||||||
</button>
|
</button>
|
||||||
|
{recordsError === "more" && (
|
||||||
|
<span className="note note--attn" role="alert">
|
||||||
|
Couldn't load more records.{" "}
|
||||||
|
<button type="button" className="linklike" onClick={() => void showMore()}>
|
||||||
|
Retry
|
||||||
|
</button>
|
||||||
|
</span>
|
||||||
|
)}
|
||||||
</p>
|
</p>
|
||||||
)}
|
)}
|
||||||
<div className="sticky-footer">
|
<div className="sticky-footer" aria-live="polite">
|
||||||
{stale ? (
|
{stale ? (
|
||||||
<Banner tone="attn" title="Your catalog changed since this preview — upload the file again">
|
<Banner tone="attn" title="Your catalog changed since this preview — upload the file again">
|
||||||
<a href="#/products/import">Upload the file again</a>
|
<a href="#/products/import">Upload the file again</a>
|
||||||
|
|||||||
@@ -44,7 +44,9 @@ export default function ImportUpload() {
|
|||||||
disabled={busy}
|
disabled={busy}
|
||||||
onChange={(e) => void onPick(e.target.files)}
|
onChange={(e) => void onPick(e.target.files)}
|
||||||
/>
|
/>
|
||||||
<span className="dropzone__title">{busy ? "Validating…" : "Choose a CSV file"}</span>
|
<span className="dropzone__title" aria-live="polite">
|
||||||
|
{busy ? "Validating…" : "Choose a CSV file"}
|
||||||
|
</span>
|
||||||
<span className="note">CSV, up to 5,000 rows</span>
|
<span className="note">CSV, up to 5,000 rows</span>
|
||||||
</label>
|
</label>
|
||||||
<p className="note">
|
<p className="note">
|
||||||
|
|||||||
@@ -1,7 +1,13 @@
|
|||||||
// Products page (SD-0002 §5.2) — the catalog's home: where imports start and history
|
// Products page (SD-0002 §5.2) — the catalog's home: where imports start and history
|
||||||
// lives. SLICE-5: export is disabled (SLICE-6 ships it); the browsable list is #14's.
|
// lives. SLICE-5: export is disabled (SLICE-6 ships it); the browsable list is #14's.
|
||||||
import { useEffect, useState } from "react";
|
import { useEffect, useState } from "react";
|
||||||
import { getProductsSummary, listRuns, type ProductsSummary, type RunSummary } from "../../productsApi";
|
import {
|
||||||
|
dialectLabel,
|
||||||
|
getProductsSummary,
|
||||||
|
listRuns,
|
||||||
|
type ProductsSummary,
|
||||||
|
type RunSummary,
|
||||||
|
} from "../../productsApi";
|
||||||
import { Banner } from "../../ui/kit";
|
import { Banner } from "../../ui/kit";
|
||||||
|
|
||||||
const STATUS_LABELS: Record<string, string> = {
|
const STATUS_LABELS: Record<string, string> = {
|
||||||
@@ -40,7 +46,13 @@ export default function ProductsPage() {
|
|||||||
</Banner>
|
</Banner>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
if (!summary || !runs) return <p className="note">Loading…</p>;
|
if (!summary || !runs) {
|
||||||
|
return (
|
||||||
|
<p className="note" role="status">
|
||||||
|
Loading…
|
||||||
|
</p>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
const empty = summary.product_count === 0;
|
const empty = summary.product_count === 0;
|
||||||
return (
|
return (
|
||||||
@@ -51,9 +63,12 @@ export default function ProductsPage() {
|
|||||||
{!empty && <span className="products__count"> · {summary.product_count.toLocaleString()}</span>}
|
{!empty && <span className="products__count"> · {summary.product_count.toLocaleString()}</span>}
|
||||||
</h1>
|
</h1>
|
||||||
<div className="products__actions">
|
<div className="products__actions">
|
||||||
<button type="button" className="btn-secondary" disabled title="Export arrives in a coming release">
|
<div className="products__export">
|
||||||
Export
|
<button type="button" className="btn-secondary" disabled title="Export arrives in a coming release">
|
||||||
</button>
|
Export
|
||||||
|
</button>
|
||||||
|
<span className="note">Export arrives in a coming release</span>
|
||||||
|
</div>
|
||||||
<a className="btn-primary" href="#/products/import">
|
<a className="btn-primary" href="#/products/import">
|
||||||
Import products
|
Import products
|
||||||
</a>
|
</a>
|
||||||
@@ -95,8 +110,13 @@ export default function ProductsPage() {
|
|||||||
}}
|
}}
|
||||||
>
|
>
|
||||||
<td>{new Date(r.created_at).toLocaleString()}</td>
|
<td>{new Date(r.created_at).toLocaleString()}</td>
|
||||||
<td>{r.file_name}</td>
|
<td>
|
||||||
<td>{r.dialect}</td>
|
{/* Anchor = the keyboard/SR path (§6.6); the row onClick stays as a
|
||||||
|
mouse convenience. Both set the same hash, so the double fire on
|
||||||
|
an anchor click is idempotent. */}
|
||||||
|
<a href={`#/products/imports/runs/${r.id}`}>{r.file_name}</a>
|
||||||
|
</td>
|
||||||
|
<td>{dialectLabel(r.dialect)}</td>
|
||||||
<td>{r.products_added}</td>
|
<td>{r.products_added}</td>
|
||||||
<td>{r.products_updated}</td>
|
<td>{r.products_updated}</td>
|
||||||
<td>{r.rows_errored}</td>
|
<td>{r.rows_errored}</td>
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
// Run detail (SD-0002 §5.5) — report card for a completed or in-progress import run.
|
// Run detail (SD-0002 §5.5) — report card for a completed or in-progress import run.
|
||||||
// NO images section this slice (SLICE-7). PUC-4/5/8.
|
// NO images section this slice (SLICE-7). PUC-4/5/8.
|
||||||
import { useEffect, useState } from "react";
|
import { useEffect, useState } from "react";
|
||||||
import { getRun, type RunDetail as RunDetailType } from "../../productsApi";
|
import { dialectLabel, getRun, type RunDetail as RunDetailType } from "../../productsApi";
|
||||||
import { Banner } from "../../ui/kit";
|
import { Banner } from "../../ui/kit";
|
||||||
|
|
||||||
const STATUS_LABELS: Record<string, string> = {
|
const STATUS_LABELS: Record<string, string> = {
|
||||||
@@ -46,9 +46,13 @@ export default function RunDetail({ runId }: { runId: number }) {
|
|||||||
</Banner>
|
</Banner>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
if (!run) return <p className="note">Loading…</p>;
|
if (!run) {
|
||||||
|
return (
|
||||||
const dialectLabel = run.dialect === "canonical" ? "Canonical format" : run.dialect;
|
<p className="note" role="status">
|
||||||
|
Loading…
|
||||||
|
</p>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="products">
|
<div className="products">
|
||||||
@@ -57,7 +61,7 @@ export default function RunDetail({ runId }: { runId: number }) {
|
|||||||
</p>
|
</p>
|
||||||
<h1>{run.file_name}</h1>
|
<h1>{run.file_name}</h1>
|
||||||
<p className="note">
|
<p className="note">
|
||||||
Imported {new Date(run.created_at).toLocaleString()} by {run.by} · {dialectLabel}
|
Imported {new Date(run.created_at).toLocaleString()} by {run.by} · {dialectLabel(run.dialect)}
|
||||||
</p>
|
</p>
|
||||||
<p className="note">
|
<p className="note">
|
||||||
{run.products_added} added · {run.products_updated} updated · {run.rows_errored} rows in
|
{run.products_added} added · {run.products_updated} updated · {run.rows_errored} rows in
|
||||||
@@ -77,7 +81,7 @@ export default function RunDetail({ runId }: { runId: number }) {
|
|||||||
{run.errors.map((e, i) => (
|
{run.errors.map((e, i) => (
|
||||||
<tr key={i}>
|
<tr key={i}>
|
||||||
<td>{e.line}</td>
|
<td>{e.line}</td>
|
||||||
<td>{e.column ?? ""}</td>
|
<td>{e.column ?? "—"}</td>
|
||||||
<td>{e.message}</td>
|
<td>{e.message}</td>
|
||||||
</tr>
|
</tr>
|
||||||
))}
|
))}
|
||||||
|
|||||||
@@ -53,6 +53,9 @@
|
|||||||
}
|
}
|
||||||
.products__count { color: var(--text-on-dark-mute); font-weight: var(--weight-medium); }
|
.products__count { color: var(--text-on-dark-mute); font-weight: var(--weight-medium); }
|
||||||
.products__actions { display: flex; gap: 12px; align-items: center; }
|
.products__actions { display: flex; gap: 12px; align-items: center; }
|
||||||
|
/* Disabled Export + its visible "coming release" caption, stacked. */
|
||||||
|
.products__export { display: flex; flex-direction: column; gap: 4px; align-items: center; }
|
||||||
|
.products__export .note { font-size: 11.5px; }
|
||||||
.products .btn-primary { width: auto; text-decoration: none; }
|
.products .btn-primary { width: auto; text-decoration: none; }
|
||||||
.products .empty { margin: 24px auto 0; }
|
.products .empty { margin: 24px auto 0; }
|
||||||
|
|
||||||
@@ -160,7 +163,6 @@
|
|||||||
|
|
||||||
/* ── diff list (preview records, Task 13) ───────────────────────────────────── */
|
/* ── diff list (preview records, Task 13) ───────────────────────────────────── */
|
||||||
.difflist { list-style: none; margin: 0; padding: 0; }
|
.difflist { list-style: none; margin: 0; padding: 0; }
|
||||||
.difflist > li { padding: 12px 4px; border-bottom: 1px solid var(--border-soft); }
|
|
||||||
.difflist__item { padding: 12px 4px; border-bottom: 1px solid var(--border-soft); }
|
.difflist__item { padding: 12px 4px; border-bottom: 1px solid var(--border-soft); }
|
||||||
.difflist__item > summary {
|
.difflist__item > summary {
|
||||||
cursor: pointer;
|
cursor: pointer;
|
||||||
|
|||||||
Reference in New Issue
Block a user