From 4fd8ad284fa5edef5a8e580c9c6edad0c5f4afb0 Mon Sep 17 00:00:00 2001
From: Jan-Henrik Bruhn <jan-henrik.bruhn@offis.de>
Date: Thu, 26 Mar 2026 12:26:05 +0100
Subject: [PATCH] fix: Improve step control UX and fix machine error display

- Consolidate progress stats into 3 cards (stitches, time, speed)
- Keep rollback info visible after machine clears error while paused
- Remove Resume/Start Sewing buttons in STOP state (error must be
  resolved on machine first)
- Use adjustedStitchIndex for progress display to prevent desync
- Make step control layout stable (always render all buttons)
- Reduce polling interval from 500ms to 1000ms during sewing
- Fix machine errors (e.g. HoopError) not showing in error badge
  when there was no accompanying string error message

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 src/components/AppHeader.tsx                  |  8 +-
 .../ProgressMonitor/ProgressMonitor.tsx       | 21 ++---
 .../ProgressMonitor/ProgressSection.tsx       | 33 +------
 .../ProgressMonitor/ProgressStats.tsx         | 18 ++--
 .../ProgressMonitor/StitchStepControl.tsx     | 92 +++++++++----------
 src/stores/useMachineStore.ts                 | 13 +--
 src/utils/machineStateHelpers.ts              | 17 ++--
 7 files changed, 85 insertions(+), 117 deletions(-)

diff --git a/src/components/AppHeader.tsx b/src/components/AppHeader.tsx
index 51af7f1..a234581 100644
--- a/src/components/AppHeader.tsx
+++ b/src/components/AppHeader.tsx
@@ -187,7 +187,9 @@ export function AppHeader() {
                     <button
                       className={cn(
                         "inline-flex items-center rounded-full border border-transparent bg-destructive text-white px-2.5 py-1.5 text-xs font-semibold gap-1.5 cursor-pointer hover:bg-destructive/90 transition-all focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-destructive focus-visible:ring-offset-2",
-                        machineErrorMessage || pyodideError
+                        machineErrorMessage ||
+                          pyodideError ||
+                          hasError(machineError)
                           ? "animate-pulse hover:animate-none"
                           : "invisible pointer-events-none",
                       )}
@@ -228,7 +230,9 @@ export function AppHeader() {
                   </PopoverTrigger>
 
                   {/* Error popover content - unchanged */}
-                  {(machineErrorMessage || pyodideError) && (
+                  {(machineErrorMessage ||
+                    pyodideError ||
+                    hasError(machineError)) && (
                     <ErrorPopoverContent
                       machineError={
                         machineError != 0xdd ? machineError : undefined
diff --git a/src/components/ProgressMonitor/ProgressMonitor.tsx b/src/components/ProgressMonitor/ProgressMonitor.tsx
index d0a05f6..2d26bad 100644
--- a/src/components/ProgressMonitor/ProgressMonitor.tsx
+++ b/src/components/ProgressMonitor/ProgressMonitor.tsx
@@ -78,19 +78,18 @@ export function ProgressMonitor() {
       : patternInfo.totalStitches
     : 0;
 
+  // Use adjustedStitchIndex (from step control) when available, otherwise machine-reported
+  const currentStitch =
+    adjustedStitchIndex ?? sewingProgress?.currentStitch ?? 0;
+
   const progressPercent =
-    totalStitches > 0
-      ? ((sewingProgress?.currentStitch || 0) / totalStitches) * 100
-      : 0;
+    totalStitches > 0 ? (currentStitch / totalStitches) * 100 : 0;
 
   // Calculate color block information from decoded penStitches
   const colorBlocks = useMemo(
     () => calculateColorBlocks(displayPattern),
     [displayPattern],
   );
-
-  // Determine current color block based on current stitch
-  const currentStitch = sewingProgress?.currentStitch || 0;
   const currentBlockIndex = findCurrentBlockIndex(colorBlocks, currentStitch);
 
   // Calculate time based on color blocks (matches Brother app calculation)
@@ -127,7 +126,9 @@ export function ProgressMonitor() {
         {/* Pattern Info */}
         {patternInfo && (
           <ProgressStats
+            currentStitch={currentStitch}
             totalStitches={totalStitches}
+            elapsedMinutes={elapsedMinutes}
             totalMinutes={totalMinutes}
             speed={patternInfo.speed}
           />
@@ -135,13 +136,7 @@ export function ProgressMonitor() {
 
         {/* Progress Bar */}
         {sewingProgress && (
-          <ProgressSection
-            currentStitch={sewingProgress.currentStitch}
-            totalStitches={totalStitches}
-            elapsedMinutes={elapsedMinutes}
-            totalMinutes={totalMinutes}
-            progressPercent={progressPercent}
-          />
+          <ProgressSection progressPercent={progressPercent} />
         )}
 
         {/* Color Blocks */}
diff --git a/src/components/ProgressMonitor/ProgressSection.tsx b/src/components/ProgressMonitor/ProgressSection.tsx
index 4370e13..19ee488 100644
--- a/src/components/ProgressMonitor/ProgressSection.tsx
+++ b/src/components/ProgressMonitor/ProgressSection.tsx
@@ -1,49 +1,22 @@
 /**
  * ProgressSection Component
  *
- * Displays the progress bar and current/total stitch information
+ * Displays the progress bar
  */
 
 import { Progress } from "@/components/ui/progress";
 
 interface ProgressSectionProps {
-  currentStitch: number;
-  totalStitches: number;
-  elapsedMinutes: number;
-  totalMinutes: number;
   progressPercent: number;
 }
 
-export function ProgressSection({
-  currentStitch,
-  totalStitches,
-  elapsedMinutes,
-  totalMinutes,
-  progressPercent,
-}: ProgressSectionProps) {
+export function ProgressSection({ progressPercent }: ProgressSectionProps) {
   return (
     <div className="mb-3">
       <Progress
         value={progressPercent}
-        className="h-3 mb-2 [&>div]:bg-gradient-to-r [&>div]:from-accent-600 [&>div]:to-accent-700 dark:[&>div]:from-accent-600 dark:[&>div]:to-accent-800"
+        className="h-3 [&>div]:bg-gradient-to-r [&>div]:from-accent-600 [&>div]:to-accent-700 dark:[&>div]:from-accent-600 dark:[&>div]:to-accent-800"
       />
-
-      <div className="grid grid-cols-2 gap-2 text-xs mb-3">
-        <div className="bg-gray-200 dark:bg-gray-700/50 p-2 rounded">
-          <span className="text-gray-600 dark:text-gray-400 block">
-            Current Stitch
-          </span>
-          <span className="font-semibold text-gray-900 dark:text-gray-100">
-            {currentStitch.toLocaleString()} / {totalStitches.toLocaleString()}
-          </span>
-        </div>
-        <div className="bg-gray-200 dark:bg-gray-700/50 p-2 rounded">
-          <span className="text-gray-600 dark:text-gray-400 block">Time</span>
-          <span className="font-semibold text-gray-900 dark:text-gray-100">
-            {elapsedMinutes} / {totalMinutes} min
-          </span>
-        </div>
-      </div>
     </div>
   );
 }
diff --git a/src/components/ProgressMonitor/ProgressStats.tsx b/src/components/ProgressMonitor/ProgressStats.tsx
index d65945c..552f717 100644
--- a/src/components/ProgressMonitor/ProgressStats.tsx
+++ b/src/components/ProgressMonitor/ProgressStats.tsx
@@ -1,36 +1,36 @@
 /**
  * ProgressStats Component
  *
- * Displays three stat cards: total stitches, total time, and speed
+ * Displays three stat cards: stitches (current/total), time (elapsed/total), and speed
  */
 
 interface ProgressStatsProps {
+  currentStitch: number;
   totalStitches: number;
+  elapsedMinutes: number;
   totalMinutes: number;
   speed: number;
 }
 
 export function ProgressStats({
+  currentStitch,
   totalStitches,
+  elapsedMinutes,
   totalMinutes,
   speed,
 }: ProgressStatsProps) {
   return (
     <div className="grid grid-cols-3 gap-2 text-xs mb-3">
       <div className="bg-gray-200 dark:bg-gray-700/50 p-2 rounded">
-        <span className="text-gray-600 dark:text-gray-400 block">
-          Total Stitches
-        </span>
+        <span className="text-gray-600 dark:text-gray-400 block">Stitches</span>
         <span className="font-semibold text-gray-900 dark:text-gray-100">
-          {totalStitches.toLocaleString()}
+          {currentStitch.toLocaleString()} / {totalStitches.toLocaleString()}
         </span>
       </div>
       <div className="bg-gray-200 dark:bg-gray-700/50 p-2 rounded">
-        <span className="text-gray-600 dark:text-gray-400 block">
-          Total Time
-        </span>
+        <span className="text-gray-600 dark:text-gray-400 block">Time</span>
         <span className="font-semibold text-gray-900 dark:text-gray-100">
-          {totalMinutes} min
+          {elapsedMinutes} / {totalMinutes} min
         </span>
       </div>
       <div className="bg-gray-200 dark:bg-gray-700/50 p-2 rounded">
diff --git a/src/components/ProgressMonitor/StitchStepControl.tsx b/src/components/ProgressMonitor/StitchStepControl.tsx
index 1f265a9..ce479bb 100644
--- a/src/components/ProgressMonitor/StitchStepControl.tsx
+++ b/src/components/ProgressMonitor/StitchStepControl.tsx
@@ -1,7 +1,7 @@
 /**
  * StitchStepControl Component
  *
- * Manual stitch position control shown when machine is paused/stopped/interrupted.
+ * Compact stitch position control shown when machine is paused/stopped/interrupted.
  * Allows stepping forward/backward by 1, 10, or 100 stitches,
  * jumping to thread color boundaries, and resetting to current position.
  */
@@ -15,7 +15,10 @@ import {
   ArrowUturnLeftIcon,
 } from "@heroicons/react/24/solid";
 import { Button } from "@/components/ui/button";
-import { getErrorStitchRollback, getErrorMessage } from "../../utils/errorCodeHelpers";
+import {
+  getErrorStitchRollback,
+  getErrorMessage,
+} from "../../utils/errorCodeHelpers";
 import type { ColorBlock } from "./types";
 import { findCurrentBlockIndex } from "../../utils/colorBlockHelpers";
 
@@ -42,7 +45,6 @@ export function StitchStepControl({
 }: StitchStepControlProps) {
   const displayStitch = adjustedStitchIndex ?? currentStitch;
 
-  // Find the start of the current thread color block
   const handleGoToThreadStart = () => {
     const blockIndex = findCurrentBlockIndex(colorBlocks, displayStitch);
     if (blockIndex >= 0) {
@@ -50,14 +52,12 @@ export function StitchStepControl({
     }
   };
 
-  // Reset to the position when the machine was paused (after auto-rollback, before manual adjustments)
   const handleGoToPausedStitch = () => {
     if (pausedStitchIndex !== null) {
       onSetPosition(pausedStitchIndex);
     }
   };
 
-  // Rollback info text
   const rollbackAmount = lastRolledBackError
     ? getErrorStitchRollback(lastRolledBackError)
     : null;
@@ -65,31 +65,48 @@ export function StitchStepControl({
     ? getErrorMessage(lastRolledBackError)
     : null;
 
-  return (
-    <div className="mb-3 bg-gray-200 dark:bg-gray-700/50 p-3 rounded-lg">
-      <div className="text-xs font-medium text-gray-600 dark:text-gray-400 mb-2">
-        Stitch Position
-      </div>
+  const showGoToPaused =
+    pausedStitchIndex !== null && displayStitch !== pausedStitchIndex;
 
-      {/* Current position display */}
-      <div className="text-center mb-2">
-        <span className="text-lg font-bold text-gray-900 dark:text-gray-100 tabular-nums">
-          {displayStitch.toLocaleString()}
+  return (
+    <div className="mb-3 bg-gray-200 dark:bg-gray-700/50 px-3 py-2 rounded-lg">
+      {/* Header: label + stitch count on one line */}
+      <div className="flex items-baseline justify-between mb-1.5">
+        <span className="text-xs font-medium text-gray-600 dark:text-gray-400">
+          Stitch Position
         </span>
-        <span className="text-xs text-gray-500 dark:text-gray-400 ml-1">
-          / {totalStitches.toLocaleString()}
+        <span>
+          <span className="text-sm font-bold text-gray-900 dark:text-gray-100 tabular-nums">
+            {displayStitch.toLocaleString()}
+          </span>
+          <span className="text-xs text-gray-500 dark:text-gray-400 ml-1">
+            / {totalStitches.toLocaleString()}
+          </span>
         </span>
       </div>
 
       {/* Rollback info */}
       {rollbackAmount !== null && rollbackErrorName && (
-        <div className="text-xs text-amber-600 dark:text-amber-400 text-center mb-2">
-          Moved back {rollbackAmount} stitches due to {rollbackErrorName.toLowerCase()}
+        <div className="text-xs text-amber-600 dark:text-amber-400 text-center mb-1.5">
+          Moved back {rollbackAmount} stitches (
+          {rollbackErrorName.toLowerCase()})
         </div>
       )}
 
-      {/* Step buttons */}
+      {/* Step buttons + navigation in one row */}
       <div className="flex items-center justify-center gap-1">
+        {colorBlocks.length > 0 && (
+          <Button
+            variant="outline"
+            size="icon-sm"
+            onClick={handleGoToThreadStart}
+            title="Go to the beginning of the selected thread color"
+            aria-label="Go to the beginning of the selected thread color"
+          >
+            <SwatchIcon className="w-4 h-4" />
+          </Button>
+        )}
+
         <Button
           variant="outline"
           size="icon-sm"
@@ -153,34 +170,17 @@ export function StitchStepControl({
         >
           <ChevronDoubleRightIcon className="w-4 h-4" />
         </Button>
-      </div>
 
-      {/* Navigation buttons */}
-      <div className="flex items-center justify-center gap-1 mt-2">
-        {colorBlocks.length > 0 && (
-          <Button
-            variant="outline"
-            size="sm"
-            onClick={handleGoToThreadStart}
-            title="Go to the beginning of the selected thread color"
-            aria-label="Go to the beginning of the selected thread color"
-          >
-            <SwatchIcon className="w-3.5 h-3.5" />
-            Thread Start
-          </Button>
-        )}
-        {pausedStitchIndex !== null && displayStitch !== pausedStitchIndex && (
-          <Button
-            variant="outline"
-            size="sm"
-            onClick={handleGoToPausedStitch}
-            title="Go to the current stitch"
-            aria-label="Go to the current stitch"
-          >
-            <ArrowUturnLeftIcon className="w-3.5 h-3.5" />
-            Current Stitch
-          </Button>
-        )}
+        <Button
+          variant="outline"
+          size="icon-sm"
+          onClick={handleGoToPausedStitch}
+          disabled={!showGoToPaused}
+          title="Go to the current stitch"
+          aria-label="Go to the current stitch"
+        >
+          <ArrowUturnLeftIcon className="w-4 h-4" />
+        </Button>
       </div>
     </div>
   );
diff --git a/src/stores/useMachineStore.ts b/src/stores/useMachineStore.ts
index 8dfbc17..de9faa3 100644
--- a/src/stores/useMachineStore.ts
+++ b/src/stores/useMachineStore.ts
@@ -487,13 +487,8 @@ export const useMachineStore = create<MachineState>((set, get) => ({
           });
         }
       }
-      // Reset rollback tracking when error clears while still paused
-      else if (
-        currentState.lastRolledBackError !== null &&
-        currentState.machineError === SewingMachineError.None
-      ) {
-        set({ lastRolledBackError: null });
-      }
+      // Note: we intentionally do NOT clear lastRolledBackError when the error clears
+      // while still paused, so the rollback info text remains visible to the user.
 
       // Auto-rollback for thread errors when machine is interrupted or paused mid-sew
       // Only runs once on entering paused state (when pausedStitchIndex is not yet set)
@@ -508,9 +503,7 @@ export const useMachineStore = create<MachineState>((set, get) => ({
 
         // Snapshot the paused position (after rollback, before manual adjustments)
         const postRollbackStitch =
-          get().adjustedStitchIndex ??
-          get().sewingProgress?.currentStitch ??
-          0;
+          get().adjustedStitchIndex ?? get().sewingProgress?.currentStitch ?? 0;
         set({ pausedStitchIndex: postRollbackStitch });
       }
 
diff --git a/src/utils/machineStateHelpers.ts b/src/utils/machineStateHelpers.ts
index a933281..fc57c5b 100644
--- a/src/utils/machineStateHelpers.ts
+++ b/src/utils/machineStateHelpers.ts
@@ -97,10 +97,8 @@ export function canUploadPattern(status: MachineStatus): boolean {
 export function canStartSewing(status: MachineStatus): boolean {
   // Only in specific ready states
   return (
-    status === MachineStatus.SEWING_WAIT ||
     status === MachineStatus.MASK_TRACE_COMPLETE ||
     status === MachineStatus.PAUSE ||
-    status === MachineStatus.STOP ||
     status === MachineStatus.SEWING_INTERRUPTION
   );
 }
@@ -114,7 +112,10 @@ export function canStartMaskTrace(
   status: MachineStatus,
   hasSewingProgress = false,
 ): boolean {
-  if (status === MachineStatus.IDLE || status === MachineStatus.MASK_TRACE_COMPLETE) {
+  if (
+    status === MachineStatus.IDLE ||
+    status === MachineStatus.MASK_TRACE_COMPLETE
+  ) {
     return true;
   }
   // Only allow mask trace in SEWING_WAIT if sewing hasn't started yet
@@ -126,12 +127,14 @@ export function canStartMaskTrace(
 
 /**
  * Determines if sewing can be resumed in the current state.
- * Only for interrupted operations (PAUSE, STOP, SEWING_INTERRUPTION).
+ * Only for PAUSE and SEWING_INTERRUPTION - not STOP, which requires
+ * the user to resolve the error on the machine first.
  */
 export function canResumeSewing(status: MachineStatus): boolean {
-  // Only in interrupted states
-  const category = getMachineStateCategory(status);
-  return category === MachineStateCategory.INTERRUPTED;
+  return (
+    status === MachineStatus.PAUSE ||
+    status === MachineStatus.SEWING_INTERRUPTION
+  );
 }
 
 /**