diff --git a/src/components/Output/ImportReportModal.tsx b/src/components/Output/ImportReportModal.tsx deleted file mode 100644 index 0b236aa3..00000000 --- a/src/components/Output/ImportReportModal.tsx +++ /dev/null @@ -1,107 +0,0 @@ -import { useState } from 'react'; -import { XMarkIcon, ClipboardDocumentIcon, CheckIcon } from '@heroicons/react/16/solid'; -import { partialLoss, formatReportAsText } from '../../lib/importReport'; -import type { ImportResult } from '../../lib/importReport'; -import { useT } from '../../lib/useT'; -import { DialogShell } from '../ui/DialogShell'; - -export type { ImportResult }; - -export function ImportSummaryBody({ result }: { result: ImportResult }) { - const { objectCount, report } = result; - const hasIssues = report.partial.length > 0 || report.browserLimit.length > 0 || report.unknown.length > 0; - const uniqueLosses = [...new Set(report.partial.map(partialLoss))]; - - return ( -
-

- {objectCount} object{objectCount !== 1 ? 's' : ''} imported as editable reconstruction. - The original ZPL model is not preserved — save as .json to keep an exact copy. -

- - {report.partial.length > 0 && ( -
- Partially imported{' '} - ({report.partial.join(', ')}): {uniqueLosses.join('; ')} -
- )} - - {report.browserLimit.length > 0 && ( -
- Skipped (printer-only){' '} - — {report.browserLimit.map((t) => t.split(',')[0]).join(', ')} -
- )} - - {report.unknown.length > 0 && ( -
- Skipped (unrecognised){' '} - — {report.unknown.length} command{report.unknown.length !== 1 ? 's' : ''} not mapped:{' '} - {report.unknown.slice(0, 5).map((t) => t.split(',')[0]).join(', ')} - {report.unknown.length > 5 && ` … +${report.unknown.length - 5} more`} -
- )} - - {!hasIssues && ( -

- All commands recognised — no design information was lost. -

- )} -
- ); -} - -interface Props { - result: ImportResult; - onClose: () => void; -} - -export function ImportReportModal({ result, onClose }: Props) { - const t = useT(); - const [copied, setCopied] = useState(false); - - const handleCopy = () => { - navigator.clipboard.writeText(formatReportAsText(result)).then(() => { - setCopied(true); - setTimeout(() => setCopied(false), 1500); - }); - }; - - return ( - -
- Import Report - -
- - - -
- - -
- - ); -} diff --git a/src/components/Output/ImportSummary.tsx b/src/components/Output/ImportSummary.tsx new file mode 100644 index 00000000..d45e6123 --- /dev/null +++ b/src/components/Output/ImportSummary.tsx @@ -0,0 +1,60 @@ +import { describeFinding } from '../../lib/importReport'; +import type { ImportFinding, ImportFindingKind, ImportResult } from '../../lib/importReport'; + +export type { ImportResult }; + +/** Severity colour per finding kind. Tailwind classes only. The table + * form over a switch keeps the kind→tone mapping in one expression + * and lets the compiler check exhaustiveness via the Record type. */ +const TONE: Record = { + partial: 'text-amber-400', + browserLimit: 'text-amber-400', + unknown: 'text-muted', +}; + +function FindingRow({ finding, showPage }: { finding: ImportFinding; showPage: boolean }) { + const { title, detail } = describeFinding(finding); + return ( +
+ {showPage && ( + + Page {finding.pageIndex + 1} + + )} +
+ + {title} + + + {detail} + +
+
+ ); +} + +export function ImportSummaryBody({ result }: { result: ImportResult }) { + const { objectCount, report } = result; + const { findings } = report; + // Only show per-row page badges when the import actually spanned multiple + // pages. Single-page reports don't need the badge clutter. + const multiPage = findings.some((f) => f.pageIndex > 0); + + return ( +
+

+ Imported {objectCount} object{objectCount !== 1 ? 's' : ''} with{' '} + {findings.length} issue{findings.length !== 1 ? 's' : ''}: +

+
+ {findings.map((f, i) => ( + + ))} +
+
+ ); +} diff --git a/src/components/Output/ZplImportModal.tsx b/src/components/Output/ZplImportModal.tsx index 89fe3b63..277e51bb 100644 --- a/src/components/Output/ZplImportModal.tsx +++ b/src/components/Output/ZplImportModal.tsx @@ -4,8 +4,8 @@ import { importZplText } from '../../lib/zplImportService'; import { readFileAsText } from '../../lib/readFile'; import { useLabelStore, type Page } from '../../store/labelStore'; import type { LabelConfig } from '../../types/ObjectType'; -import { formatReportAsText, type ImportResult } from '../../lib/importReport'; -import { ImportSummaryBody } from './ImportReportModal'; +import { formatReportAsText, type ImportReport, type ImportResult } from '../../lib/importReport'; +import { ImportSummaryBody } from './ImportSummary'; import { useT } from '../../lib/useT'; import { DialogShell } from '../ui/DialogShell'; @@ -35,7 +35,7 @@ export function ZplImportModal({ onClose }: Props) { const applyImport = (labelConfig: Partial, importedPages: Page[]) => { if (appendMode && hasExistingContent) { - // Keep the current label config — the user opted to keep the + // Keep the current label config: the user opted to keep the // existing design's dimensions, so any imported ^PW/^LL is // intentionally discarded. appendPages(importedPages); @@ -44,23 +44,41 @@ export function ZplImportModal({ onClose }: Props) { } }; - const handleImport = () => { - setError(null); - if (!zpl.trim()) { - setError('Please paste some ZPL code first.'); - return; + // Feedback through state change: when the import has no findings the + // changed canvas is confirmation enough. We only stop on the result + // view when there is something the user could not otherwise see, i.e. + // one or more findings to review. + const finishImport = (totalObjects: number, report: ImportReport) => { + if (report.findings.length === 0) { + onClose(); + } else { + setResult({ objectCount: totalObjects, report }); } + }; - const { labelConfig, pages: importedPages, report } = importZplText(zpl, label.dpmm); + // Shared post-source path: parse, gate on supported content, hand off + // to the store + finishImport. The two entry points (paste textarea, + // file picker) only differ in how they obtain the text and which + // source-specific error they surface; everything past that point is + // identical, so it lives here. + const processImport = (text: string) => { + const { labelConfig, pages: importedPages, report } = importZplText(text, label.dpmm); const totalObjects = importedPages.reduce((s, p) => s + p.objects.length, 0); - if (totalObjects === 0 && Object.keys(labelConfig).length === 0) { setError('No supported objects found in the ZPL code.'); return; } - applyImport(labelConfig, importedPages); - setResult({ objectCount: totalObjects, report }); + finishImport(totalObjects, report); + }; + + const handleImport = () => { + setError(null); + if (!zpl.trim()) { + setError('Please paste some ZPL code first.'); + return; + } + processImport(zpl); }; const handleFileSelect = async (e: React.ChangeEvent) => { @@ -81,17 +99,7 @@ export function ZplImportModal({ onClose }: Props) { setError('The file appears to be empty.'); return; } - - const { labelConfig, pages: importedPages, report } = importZplText(text, label.dpmm); - const totalObjects = importedPages.reduce((s, p) => s + p.objects.length, 0); - - if (totalObjects === 0 && Object.keys(labelConfig).length === 0) { - setError('No supported objects found in the ZPL code.'); - return; - } - - applyImport(labelConfig, importedPages); - setResult({ objectCount: totalObjects, report }); + processImport(text); }; const handleCopy = () => { diff --git a/src/lib/importReport.ts b/src/lib/importReport.ts index 3cc949ce..3cd343b5 100644 --- a/src/lib/importReport.ts +++ b/src/lib/importReport.ts @@ -1,6 +1,11 @@ -import type { ImportReport } from './zplParser'; +import type { ImportFinding, ImportFindingKind, ImportReport } from './zplParser'; import { ZPL_COMMAND_MAP } from './zplCommandSupport'; +// Re-export the parser-side domain types so UI code talks to the report +// surface through this module exclusively instead of reaching into the +// parser for type imports. +export type { ImportFinding, ImportFindingKind, ImportReport }; + export interface ImportResult { objectCount: number; report: ImportReport; @@ -13,25 +18,62 @@ export function partialLoss(cmd: string): string { return entry?.loss ?? 'imported with limitations'; } +/** + * Single source of truth for finding wording. Both the UI list and the + * copy-as-text formatter feed off this so the user sees the same + * description in both surfaces; without it, the two pathways drift + * whenever someone tweaks a string in one place. + * + * `title` is the headline (kind + command code where useful); + * `detail` is the secondary line (loss description for partial, raw + * token for the others). + */ +export function describeFinding(f: ImportFinding): { title: string; detail: string } { + if (f.kind === 'partial') { + return { + title: `Partially imported (${f.command})`, + detail: partialLoss(f.command), + }; + } + if (f.kind === 'browserLimit') { + return { + title: 'Skipped: needs printer hardware', + detail: f.command, + }; + } + return { + title: 'Skipped: command not recognised', + detail: f.command, + }; +} + +/** Compact "Page N: " prefix when a finding originates from a multi-page + * import. Single-page reports omit it to stay terse. */ +function pagePrefix(f: ImportFinding, multiPage: boolean): string { + return multiPage ? `Page ${f.pageIndex + 1}: ` : ''; +} + export function formatReportAsText(result: ImportResult): string { const { objectCount, report } = result; + const findings = report.findings; + const multiPage = findings.some((f) => f.pageIndex > 0); + const lines: string[] = [ `ZPL Import Report`, `Objects imported: ${objectCount}`, '', ]; - if (report.partial.length > 0) { - const uniqueLosses = [...new Set(report.partial.map(partialLoss))]; - lines.push(`Partially imported (${report.partial.join(', ')}): ${uniqueLosses.join('; ')}`); - } - if (report.browserLimit.length > 0) { - lines.push(`Skipped (printer-only): ${report.browserLimit.map((t) => t.split(',')[0]).join(', ')}`); - } - if (report.unknown.length > 0) { - lines.push(`Skipped (unrecognised): ${report.unknown.map((t) => t.split(',')[0]).join(', ')}`); + + if (findings.length === 0) { + lines.push('All commands recognised. No design information was lost.'); + return lines.join('\n'); } - if (report.partial.length === 0 && report.browserLimit.length === 0 && report.unknown.length === 0) { - lines.push('All commands recognised — no design information was lost.'); + + // One row per finding (per page-occurrence). Matches the UI list so the + // copied text mirrors what the user sees in the modal. + for (const f of findings) { + const { title, detail } = describeFinding(f); + lines.push(`${pagePrefix(f, multiPage)}${title}: ${detail}`); } return lines.join('\n'); } diff --git a/src/lib/zplImportService.test.ts b/src/lib/zplImportService.test.ts index c455b802..3e23bb32 100644 --- a/src/lib/zplImportService.test.ts +++ b/src/lib/zplImportService.test.ts @@ -9,12 +9,6 @@ describe('importZplText — single label', () => { expect(result.pages[0]?.objects).toHaveLength(1); }); - it('reports a single-page notice', () => { - const zpl = '^XA^FO10,20^A0N,30,0^FDHello^FS^XZ'; - const result = importZplText(zpl, 8); - expect(result.notice).toContain('1 object'); - expect(result.notice).not.toContain('pages'); - }); }); describe('importZplText — multi-label', () => { @@ -31,12 +25,6 @@ describe('importZplText — multi-label', () => { expect(result.pages[2]?.objects).toHaveLength(1); }); - it('mentions the page count in the notice', () => { - const zpl = '^XA^FDOne^FS^XZ\n^XA^FDTwo^FS^XZ'; - const result = importZplText(zpl, 8); - expect(result.notice).toContain('across 2 pages'); - }); - it('uses the first block\'s label dimensions', () => { const zpl = [ '^XA^PW800^LL400^FDOne^FS^XZ', @@ -47,24 +35,6 @@ describe('importZplText — multi-label', () => { expect(result.labelConfig.heightMm).toBe(50); }); - it('flags differing dimensions in the notice', () => { - const zpl = [ - '^XA^PW800^LL400^FDOne^FS^XZ', - '^XA^PW400^LL200^FDTwo^FS^XZ', - ].join('\n'); - const result = importZplText(zpl, 8); - expect(result.notice).toContain('different dimensions'); - }); - - it('does not flag dimensions when they match', () => { - const zpl = [ - '^XA^PW800^LL400^FDOne^FS^XZ', - '^XA^PW800^LL400^FDTwo^FS^XZ', - ].join('\n'); - const result = importZplText(zpl, 8); - expect(result.notice).not.toContain('different dimensions'); - }); - it('discards content before the first ^XA', () => { const zpl = 'garbage text before\n^XA^FDOne^FS^XZ'; const result = importZplText(zpl, 8); @@ -82,6 +52,51 @@ describe('importZplText — empty / malformed', () => { it('returns no pages when no ^XA is present', () => { const result = importZplText('not zpl at all', 8); expect(result.pages).toHaveLength(0); - expect(result.notice).toContain('No labels found'); + }); + + it('returns an empty findings list with empty buckets', () => { + const result = importZplText('not zpl at all', 8); + expect(result.report.findings).toEqual([]); + expect(result.report.partial).toEqual([]); + }); +}); + +describe('importZplText: findings.pageIndex', () => { + it('stamps page 0 on findings from the first block', () => { + const zpl = '^XA^FO0,0^A@N,30,0,E:A.TTF^FDx^FS^XZ'; + const { report } = importZplText(zpl, 8); + const partial = report.findings.filter((f) => f.kind === 'partial'); + expect(partial).toHaveLength(1); + expect(partial[0]?.pageIndex).toBe(0); + }); + + it('stamps the correct page index across multiple blocks', () => { + // Page 0: clean. Page 1: ^A@ partial. Page 2: ^IM browser-limit + ^XX unknown. + const zpl = [ + '^XA^FO0,0^A0N,30,0^FDclean^FS^XZ', + '^XA^FO0,0^A@N,30,0,E:A.TTF^FDfont^FS^XZ', + '^XA^IMR:LOGO.GRF^XX99^XZ', + ].join('\n'); + const { report } = importZplText(zpl, 8); + const byPage = (idx: number) => + report.findings.filter((f) => f.pageIndex === idx).map((f) => f.kind); + expect(byPage(0)).toEqual([]); + expect(byPage(1)).toContain('partial'); + expect(byPage(2)).toContain('browserLimit'); + expect(byPage(2)).toContain('unknown'); + }); + + it('per-page partial dedup is preserved (one partial per page even with two ^A@)', () => { + // Inside page 1, ^A@ appears twice but is deduped per-block. + const zpl = [ + '^XA^FO0,0^A0N,30,0^FDclean^FS^XZ', + '^XA^FO0,0^A@N,30,0,E:A.TTF^FDa^FS^FO0,50^A@N,30,0,E:B.TTF^FDb^FS^XZ', + ].join('\n'); + const { report } = importZplText(zpl, 8); + const onPage1 = report.findings.filter( + (f) => f.kind === 'partial' && f.pageIndex === 1, + ); + expect(onPage1).toHaveLength(1); + expect(onPage1[0]?.command).toBe('^A@'); }); }); diff --git a/src/lib/zplImportService.ts b/src/lib/zplImportService.ts index 774a61ca..6e170000 100644 --- a/src/lib/zplImportService.ts +++ b/src/lib/zplImportService.ts @@ -1,4 +1,4 @@ -import { parseZPL, type ImportReport } from "./zplParser"; +import { parseZPL, type ImportFinding, type ImportFindingKind, type ImportReport } from "./zplParser"; import type { LabelConfig } from "../types/ObjectType"; import type { LabelObject } from "../registry"; @@ -6,7 +6,6 @@ export interface ZplImportResult { labelConfig: Partial; pages: { objects: LabelObject[] }[]; report: ImportReport; - notice: string; } /** @@ -30,76 +29,39 @@ export function importZplText(zpl: string, dpmm: number): ZplImportResult { return { labelConfig: {}, pages: [], - report: { partial: [], browserLimit: [], unknown: [] }, - notice: 'No labels found in the ZPL code.', + report: { findings: [], partial: [], browserLimit: [], unknown: [] }, }; } let labelConfig: Partial = {}; const pages: { objects: LabelObject[] }[] = []; - const partial: string[] = []; - const browserLimit: string[] = []; - const unknown: string[] = []; - let dimensionsDiffered = false; + const findings: ImportFinding[] = []; blocks.forEach((block, i) => { const result = parseZPL(block, dpmm); pages.push({ objects: result.objects }); if (i === 0) { labelConfig = result.labelConfig; - } else { - const cfg = result.labelConfig; - if ( - (cfg.widthMm !== undefined && cfg.widthMm !== labelConfig.widthMm) || - (cfg.heightMm !== undefined && cfg.heightMm !== labelConfig.heightMm) || - (cfg.dpmm !== undefined && cfg.dpmm !== labelConfig.dpmm) - ) { - dimensionsDiffered = true; - } } - partial.push(...result.importReport.partial); - browserLimit.push(...result.importReport.browserLimit); - unknown.push(...result.importReport.unknown); + // Per-block findings come from the parser with pageIndex=0; stamp the + // real page index here so the UI can navigate to them. + for (const f of result.importReport.findings) { + findings.push({ ...f, pageIndex: i }); + } }); + // Bucket views deduplicate by command code to match the JSDoc contract on + // ImportReport (see zplParser.ts). The per-occurrence model lives in + // `findings`; consumers that only need the set of distinct affected + // commands read these buckets unchanged. + const dedupBy = (kind: ImportFindingKind) => + [...new Set(findings.filter((f) => f.kind === kind).map((f) => f.command))]; const report: ImportReport = { - partial: [...new Set(partial)], - browserLimit: [...new Set(browserLimit)], - unknown: [...new Set(unknown)], + findings, + partial: dedupBy('partial'), + browserLimit: dedupBy('browserLimit'), + unknown: dedupBy('unknown'), }; - const objectCount = pages.reduce((s, p) => s + p.objects.length, 0); - const notice = buildNotice(objectCount, pages.length, report, dimensionsDiffered); - - return { labelConfig, pages, report, notice }; -} - -function buildNotice( - objectCount: number, - pageCount: number, - report: ImportReport, - dimensionsDiffered: boolean, -): string { - const parts: string[] = []; - - const objectsText = `${objectCount} object${objectCount !== 1 ? 's' : ''}`; - if (pageCount > 1) { - parts.push(`Editable reconstruction: ${objectsText} across ${pageCount} pages imported.`); - } else { - parts.push(`Editable reconstruction: ${objectsText} imported.`); - } - - if (dimensionsDiffered) { - parts.push(`Pages have different dimensions; using the first page's size.`); - } - - if (report.partial.length > 0) { - parts.push(`Font face not preserved (${report.partial.join(', ')}).`); - } - const skippedCount = report.browserLimit.length + report.unknown.length; - if (skippedCount > 0) { - parts.push(`${skippedCount} command${skippedCount !== 1 ? 's' : ''} skipped.`); - } - - return parts.join(' '); + return { labelConfig, pages, report }; } diff --git a/src/lib/zplParser.test.ts b/src/lib/zplParser.test.ts index 712bc7c2..c4921bec 100644 --- a/src/lib/zplParser.test.ts +++ b/src/lib/zplParser.test.ts @@ -954,6 +954,28 @@ describe('parseZPL — importReport.unknown', () => { expect(importReport.partial).toHaveLength(0); expect(importReport.browserLimit).toHaveLength(0); expect(importReport.unknown).toHaveLength(0); + expect(importReport.findings).toHaveLength(0); + }); +}); + +describe('parseZPL: importReport.findings', () => { + it('emits one finding per kind with pageIndex=0 (set by service layer later)', () => { + const zpl = '^XA^FO0,0^A@N,30,0,E:ARIAL.TTF^FDText^FS^IMR:LOGO.GRF^XX99^XZ'; + const { importReport } = parseZPL(zpl, 8); + const kinds = importReport.findings.map((f) => f.kind); + expect(kinds).toContain('partial'); + expect(kinds).toContain('browserLimit'); + expect(kinds).toContain('unknown'); + expect(importReport.findings.every((f) => f.pageIndex === 0)).toBe(true); + }); + + it('partial findings are deduplicated by command code', () => { + // Two ^A@ uses → one partial finding for "^A@". + const zpl = '^XA^FO0,0^A@N,30,0,E:A.TTF^FDFirst^FS^FO0,50^A@N,30,0,E:B.TTF^FDSecond^FS^XZ'; + const { importReport } = parseZPL(zpl, 8); + const partialFindings = importReport.findings.filter((f) => f.kind === 'partial'); + expect(partialFindings).toHaveLength(1); + expect(partialFindings[0]?.command).toBe('^A@'); }); }); diff --git a/src/lib/zplParser.ts b/src/lib/zplParser.ts index 7535d403..3067452b 100644 --- a/src/lib/zplParser.ts +++ b/src/lib/zplParser.ts @@ -21,13 +21,40 @@ import type { CodablockProps } from "../registry/codablock"; import { putImage } from "./imageCache"; import { GS1_DATABAR_DEFAULT_SEGMENTS } from "./gs1"; +export type ImportFindingKind = 'partial' | 'browserLimit' | 'unknown'; + +/** + * One import finding. Created per-occurrence so each entry can be navigated + * to its source page in the UI; cross-block dedup happens (if at all) in the + * service layer that merges per-page parser runs. + */ +export interface ImportFinding { + kind: ImportFindingKind; + /** Command token. For 'partial' the bare code (e.g. "^A@"); for + * 'browserLimit' / 'unknown' the full token including parameters + * (e.g. "^IM,R:LOGO.GRF"). Matches the pre-finding-restructure + * format so existing UI / format helpers keep working. */ + command: string; + /** Page index this finding originated from. The parser doesn't know about + * pages and emits 0; `zplImportService` overwrites it when it merges the + * per-block parser results into a multi-page report. */ + pageIndex: number; +} + /** * Categorised import report produced alongside the parsed objects. - * Enables the UI to give the user precise, actionable feedback. + * `findings` is the source of truth; the three string buckets are derived + * views kept for backward compatibility with existing parser tests and + * the textual report formatter. */ export interface ImportReport { + /** Per-block findings grouped by kind (partial, then browserLimit, then + * unknown). Inside each group, entries are in encounter order; the + * partial group is deduplicated by command code. The derived arrays + * below offer a kind-filtered view. */ + findings: ImportFinding[]; /** Commands that were imported with known loss (e.g. ^A@ → font face not available in browser). - * An object WAS created; something about it is approximate. Deduplicated by command code. */ + * An object WAS created; something about it is approximate. Deduplicated by command code. */ partial: string[]; /** Commands skipped because they require printer hardware or file storage * (e.g. ^IM, ~DG). No object was created for these. */ @@ -1272,11 +1299,22 @@ export function parseZPL(zpl: string, dpmm = 8): ParsedZPL { } } + // Build findings list. `partialCmds` is deduplicated by command code; + // the others are per-occurrence in encounter order. pageIndex stays 0 + // here; `zplImportService.importZplText` fills it once it knows which + // ^XA…^XZ block this came from. + const findings: ImportFinding[] = [ + ...[...partialCmds].map((command): ImportFinding => ({ kind: 'partial', command, pageIndex: 0 })), + ...browserLimit.map((command): ImportFinding => ({ kind: 'browserLimit', command, pageIndex: 0 })), + ...unknown.map((command): ImportFinding => ({ kind: 'unknown', command, pageIndex: 0 })), + ]; + return { labelConfig, objects, skipped, importReport: { + findings, partial: [...partialCmds], browserLimit, unknown,