feat: Add tree editor validation UI (Workstream A complete)
Implements comprehensive validation feedback system for tree editor: Task A.1 - Circular Reference Detection: - Added detectCircularRefs() function in treeEditorStore - Detects loops in both decision options and action next_node_id chains - Prevents infinite navigation paths Task A.2 - ValidationSummary Component: - Created collapsible panel showing error/warning count - Click error to select problematic node - Color-coded: red for errors, yellow for warnings - Icon indicators (AlertCircle, AlertTriangle) Task A.3 - TreeEditorPage Integration: - Added ValidationSummary component display - Save button disabled when errors exist - Warnings are informational only (don't block save) - Added manual "Validate" button in toolbar - Imported CheckCircle2 icon for validate button Task A.4 - Visual Node Error Indicators: - Added error/warning badges on problem nodes - Tooltip on hover showing specific error messages - Red ring for errors, yellow ring for warnings - Shows count of errors/warnings per node All tasks from implementation plan completed. Build tested successfully. Related: Issue #1 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -10,7 +10,9 @@ import {
|
|||||||
CheckCircle,
|
CheckCircle,
|
||||||
ChevronDown,
|
ChevronDown,
|
||||||
ChevronRight,
|
ChevronRight,
|
||||||
Play
|
Play,
|
||||||
|
AlertCircle,
|
||||||
|
AlertTriangle
|
||||||
} from 'lucide-react'
|
} from 'lucide-react'
|
||||||
import { useTreeEditorStore } from '@/store/treeEditorStore'
|
import { useTreeEditorStore } from '@/store/treeEditorStore'
|
||||||
import { NodeEditorModal } from './NodeEditorModal'
|
import { NodeEditorModal } from './NodeEditorModal'
|
||||||
@@ -58,10 +60,24 @@ function NodeListItem({
|
|||||||
const [isCollapsed, setIsCollapsed] = useState(false)
|
const [isCollapsed, setIsCollapsed] = useState(false)
|
||||||
const isSelected = selectedNodeId === node.id
|
const isSelected = selectedNodeId === node.id
|
||||||
const isRootNode = node.id === 'root'
|
const isRootNode = node.id === 'root'
|
||||||
const hasError = validationErrors.some(e => e.nodeId === node.id && e.severity === 'error')
|
const nodeErrors = validationErrors.filter(e => e.nodeId === node.id && e.severity === 'error')
|
||||||
const hasWarning = validationErrors.some(e => e.nodeId === node.id && e.severity === 'warning')
|
const nodeWarnings = validationErrors.filter(e => e.nodeId === node.id && e.severity === 'warning')
|
||||||
|
const hasError = nodeErrors.length > 0
|
||||||
|
const hasWarning = nodeWarnings.length > 0
|
||||||
const hasChildren = node.children && node.children.length > 0
|
const hasChildren = node.children && node.children.length > 0
|
||||||
|
|
||||||
|
// Get error/warning messages for tooltip
|
||||||
|
const getValidationTooltip = () => {
|
||||||
|
const messages: string[] = []
|
||||||
|
if (hasError) {
|
||||||
|
messages.push(...nodeErrors.map(e => `❌ ${e.message}`))
|
||||||
|
}
|
||||||
|
if (hasWarning) {
|
||||||
|
messages.push(...nodeWarnings.map(e => `⚠️ ${e.message}`))
|
||||||
|
}
|
||||||
|
return messages.join('\n')
|
||||||
|
}
|
||||||
|
|
||||||
const isDragTarget =
|
const isDragTarget =
|
||||||
dragOverTarget?.parentId === parentId && dragOverTarget?.index === index
|
dragOverTarget?.parentId === parentId && dragOverTarget?.index === index
|
||||||
|
|
||||||
@@ -200,6 +216,28 @@ function NodeListItem({
|
|||||||
{getNodeLabel()}
|
{getNodeLabel()}
|
||||||
</span>
|
</span>
|
||||||
|
|
||||||
|
{/* Error/Warning badge */}
|
||||||
|
{(hasError || hasWarning) && (
|
||||||
|
<span
|
||||||
|
title={getValidationTooltip()}
|
||||||
|
className={cn(
|
||||||
|
'flex items-center gap-1 rounded px-1.5 py-0.5 text-xs',
|
||||||
|
hasError
|
||||||
|
? 'bg-destructive/20 text-destructive'
|
||||||
|
: 'bg-yellow-500/20 text-yellow-600 dark:text-yellow-500'
|
||||||
|
)}
|
||||||
|
>
|
||||||
|
{hasError ? (
|
||||||
|
<AlertCircle className="h-3 w-3" />
|
||||||
|
) : (
|
||||||
|
<AlertTriangle className="h-3 w-3" />
|
||||||
|
)}
|
||||||
|
<span className="hidden sm:inline">
|
||||||
|
{hasError ? `${nodeErrors.length} error${nodeErrors.length !== 1 ? 's' : ''}` : `${nodeWarnings.length} warning${nodeWarnings.length !== 1 ? 's' : ''}`}
|
||||||
|
</span>
|
||||||
|
</span>
|
||||||
|
)}
|
||||||
|
|
||||||
{/* Node ID */}
|
{/* Node ID */}
|
||||||
<span
|
<span
|
||||||
className="hidden text-xs text-muted-foreground sm:inline cursor-help"
|
className="hidden text-xs text-muted-foreground sm:inline cursor-help"
|
||||||
|
|||||||
119
frontend/src/components/tree-editor/ValidationSummary.tsx
Normal file
119
frontend/src/components/tree-editor/ValidationSummary.tsx
Normal file
@@ -0,0 +1,119 @@
|
|||||||
|
import { useState } from 'react'
|
||||||
|
import { AlertCircle, AlertTriangle, ChevronDown, ChevronUp } from 'lucide-react'
|
||||||
|
import { cn } from '@/lib/utils'
|
||||||
|
import type { ValidationError } from '@/store/treeEditorStore'
|
||||||
|
|
||||||
|
interface ValidationSummaryProps {
|
||||||
|
errors: ValidationError[]
|
||||||
|
onSelectNode: (nodeId: string) => void
|
||||||
|
}
|
||||||
|
|
||||||
|
export function ValidationSummary({ errors, onSelectNode }: ValidationSummaryProps) {
|
||||||
|
const [isExpanded, setIsExpanded] = useState(true)
|
||||||
|
|
||||||
|
const errorItems = errors.filter(e => e.severity === 'error')
|
||||||
|
const warningItems = errors.filter(e => e.severity === 'warning')
|
||||||
|
|
||||||
|
if (errors.length === 0) return null
|
||||||
|
|
||||||
|
const handleErrorClick = (error: ValidationError) => {
|
||||||
|
if (error.nodeId) {
|
||||||
|
onSelectNode(error.nodeId)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return (
|
||||||
|
<div
|
||||||
|
className={cn(
|
||||||
|
'rounded-lg border',
|
||||||
|
errorItems.length > 0
|
||||||
|
? 'border-destructive/50 bg-destructive/5'
|
||||||
|
: 'border-yellow-500/50 bg-yellow-50 dark:bg-yellow-900/10'
|
||||||
|
)}
|
||||||
|
>
|
||||||
|
{/* Header */}
|
||||||
|
<button
|
||||||
|
onClick={() => setIsExpanded(!isExpanded)}
|
||||||
|
className={cn(
|
||||||
|
'flex w-full items-center justify-between p-3 text-left transition-colors hover:bg-black/5 dark:hover:bg-white/5',
|
||||||
|
errorItems.length > 0 ? 'text-destructive' : 'text-yellow-700 dark:text-yellow-500'
|
||||||
|
)}
|
||||||
|
>
|
||||||
|
<div className="flex items-center gap-2">
|
||||||
|
{errorItems.length > 0 ? (
|
||||||
|
<AlertCircle className="h-5 w-5" />
|
||||||
|
) : (
|
||||||
|
<AlertTriangle className="h-5 w-5" />
|
||||||
|
)}
|
||||||
|
<span className="font-medium">
|
||||||
|
{errorItems.length > 0 && (
|
||||||
|
<>
|
||||||
|
{errorItems.length} {errorItems.length === 1 ? 'Error' : 'Errors'}
|
||||||
|
</>
|
||||||
|
)}
|
||||||
|
{errorItems.length > 0 && warningItems.length > 0 && ', '}
|
||||||
|
{warningItems.length > 0 && (
|
||||||
|
<>
|
||||||
|
{warningItems.length} {warningItems.length === 1 ? 'Warning' : 'Warnings'}
|
||||||
|
</>
|
||||||
|
)}
|
||||||
|
</span>
|
||||||
|
</div>
|
||||||
|
{isExpanded ? <ChevronUp className="h-4 w-4" /> : <ChevronDown className="h-4 w-4" />}
|
||||||
|
</button>
|
||||||
|
|
||||||
|
{/* Error/Warning List */}
|
||||||
|
{isExpanded && (
|
||||||
|
<div className="space-y-1 border-t border-current/10 p-3">
|
||||||
|
{/* Errors */}
|
||||||
|
{errorItems.map((error, index) => (
|
||||||
|
<button
|
||||||
|
key={`error-${index}`}
|
||||||
|
onClick={() => handleErrorClick(error)}
|
||||||
|
className={cn(
|
||||||
|
'flex w-full items-start gap-2 rounded p-2 text-left text-sm transition-colors',
|
||||||
|
error.nodeId
|
||||||
|
? 'cursor-pointer hover:bg-destructive/10'
|
||||||
|
: 'cursor-default'
|
||||||
|
)}
|
||||||
|
>
|
||||||
|
<AlertCircle className="mt-0.5 h-4 w-4 flex-shrink-0 text-destructive" />
|
||||||
|
<div className="flex-1">
|
||||||
|
<p className="text-destructive">{error.message}</p>
|
||||||
|
{error.nodeId && (
|
||||||
|
<p className="mt-0.5 text-xs text-muted-foreground">
|
||||||
|
Click to select node: {error.nodeId}
|
||||||
|
</p>
|
||||||
|
)}
|
||||||
|
</div>
|
||||||
|
</button>
|
||||||
|
))}
|
||||||
|
|
||||||
|
{/* Warnings */}
|
||||||
|
{warningItems.map((warning, index) => (
|
||||||
|
<button
|
||||||
|
key={`warning-${index}`}
|
||||||
|
onClick={() => handleErrorClick(warning)}
|
||||||
|
className={cn(
|
||||||
|
'flex w-full items-start gap-2 rounded p-2 text-left text-sm transition-colors',
|
||||||
|
warning.nodeId
|
||||||
|
? 'cursor-pointer hover:bg-yellow-100 dark:hover:bg-yellow-900/20'
|
||||||
|
: 'cursor-default'
|
||||||
|
)}
|
||||||
|
>
|
||||||
|
<AlertTriangle className="mt-0.5 h-4 w-4 flex-shrink-0 text-yellow-600 dark:text-yellow-500" />
|
||||||
|
<div className="flex-1">
|
||||||
|
<p className="text-yellow-700 dark:text-yellow-500">{warning.message}</p>
|
||||||
|
{warning.nodeId && (
|
||||||
|
<p className="mt-0.5 text-xs text-muted-foreground">
|
||||||
|
Click to select node: {warning.nodeId}
|
||||||
|
</p>
|
||||||
|
)}
|
||||||
|
</div>
|
||||||
|
</button>
|
||||||
|
))}
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
|
</div>
|
||||||
|
)
|
||||||
|
}
|
||||||
@@ -1,11 +1,12 @@
|
|||||||
import { useEffect, useState, useCallback } from 'react'
|
import { useEffect, useState, useCallback } from 'react'
|
||||||
import { useParams, useNavigate, useBlocker } from 'react-router-dom'
|
import { useParams, useNavigate, useBlocker } from 'react-router-dom'
|
||||||
import { useStore } from 'zustand'
|
import { useStore } from 'zustand'
|
||||||
import { Undo2, Redo2, Save } from 'lucide-react'
|
import { Undo2, Redo2, Save, CheckCircle2 } from 'lucide-react'
|
||||||
import { treesApi } from '@/api'
|
import { treesApi } from '@/api'
|
||||||
import type { TreeCreate, TreeUpdate } from '@/types'
|
import type { TreeCreate, TreeUpdate } from '@/types'
|
||||||
import { useTreeEditorStore, useTreeEditorTemporal } from '@/store/treeEditorStore'
|
import { useTreeEditorStore, useTreeEditorTemporal } from '@/store/treeEditorStore'
|
||||||
import { TreeEditorLayout } from '@/components/tree-editor/TreeEditorLayout'
|
import { TreeEditorLayout } from '@/components/tree-editor/TreeEditorLayout'
|
||||||
|
import { ValidationSummary } from '@/components/tree-editor/ValidationSummary'
|
||||||
import { useKeyboardShortcuts } from '@/hooks/useKeyboardShortcuts'
|
import { useKeyboardShortcuts } from '@/hooks/useKeyboardShortcuts'
|
||||||
import { cn } from '@/lib/utils'
|
import { cn } from '@/lib/utils'
|
||||||
|
|
||||||
@@ -29,7 +30,8 @@ export function TreeEditorPage() {
|
|||||||
getTreeForSave,
|
getTreeForSave,
|
||||||
markSaved,
|
markSaved,
|
||||||
setLoading,
|
setLoading,
|
||||||
setSaving
|
setSaving,
|
||||||
|
selectNode
|
||||||
} = useTreeEditorStore()
|
} = useTreeEditorStore()
|
||||||
|
|
||||||
// Access undo/redo from temporal store
|
// Access undo/redo from temporal store
|
||||||
@@ -38,6 +40,9 @@ export function TreeEditorPage() {
|
|||||||
const [showDraftPrompt, setShowDraftPrompt] = useState(false)
|
const [showDraftPrompt, setShowDraftPrompt] = useState(false)
|
||||||
const [saveError, setSaveError] = useState<string | null>(null)
|
const [saveError, setSaveError] = useState<string | null>(null)
|
||||||
|
|
||||||
|
// Calculate if there are blocking errors
|
||||||
|
const hasBlockingErrors = validationErrors.some(e => e.severity === 'error')
|
||||||
|
|
||||||
// Block navigation if there are unsaved changes
|
// Block navigation if there are unsaved changes
|
||||||
const blocker = useBlocker(
|
const blocker = useBlocker(
|
||||||
({ currentLocation, nextLocation }) =>
|
({ currentLocation, nextLocation }) =>
|
||||||
@@ -122,6 +127,14 @@ export function TreeEditorPage() {
|
|||||||
setShowDraftPrompt(false)
|
setShowDraftPrompt(false)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const handleManualValidate = () => {
|
||||||
|
validate()
|
||||||
|
}
|
||||||
|
|
||||||
|
const handleSelectNode = (nodeId: string) => {
|
||||||
|
selectNode(nodeId)
|
||||||
|
}
|
||||||
|
|
||||||
const handleSave = useCallback(async () => {
|
const handleSave = useCallback(async () => {
|
||||||
setSaveError(null)
|
setSaveError(null)
|
||||||
|
|
||||||
@@ -307,13 +320,28 @@ export function TreeEditorPage() {
|
|||||||
|
|
||||||
<div className="mx-2 h-6 w-px bg-border" />
|
<div className="mx-2 h-6 w-px bg-border" />
|
||||||
|
|
||||||
|
{/* Validate */}
|
||||||
|
<button
|
||||||
|
onClick={handleManualValidate}
|
||||||
|
disabled={isSaving}
|
||||||
|
title="Validate tree structure (checks for errors and warnings)"
|
||||||
|
className={cn(
|
||||||
|
'flex items-center gap-2 rounded-md border border-border bg-background px-3 py-2 text-sm font-medium',
|
||||||
|
'hover:bg-accent disabled:opacity-50'
|
||||||
|
)}
|
||||||
|
>
|
||||||
|
<CheckCircle2 className="h-4 w-4" />
|
||||||
|
Validate
|
||||||
|
</button>
|
||||||
|
|
||||||
{/* Save */}
|
{/* Save */}
|
||||||
<button
|
<button
|
||||||
onClick={handleSave}
|
onClick={handleSave}
|
||||||
disabled={isSaving || !isDirty}
|
disabled={isSaving || !isDirty || hasBlockingErrors}
|
||||||
|
title={hasBlockingErrors ? 'Fix validation errors before saving' : undefined}
|
||||||
className={cn(
|
className={cn(
|
||||||
'flex items-center gap-2 rounded-md bg-primary px-4 py-2 text-sm font-medium text-primary-foreground',
|
'flex items-center gap-2 rounded-md bg-primary px-4 py-2 text-sm font-medium text-primary-foreground',
|
||||||
'hover:bg-primary/90 disabled:opacity-50'
|
'hover:bg-primary/90 disabled:opacity-50 disabled:cursor-not-allowed'
|
||||||
)}
|
)}
|
||||||
>
|
>
|
||||||
<Save className="h-4 w-4" />
|
<Save className="h-4 w-4" />
|
||||||
@@ -329,11 +357,13 @@ export function TreeEditorPage() {
|
|||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
{/* Validation Errors Summary */}
|
{/* Validation Summary */}
|
||||||
{validationErrors.filter(e => e.severity === 'error').length > 0 && (
|
{validationErrors.length > 0 && (
|
||||||
<div className="bg-destructive/10 px-4 py-2 text-sm text-destructive">
|
<div className="px-4 py-3">
|
||||||
{validationErrors.filter(e => e.severity === 'error').length} validation error(s) found.
|
<ValidationSummary
|
||||||
Please fix them before saving.
|
errors={validationErrors}
|
||||||
|
onSelectNode={handleSelectNode}
|
||||||
|
/>
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
|
|||||||
@@ -623,6 +623,44 @@ export const useTreeEditorStore = create<TreeEditorState>()(
|
|||||||
|
|
||||||
validateNode(state.treeStructure)
|
validateNode(state.treeStructure)
|
||||||
|
|
||||||
|
// Check for circular references in next_node_id chains
|
||||||
|
const detectCircularRefs = (startId: string, visited: Set<string> = new Set()): boolean => {
|
||||||
|
if (visited.has(startId)) return true
|
||||||
|
visited.add(startId)
|
||||||
|
|
||||||
|
const node = findNodeInTree(startId, state.treeStructure)
|
||||||
|
if (!node) return false
|
||||||
|
|
||||||
|
// Check options
|
||||||
|
if (node.options) {
|
||||||
|
for (const opt of node.options) {
|
||||||
|
if (opt.next_node_id && detectCircularRefs(opt.next_node_id, new Set(visited))) {
|
||||||
|
errors.push({
|
||||||
|
nodeId: node.id,
|
||||||
|
message: `Circular reference detected: "${opt.label}" creates a loop`,
|
||||||
|
severity: 'error'
|
||||||
|
})
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check next_node_id
|
||||||
|
if (node.next_node_id && detectCircularRefs(node.next_node_id, new Set(visited))) {
|
||||||
|
errors.push({
|
||||||
|
nodeId: node.id,
|
||||||
|
message: `Circular reference detected in node "${node.title || node.id}"`,
|
||||||
|
severity: 'error'
|
||||||
|
})
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
// Run from root
|
||||||
|
detectCircularRefs('root')
|
||||||
|
|
||||||
// Check for at least one solution
|
// Check for at least one solution
|
||||||
if (!hasSolution) {
|
if (!hasSolution) {
|
||||||
errors.push({
|
errors.push({
|
||||||
|
|||||||
Reference in New Issue
Block a user