fix: address additional PR review feedback

stefandevo · stefandevo · commit 3714cadf4fba · 2026-01-18T15:25:24.000+01:00
- Fix Textarea controlled/uncontrolled warning with default value
- Preserve IDs in useEffect sync to avoid unnecessary remounts
- Consolidate lucide-react imports
- Add JSDoc note about tag attributes limitation in xml-utils.ts
- Remove redundant disabled prop from SpecModeTabs
diff --git a/apps/ui/src/components/views/spec-view.tsx b/apps/ui/src/components/views/spec-view.tsx
@@ -200,12 +200,7 @@ export function SpecView() {
         {/* Mode tabs bar - inside the content area, centered */}
         {!isProcessing && (
           <div className="flex items-center justify-center px-4 py-2 border-b border-border bg-muted/30 relative">
-            <SpecModeTabs
-              mode={mode}
-              onModeChange={handleModeChange}
-              isParseValid={isParseValid}
-              disabled={isProcessing}
-            />
+            <SpecModeTabs mode={mode} onModeChange={handleModeChange} isParseValid={isParseValid} />
             {/* Show parse error indicator - positioned to the right */}
             {!isParseValid && parseErrors.length > 0 && (
               <span className="absolute right-4 text-xs text-destructive">
diff --git a/apps/ui/src/components/views/spec-view/components/edit-mode/roadmap-section.tsx b/apps/ui/src/components/views/spec-view/components/edit-mode/roadmap-section.tsx
@@ -1,4 +1,4 @@
-import { Plus, X } from 'lucide-react';
+import { Plus, X, Map as MapIcon } from 'lucide-react';
 import { useState, useRef, useEffect } from 'react';
 import { Button } from '@/components/ui/button';
 import { Input } from '@/components/ui/input';
@@ -12,7 +12,6 @@ import {
   SelectTrigger,
   SelectValue,
 } from '@/components/ui/select';
-import { Map as MapIcon } from 'lucide-react';
 import type { SpecOutput } from '@automaker/spec-parser';
 
 type RoadmapPhase = NonNullable<SpecOutput['implementation_roadmap']>[number];
@@ -90,7 +89,7 @@ function PhaseCard({ phase, onChange, onRemove }: PhaseCardProps) {
             <div>
               <Label className="sr-only">Description</Label>
               <Textarea
-                value={phase.description}
+                value={phase.description ?? ''}
                 onChange={(e) => handleDescriptionChange(e.target.value)}
                 placeholder="Describe what this phase involves..."
                 rows={2}
@@ -120,12 +119,24 @@ export function RoadmapSection({ phases, onChange }: RoadmapSectionProps) {
   const isInternalChange = useRef(false);
 
   // Sync external phases to internal items when phases change externally
+  // Preserve existing IDs where possible to avoid unnecessary remounts
   useEffect(() => {
     if (isInternalChange.current) {
       isInternalChange.current = false;
       return;
     }
-    setItems(phases.map(phaseToInternal));
+    setItems((currentItems) => {
+      return phases.map((phase, index) => {
+        // Try to find existing item by index (positional matching)
+        const existingItem = currentItems[index];
+        if (existingItem) {
+          // Reuse the existing ID, update the phase data
+          return { ...phase, _id: existingItem._id };
+        }
+        // New phase - generate new ID
+        return phaseToInternal(phase);
+      });
+    });
   }, [phases]);
 
   const handleAdd = () => {
diff --git a/libs/spec-parser/src/xml-utils.ts b/libs/spec-parser/src/xml-utils.ts
@@ -41,6 +41,9 @@ function escapeRegExp(value: string): string {
 /**
  * Extract the content of a specific XML section.
  *
+ * Note: This function only matches bare tags without attributes.
+ * Tags with attributes (e.g., `<tag id="1">`) are not supported.
+ *
  * @param xmlContent - The full XML content
  * @param tagName - The tag name to extract (e.g., 'implemented_features')
  * @returns The content between the tags, or null if not found
@@ -55,6 +58,9 @@ export function extractXmlSection(xmlContent: string, tagName: string): string |
 /**
  * Extract all values from repeated XML elements.
  *
+ * Note: This function only matches bare tags without attributes.
+ * Tags with attributes (e.g., `<tag id="1">`) are not supported.
+ *
  * @param xmlContent - The XML content to search
  * @param tagName - The tag name to extract values from
  * @returns Array of extracted values (unescaped and trimmed)
