Merge pull request #62 from u8array/feat/import-report-clickable

refactor(import-report): findings-only summary, silent clean import

Author: u8array

src/components/Output/ImportReportModal.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<ImportFindingKind, string> = {
+  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 (
+    <div className="flex items-start gap-2 px-3 py-2">
+      {showPage && (
+        <span className="font-mono text-[9px] uppercase tracking-wider text-muted shrink-0 mt-0.5">
+          Page&nbsp;{finding.pageIndex + 1}
+        </span>
+      )}
+      <div className="flex flex-col gap-0.5 flex-1 min-w-0">
+        <span className={`font-mono text-[10px] font-semibold ${TONE[finding.kind]}`}>
+          {title}
+        </span>
+        <span className="font-mono text-[10px] text-muted truncate">
+          {detail}
+        </span>
+      </div>
+    </div>
+  );
+}
+
+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 (
+    <div className="flex flex-col gap-3 p-4 flex-1 min-h-0 overflow-y-auto">
+      <p className="font-mono text-[10px] text-amber-400 leading-relaxed">
+        Imported {objectCount} object{objectCount !== 1 ? 's' : ''} with{' '}
+        {findings.length} issue{findings.length !== 1 ? 's' : ''}:
+      </p>
+      <div className="flex flex-col">
+        {findings.map((f, i) => (
+          <FindingRow
+            key={`${f.pageIndex}-${f.kind}-${f.command}-${i}`}
+            finding={f}
+            showPage={multiPage}
+          />
+        ))}
+      </div>
+    </div>
+  );
+}

src/components/Output/ImportSummary.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<LabelConfig>, 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<HTMLInputElement>) => {
@@ -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 = () => {

src/components/Output/ZplImportModal.tsx

@@ -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');
 }