From 6b6ce77a6789ae8036b427fcbbe51c8151a24c06 Mon Sep 17 00:00:00 2001 From: u8array Date: Wed, 13 May 2026 20:46:16 +0200 Subject: [PATCH 1/3] refactor(import-report): findings-only summary, silent clean import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restructure the import report so each issue is a per-occurrence ImportFinding ({ kind, command, pageIndex }) instead of three dedup buckets. The parser emits findings with pageIndex=0; zplImportService stamps the real page index when it merges the per-^XA block results, which lets the modal show a 'Page N' badge next to each finding when the import spans multiple pages. Introduce describeFinding() in lib/importReport as the single source of truth for finding wording. Both the UI list and the copy-as-text formatter route through it so the two surfaces cannot drift. Replace the kind→tone switch with a Record so the compiler enforces exhaustiveness. Skip the post-import result view entirely when there are no findings: the canvas filling with the imported design is feedback enough, and the pre-import disclaimer already covers the editable-reconstruction caveat. Only stop on the result view when findings exist; that view is now findings-only (no duplicated success banner). Rename ImportReportModal.tsx to ImportSummary.tsx and drop the dead ImportReportModal wrapper (only ImportSummaryBody was ever used). Remove the dead notice/buildNotice path from zplImportService: the notice string and its builder were only read by tests; the modal surfaces import status through the findings list and its own error copy. Drop the dimensionsDiffered tracking that only fed the notice. --- src/components/Output/ImportReportModal.tsx | 107 -------------------- src/components/Output/ImportSummary.tsx | 61 +++++++++++ src/components/Output/ZplImportModal.tsx | 24 ++++- src/lib/importReport.ts | 61 ++++++++--- src/lib/zplImportService.test.ts | 77 ++++++++------ src/lib/zplImportService.ts | 73 +++---------- src/lib/zplParser.test.ts | 22 ++++ src/lib/zplParser.ts | 40 +++++++- 8 files changed, 252 insertions(+), 213 deletions(-) delete mode 100644 src/components/Output/ImportReportModal.tsx create mode 100644 src/components/Output/ImportSummary.tsx 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..7330a7cb --- /dev/null +++ b/src/components/Output/ImportSummary.tsx @@ -0,0 +1,61 @@ +import { describeFinding } from '../../lib/importReport'; +import type { ImportResult } from '../../lib/importReport'; +import type { ImportFinding, ImportFindingKind } from '../../lib/zplParser'; + +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..b6b8d751 100644 --- a/src/components/Output/ZplImportModal.tsx +++ b/src/components/Output/ZplImportModal.tsx @@ -5,7 +5,7 @@ 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 { 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); @@ -60,7 +60,15 @@ export function ZplImportModal({ onClose }: Props) { } applyImport(labelConfig, importedPages); - setResult({ objectCount: totalObjects, report }); + // Feedback through state change: when the import has no findings, + // the changed canvas is the confirmation, no extra modal step. + // 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. + if (report.findings.length === 0) { + onClose(); + } else { + setResult({ objectCount: totalObjects, report }); + } }; const handleFileSelect = async (e: React.ChangeEvent) => { @@ -91,7 +99,15 @@ export function ZplImportModal({ onClose }: Props) { } applyImport(labelConfig, importedPages); - setResult({ objectCount: totalObjects, report }); + // Feedback through state change: when the import has no findings, + // the changed canvas is the confirmation, no extra modal step. + // 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. + if (report.findings.length === 0) { + onClose(); + } else { + setResult({ objectCount: totalObjects, report }); + } }; const handleCopy = () => { diff --git a/src/lib/importReport.ts b/src/lib/importReport.ts index 3cc949ce..59d9dd7c 100644 --- a/src/lib/importReport.ts +++ b/src/lib/importReport.ts @@ -1,4 +1,4 @@ -import type { ImportReport } from './zplParser'; +import type { ImportFinding, ImportReport } from './zplParser'; import { ZPL_COMMAND_MAP } from './zplCommandSupport'; export interface ImportResult { @@ -13,25 +13,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..6c880071 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 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,36 @@ 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 }); + } }); const report: ImportReport = { - partial: [...new Set(partial)], - browserLimit: [...new Set(browserLimit)], - unknown: [...new Set(unknown)], + findings, + // Bucket views stay per-occurrence too (no Set dedup): the modal lists + // findings one-per-row so it can navigate to each affected page, and + // legacy text reports / parser tests read these arrays unchanged. + partial: findings.filter((f) => f.kind === 'partial').map((f) => f.command), + browserLimit: findings.filter((f) => f.kind === 'browserLimit').map((f) => f.command), + unknown: findings.filter((f) => f.kind === 'unknown').map((f) => f.command), }; - 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..ffc7a05e 100644 --- a/src/lib/zplParser.ts +++ b/src/lib/zplParser.ts @@ -21,13 +21,38 @@ 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-occurrence findings in encounter order, grouped by kind via the + * derived arrays below. */ + 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 +1297,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, From cedcda58eb38231f90c65807811ea1bad265b05a Mon Sep 17 00:00:00 2001 From: u8array Date: Wed, 13 May 2026 21:41:16 +0200 Subject: [PATCH 2/3] refactor(import-report): address PR #62 gemini review * Restore dedup on the legacy bucket views (partial / browserLimit / unknown) so they match the JSDoc contract on ImportReport. The per-occurrence model continues to live in `findings`. Wrapped the three near-identical filter/dedup expressions into a `dedupBy(kind)` closure. * Extract a `processImport` helper in ZplImportModal so the post-text path (parse, gate on supported content, hand off to store + modal finalisation) is no longer duplicated between the paste and file-pick handlers. The handlers now only carry source-specific validation; everything past that point lives in one place. * Re-export ImportFinding, ImportFindingKind and ImportReport from lib/importReport. UI components now reach the report domain types through that module exclusively, instead of importing from the parser directly. Keeps the parser as an implementation detail of the report surface. --- src/components/Output/ImportSummary.tsx | 3 +- src/components/Output/ZplImportModal.tsx | 64 +++++++++++------------- src/lib/importReport.ts | 7 ++- src/lib/zplImportService.ts | 17 ++++--- 4 files changed, 45 insertions(+), 46 deletions(-) diff --git a/src/components/Output/ImportSummary.tsx b/src/components/Output/ImportSummary.tsx index 7330a7cb..d45e6123 100644 --- a/src/components/Output/ImportSummary.tsx +++ b/src/components/Output/ImportSummary.tsx @@ -1,6 +1,5 @@ import { describeFinding } from '../../lib/importReport'; -import type { ImportResult } from '../../lib/importReport'; -import type { ImportFinding, ImportFindingKind } from '../../lib/zplParser'; +import type { ImportFinding, ImportFindingKind, ImportResult } from '../../lib/importReport'; export type { ImportResult }; diff --git a/src/components/Output/ZplImportModal.tsx b/src/components/Output/ZplImportModal.tsx index b6b8d751..277e51bb 100644 --- a/src/components/Output/ZplImportModal.tsx +++ b/src/components/Output/ZplImportModal.tsx @@ -4,7 +4,7 @@ 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 { formatReportAsText, type ImportReport, type ImportResult } from '../../lib/importReport'; import { ImportSummaryBody } from './ImportSummary'; import { useT } from '../../lib/useT'; import { DialogShell } from '../ui/DialogShell'; @@ -44,31 +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); - // Feedback through state change: when the import has no findings, - // the changed canvas is the confirmation, no extra modal step. - // 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. - if (report.findings.length === 0) { - onClose(); - } else { - 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) => { @@ -89,25 +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); - // Feedback through state change: when the import has no findings, - // the changed canvas is the confirmation, no extra modal step. - // 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. - if (report.findings.length === 0) { - onClose(); - } else { - setResult({ objectCount: totalObjects, report }); - } + processImport(text); }; const handleCopy = () => { diff --git a/src/lib/importReport.ts b/src/lib/importReport.ts index 59d9dd7c..3cd343b5 100644 --- a/src/lib/importReport.ts +++ b/src/lib/importReport.ts @@ -1,6 +1,11 @@ -import type { ImportFinding, 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; diff --git a/src/lib/zplImportService.ts b/src/lib/zplImportService.ts index 6c880071..6e170000 100644 --- a/src/lib/zplImportService.ts +++ b/src/lib/zplImportService.ts @@ -1,4 +1,4 @@ -import { parseZPL, type ImportFinding, 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"; @@ -50,14 +50,17 @@ export function importZplText(zpl: string, dpmm: number): ZplImportResult { } }); + // 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 = { findings, - // Bucket views stay per-occurrence too (no Set dedup): the modal lists - // findings one-per-row so it can navigate to each affected page, and - // legacy text reports / parser tests read these arrays unchanged. - partial: findings.filter((f) => f.kind === 'partial').map((f) => f.command), - browserLimit: findings.filter((f) => f.kind === 'browserLimit').map((f) => f.command), - unknown: findings.filter((f) => f.kind === 'unknown').map((f) => f.command), + partial: dedupBy('partial'), + browserLimit: dedupBy('browserLimit'), + unknown: dedupBy('unknown'), }; return { labelConfig, pages, report }; From 91016b10aef56e4eb3d8864875496bce80a70393 Mon Sep 17 00:00:00 2001 From: u8array Date: Wed, 13 May 2026 22:02:51 +0200 Subject: [PATCH 3/3] docs(import-report): correct findings ordering in ImportReport JSDoc The previous wording claimed 'encounter order' but the parser concatenates kind-buckets (partial, browserLimit, unknown) when building the findings array. Encounter order only holds within each group, plus partials are deduplicated by command code. Clarify the contract so consumers know what to expect. --- src/lib/zplParser.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/lib/zplParser.ts b/src/lib/zplParser.ts index ffc7a05e..3067452b 100644 --- a/src/lib/zplParser.ts +++ b/src/lib/zplParser.ts @@ -48,8 +48,10 @@ export interface ImportFinding { * the textual report formatter. */ export interface ImportReport { - /** Per-occurrence findings in encounter order, grouped by kind via the - * derived arrays below. */ + /** 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. */