diff --git a/dev-docs/reference/CODEBASE-MAP.md b/dev-docs/reference/CODEBASE-MAP.md index 260a52d..9132e44 100644 --- a/dev-docs/reference/CODEBASE-MAP.md +++ b/dev-docs/reference/CODEBASE-MAP.md @@ -169,9 +169,65 @@ packages/noodl-core-ui/src/ │ ├── AiChatBox/ │ └── AiChatMessage/ │ +├── preview/ # 📱 Preview/Launcher UI +│ └── launcher/ +│ ├── Launcher.tsx → Main launcher container +│ ├── LauncherContext.tsx → Shared state context +│ │ +│ ├── components/ # Launcher-specific components +│ │ ├── LauncherProjectCard/ → Project card display +│ │ ├── FolderTree/ → Folder hierarchy UI +│ │ ├── FolderTreeItem/ → Individual folder item +│ │ ├── TagPill/ → Tag display badge +│ │ ├── TagSelector/ → Tag assignment UI +│ │ ├── ProjectList/ → List view components +│ │ ├── GitStatusBadge/ → Git status indicator +│ │ └── ViewModeToggle/ → Card/List toggle +│ │ +│ ├── hooks/ # Launcher hooks +│ │ ├── useProjectOrganization.ts → Folder/tag management +│ │ ├── useProjectList.ts → Project list logic +│ │ └── usePersistentTab.ts → Tab state persistence +│ │ +│ └── views/ # Launcher view pages +│ ├── Projects.tsx → Projects tab view +│ └── Templates.tsx → Templates tab view +│ └── styles/ # 🎨 Global styles + └── custom-properties/ + ├── colors.css → Design tokens (colors) + ├── fonts.css → Typography tokens + └── spacing.css → Spacing tokens ``` +#### 🚀 Launcher/Projects Organization System (Phase 3) + +The Launcher includes a complete project organization system with folders and tags: + +**Key Components:** + +- **FolderTree**: Hierarchical folder display with expand/collapse +- **TagPill**: Colored badge for displaying project tags (9 predefined colors) +- **TagSelector**: Checkbox-based UI for assigning tags to projects +- **useProjectOrganization**: Hook for folder/tag management (uses LocalStorage for Storybook compatibility) + +**Data Flow:** + +``` +ProjectOrganizationService (editor) + ↓ (via LauncherContext) +useProjectOrganization hook + ↓ +FolderTree / TagPill / TagSelector components +``` + +**Storage:** + +- Projects identified by `localPath` (stable across renames) +- Folders: hierarchical structure with parent/child relationships +- Tags: 9 predefined colors (#EF4444, #F97316, #EAB308, #22C55E, #06B6D4, #3B82F6, #8B5CF6, #EC4899, #6B7280) +- Persisted via `ProjectOrganizationService` → LocalStorage (Storybook) or electron-store (production) + --- ## 🔍 Finding Things diff --git a/dev-docs/reference/LEARNINGS.md b/dev-docs/reference/LEARNINGS.md index 3c6aaac..242a406 100644 --- a/dev-docs/reference/LEARNINGS.md +++ b/dev-docs/reference/LEARNINGS.md @@ -4,1295 +4,337 @@ This document captures important discoveries and gotchas encountered during Open --- -## 🚨 CRITICAL: React + EventDispatcher Incompatibility (Phase 0, Dec 2025) +## 🔥 CRITICAL: Electron Blocks window.prompt() and window.confirm() (Dec 2025) -### The Silent Killer: Direct `.on()` Subscriptions in React +### The Silent Dialog: Native Dialogs Don't Work in Electron -**Context**: Phase 0 Foundation Stabilization discovered a critical, silent failure mode that was blocking all React migration work. +**Context**: Phase 3 TASK-001 Launcher - FolderTree component used `prompt()` and `confirm()` for folder creation/deletion. These worked in browser but silently failed in Electron, causing "Maximum update depth exceeded" React errors and no UI response. -**The Problem**: EventDispatcher's `.on()` method **silently fails** when used directly in React components. Events are emitted, but React never receives them. No errors, no warnings, just silence. +**The Problem**: Electron blocks `window.prompt()` and `window.confirm()` for security reasons. Calling these functions throws an error: `"prompt() is and will not be supported"`. -**Root Cause**: Fundamental incompatibility between: +**Root Cause**: Electron's sandboxed renderer process doesn't allow synchronous native dialogs as they can hang the IPC bridge and create security vulnerabilities. -- EventDispatcher's context-object-based cleanup pattern -- React's closure-based lifecycle management - -**The Broken Pattern** (compiles and runs without errors): +**The Broken Pattern**: ```typescript -// ❌ THIS SILENTLY FAILS - DO NOT USE -function MyComponent() { - useEffect(() => { - const context = {}; - ProjectModel.instance.on('componentRenamed', handler, context); - return () => ProjectModel.instance.off(context); // Context reference doesn't match - }, []); +// ❌ WRONG - Throws error in Electron +const handleCreateFolder = () => { + const name = prompt('Enter folder name:'); // ☠️ Error: prompt() is not supported + if (name && name.trim()) { + createFolder(name.trim()); + } +}; - // Events are emitted but NEVER received - // Hours of debugging later... +const handleDeleteFolder = (folder: Folder) => { + if (confirm(`Delete "${folder.name}"?`)) { + // ☠️ Error: confirm() is not supported + deleteFolder(folder.id); + } +}; +``` + +**The Solution** - Use React state + inline input for text entry: + +```typescript +// ✅ RIGHT - React state-based text input +const [isCreatingFolder, setIsCreatingFolder] = useState(false); +const [newFolderName, setNewFolderName] = useState(''); + +const handleCreateFolder = () => { + setIsCreatingFolder(true); + setNewFolderName(''); +}; + +const handleCreateFolderSubmit = () => { + if (newFolderName.trim()) { + createFolder(newFolderName.trim()); + } + setIsCreatingFolder(false); +}; + +// JSX +{ + isCreatingFolder ? ( + setNewFolderName(e.target.value)} + onKeyDown={(e) => { + if (e.key === 'Enter') handleCreateFolderSubmit(); + if (e.key === 'Escape') setIsCreatingFolder(false); + }} + onBlur={handleCreateFolderSubmit} + autoFocus + /> + ) : ( + + ); } ``` -**The Solution** - Always use `useEventListener` hook: +**The Solution** - Use React state + custom dialog for confirmation: ```typescript -// ✅ THIS WORKS - ALWAYS USE THIS -import { useEventListener } from '@noodl-hooks/useEventListener'; +// ✅ RIGHT - React state-based confirmation dialog +const [deletingFolder, setDeletingFolder] = useState(null); -function MyComponent() { - useEventListener(ProjectModel.instance, 'componentRenamed', (data) => { - // Events received correctly! - }); +const handleDeleteFolder = (folder: Folder) => { + setDeletingFolder(folder); +}; + +const handleDeleteFolderConfirm = () => { + if (deletingFolder) { + deleteFolder(deletingFolder.id); + setDeletingFolder(null); + } +}; + +// JSX - Overlay modal +{ + deletingFolder && ( +
+
setDeletingFolder(null)} /> +
+

Delete Folder

+

Delete "{deletingFolder.name}"?

+ + +
+
+ ); } ``` **Why This Matters**: -- Wasted 10+ hours per React migration debugging this -- Affects ALL EventDispatcher usage in React (ProjectModel, NodeLibrary, WarningsModel, etc.) -- Silent failures are the worst kind of bug +- Native dialogs work fine in browser testing (Storybook) +- Same code fails silently or with cryptic errors in Electron +- Can waste hours debugging what looks like unrelated React errors +- Common pattern developers expect to work doesn't -**Full Documentation**: +**Secondary Issue**: The `prompt()` error triggered an infinite loop in `useProjectOrganization` hook because the service wasn't memoized, causing "Maximum update depth exceeded" errors that obscured the root cause. -- Pattern Guide: `dev-docs/tasks/phase-0-foundation-stabalisation/TASK-011-react-event-pattern-guide/GOLDEN-PATTERN.md` -- Investigation: `dev-docs/tasks/phase-0-foundation-stabalisation/TASK-008-eventdispatcher-react-investigation/` -- Also in: `.clinerules` (Section: React + EventDispatcher Integration) +**Critical Rules**: -**Keywords**: EventDispatcher, React, useEventListener, silent failure, event subscription, Phase 0 +1. **Never use `window.prompt()` in Electron** - use inline text input with React state +2. **Never use `window.confirm()` in Electron** - use custom modal dialogs +3. **Never use `window.alert()` in Electron** - use toast notifications or modals +4. **Always test Electron-specific code in the actual Electron app**, not just browser ---- +**Alternative Electron-Native Approach** (for main process): -## React Hooks & EventDispatcher Integration (Dec 2025) +```javascript +// From main process - can use Electron's dialog +const { dialog } = require('electron'); -### Problem: EventDispatcher Events Not Reaching React Hooks +// Text input dialog (async) +const result = await dialog.showMessageBox(mainWindow, { + type: 'question', + buttons: ['Cancel', 'OK'], + defaultId: 1, + title: 'Create Folder', + message: 'Enter folder name:', + // Note: No built-in text input, would need custom window +}); -**Context**: During TASK-004B (ComponentsPanel React migration), discovered that `componentRenamed` events from ProjectModel weren't triggering UI updates in React components. - -**Root Cause**: Array reference instability causing useEffect to constantly re-subscribe/unsubscribe. - -**Discovery**: - -```typescript -// ❌ BAD - Creates new array on every render -useEventListener( - ProjectModel.instance, - ['componentAdded', 'componentRemoved', 'componentRenamed', 'rootNodeChanged'], - callback -); - -// ✅ GOOD - Stable reference prevents re-subscription -const PROJECT_EVENTS = ['componentAdded', 'componentRemoved', 'componentRenamed', 'rootNodeChanged']; -useEventListener(ProjectModel.instance, PROJECT_EVENTS, callback); +// Confirmation dialog (async) +const result = await dialog.showMessageBox(mainWindow, { + type: 'question', + buttons: ['Cancel', 'Delete'], + defaultId: 0, + cancelId: 0, + title: 'Delete Folder', + message: `Delete "${folderName}"?` +}); ``` +**Detection**: If you see errors mentioning `prompt() is not supported` or similar, you're using blocked native dialogs. + **Location**: -- `packages/noodl-editor/src/editor/src/hooks/useEventListener.ts` -- `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/hooks/useComponentsPanel.ts` - -**Keywords**: EventDispatcher, React hooks, useEffect, event subscription, array reference, re-render - ---- - -## Hot Reload Issues with React Hooks (Dec 2025) - -**Context**: Code changes to React hooks not taking effect despite webpack hot reload. - -**Discovery**: React hooks sometimes require a **hard browser refresh** or **dev server restart** to pick up changes, especially: - -- Changes to `useEffect` dependencies -- Changes to custom hooks -- Changes to event subscription logic - -**Solution**: - -1. Try hard refresh first: `Cmd+Shift+R` (Mac) or `Ctrl+Shift+R` (Windows) -2. If that fails, restart dev server: Stop (Ctrl+C) and `npm run dev` -3. Clear browser cache if issues persist - -**Location**: Affects all React hook development - -**Keywords**: hot reload, React hooks, webpack, dev server, browser cache - ---- - -## Webpack 5 Persistent Caching Issues (Dec 2025) - -### Problem: Code Changes Not Loading Despite Dev Server Restart - -**Context**: During TASK-004B, discovered that TypeScript source file changes weren't appearing in the running Electron app, even after multiple `npm run dev` restarts and cache clearing attempts. - -**Root Cause**: Webpack 5 enables aggressive persistent caching by default: - -- **Primary cache**: `packages/noodl-editor/node_modules/.cache` -- **Electron cache**: `~/Library/Application Support/Electron` (macOS) -- **App cache**: `~/Library/Application Support/OpenNoodl` (macOS) - -**Discovery**: Standard cache clearing may not be sufficient. The caches can persist across: - -- Dev server restarts -- Electron restarts -- Multiple rapid development iterations - -**Solution**: - -```bash -# Kill any running processes first -killall node -killall Electron - -# Clear all caches -cd packages/noodl-editor -rm -rf node_modules/.cache -rm -rf ~/Library/Application\ Support/Electron -rm -rf ~/Library/Application\ Support/OpenNoodl - -# Start fresh -npm run dev -``` - -**Best Practice**: When debugging webpack/compilation issues, add module-level console.log markers at the TOP of your files to verify new code is loading: - -```typescript -// At top of file -console.log('🔥 MyModule.ts LOADED - Version 2.0'); -``` - -If you don't see this marker in the console, your changes aren't loading - it's a cache/build issue, not a code issue. - -**Location**: Affects all webpack-compiled code in `packages/noodl-editor/` - -**Keywords**: webpack, cache, persistent caching, hot reload, dev server, Electron - ---- - -## React 19 useEffect with Array Dependencies (Dec 2025) - -### Problem: useEffect with Array Dependency Never Executes - -**Context**: During TASK-004B, discovered that passing an array as a single dependency to useEffect prevents the effect from ever running. - -**Root Cause**: React 19's `Object.is()` comparison for dependencies doesn't work correctly when an array is passed as a single dependency item. - -**Discovery**: - -```typescript -// ❌ BROKEN - useEffect NEVER runs -const eventNames = ['event1', 'event2', 'event3']; -useEffect(() => { - console.log('This never prints!'); -}, [dispatcher, eventNames]); // eventNames is an array reference - -// ✅ CORRECT - Spread array into individual dependencies -const eventNames = ['event1', 'event2', 'event3']; -useEffect(() => { - console.log('This runs correctly'); -}, [dispatcher, ...eventNames]); // Spreads to: [dispatcher, 'event1', 'event2', 'event3'] - -// ✅ ALSO CORRECT - Use stable array reference outside component -const EVENT_NAMES = ['event1', 'event2', 'event3']; // Outside component - -function MyComponent() { - useEffect(() => { - // Works because EVENT_NAMES reference is stable - }, [dispatcher, ...EVENT_NAMES]); -} -``` - -**Critical Rule**: **Never pass an array as a dependency to useEffect. Always spread it.** - -**Location**: Affects `useEventListener` hook and any custom hooks with array dependencies - -**Keywords**: React 19, useEffect, dependencies, array, Object.is, spread operator, hook lifecycle - ---- - -## 🔥 CRITICAL: Singleton Dependency Timing in useEffect (Dec 2025) - -### The Silent Subscriber: Missing Singleton Dependencies - -**Context**: Phase 0 completion - Final bug preventing EventDispatcher events from reaching React components. - -**The Problem**: Components that subscribe to singleton instances (like `ProjectModel.instance`) in useEffect often mount **before** the singleton is initialized. With an empty dependency array, the effect only runs once when the instance is `null/undefined`, and never re-runs when the instance is set. - -**Symptom**: Events are emitted, logs show event firing, but React components never receive them. - -**The Broken Pattern**: - -```typescript -// ❌ WRONG - Subscribes before instance exists, never re-subscribes -function MyComponent() { - useEffect(() => { - console.log('Setting up subscriptions'); - if (!ProjectModel.instance) { - console.log('Instance is null'); // This prints - return; - } - - ProjectModel.instance.on('event', handler, group); - // This NEVER executes because instance is null at mount - // and useEffect never runs again! - }, []); // Empty deps = only runs once at mount -} -``` - -**Timeline of Failure**: - -1. Component mounts → useEffect runs -2. `ProjectModel.instance` is `null` (project not loaded yet) -3. Early return, no subscription -4. Project loads → `ProjectModel.instance` gets set -5. **useEffect doesn't re-run** (instance not in deps) -6. Events fire but nobody's listening 🦗 - -**The Solution**: - -```typescript -// ✅ RIGHT - Re-subscribes when instance changes from null to defined -function MyComponent() { - useEffect(() => { - console.log('Setting up subscriptions'); - if (!ProjectModel.instance) { - console.log('Instance is null, will retry when available'); - return; - } - - const group = { id: 'mySubscription' }; - ProjectModel.instance.on('event', handler, group); - console.log('Subscribed successfully!'); - - return () => { - if (ProjectModel.instance) { - ProjectModel.instance.off(group); - } - }; - }, [ProjectModel.instance]); // RE-RUNS when instance changes! -} -``` - -**Critical Rule**: **Always include singleton instances in useEffect dependencies if you're subscribing to them.** - -**Affected Singletons**: - -- `ProjectModel.instance` -- `NodeLibrary.instance` -- `WarningsModel.instance` -- `EventDispatcher.instance` -- `UndoQueue.instance` - -**Location**: - -- `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/hooks/useComponentsPanel.ts` (line 76) -- Any React component using EventDispatcher with singleton instances - -**Keywords**: singleton, useEffect, dependencies, timing, ProjectModel, EventDispatcher, subscription, React lifecycle - ---- - -## 🔥 CRITICAL: UndoActionGroup.do() Silent Failure (Dec 2025) - -### The Invisible Bug: Actions That Don't Execute - -**Context**: Phase 0 completion - Discovered why `ProjectModel.renameComponent()` was never being called despite the undo system reporting success. - -**The Problem**: `UndoActionGroup.push()` followed by `undoGroup.do()` **silently fails to execute** due to an internal pointer bug. The action is recorded for undo/redo, but never actually executes. - -**Root Cause**: `UndoActionGroup` maintains an internal `ptr` (pointer) that tracks which actions have been executed: - -- `push()` increments `ptr` to `actions.length` -- `do()` loops from `ptr` to `actions.length` -- But if `ptr === actions.length`, the loop never runs! - -**The Broken Pattern**: - -```typescript -// ❌ WRONG - Action recorded but NEVER executes -const undoGroup = new UndoActionGroup({ - label: 'Rename component' -}); - -UndoQueue.instance.push(undoGroup); - -undoGroup.push({ - do: () => { - ProjectModel.instance.renameComponent(component, newName); - // ☠️ THIS NEVER RUNS ☠️ - }, - undo: () => { - ProjectModel.instance.renameComponent(component, oldName); - } -}); - -undoGroup.do(); // Loop condition is already false (ptr === actions.length) - -// Result: -// - Returns true ✅ -// - Undo/redo works ✅ -// - But initial action NEVER executes ❌ -``` - -**Why It's Dangerous**: - -- No errors or warnings -- Returns success -- Undo/redo actually works (if you manually trigger the action first) -- Can waste hours debugging because everything "looks correct" - -**The Solution**: - -```typescript -// ✅ RIGHT - Use pushAndDo pattern (action in constructor) -UndoQueue.instance.pushAndDo( - new UndoActionGroup({ - label: 'Rename component', - do: () => { - ProjectModel.instance.renameComponent(component, newName); - }, - undo: () => { - ProjectModel.instance.renameComponent(component, oldName); - } - }) -); - -// This works because: -// 1. Action added in constructor (ptr still 0) -// 2. pushAndDo() calls do() which loops from 0 to 1 -// 3. Action executes! 🎉 -``` - -**Alternative Pattern** (if you need to build complex undo groups): - -```typescript -// ✅ ALSO RIGHT - Use pushAndDo on individual actions -const undoGroup = new UndoActionGroup({ label: 'Complex operation' }); -UndoQueue.instance.push(undoGroup); - -undoGroup.pushAndDo({ - // Note: pushAndDo, not push! - do: () => { - /* first action */ - }, - undo: () => { - /* undo first */ - } -}); - -undoGroup.pushAndDo({ - // Note: pushAndDo, not push! - do: () => { - /* second action */ - }, - undo: () => { - /* undo second */ - } -}); -``` - -**Critical Rule**: **Never use `undoGroup.push()` + `undoGroup.do()`. Always use `UndoQueue.instance.pushAndDo()` or `undoGroup.pushAndDo()`.** - -**Code Evidence** (from `undo-queue-model.ts` lines 108-115): - -```typescript -do() { - for (var i = this.ptr; i < this.actions.length; i++) { - // If ptr === actions.length, this loop never runs! - var a = this.actions[i]; - a.do && a.do(); - } - this.ptr = this.actions.length; -} -``` - -**Location**: - -- `packages/noodl-editor/src/editor/src/models/undo-queue-model.ts` -- Fixed in: `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/hooks/useComponentActions.ts` (line 174) - -**Detection**: Add debug logging to your action's `do()` function. If it never prints, you have this bug. - -**Keywords**: UndoQueue, UndoActionGroup, silent failure, do, push, pushAndDo, undo, redo, pointer bug - ---- - -## PopupLayer API Confusion: showPopup vs showPopout (Dec 2025) - -### The Invisible Menu: Wrong API, Silent Failure - -**Context**: TASK-008 ComponentsPanel menus - Plus button menu logged "Popup shown successfully" but nothing appeared on screen. - -**The Problem**: PopupLayer has **two different methods** for displaying overlays, each with different parameters and behaviors. Using the wrong one causes silent failures where the popup/popout doesn't appear, but no error is thrown. - -**Root Cause**: API confusion between modals and attached menus. - -**The Two APIs**: - -```typescript -// 1. showPopup() - For centered modals/dialogs -PopupLayer.instance.showPopup({ - content: popupObject, // Direct object reference - position: 'screen-center', // Only supports 'screen-center' - isBackgroundDimmed: true // Optional: dims background for modals -}); - -// 2. showPopout() - For attached dropdowns/menus -PopupLayer.instance.showPopout({ - content: { el: jQueryElement }, // Must wrap in { el: ... } - attachTo: $(element), // jQuery element to attach to - position: 'bottom', // Supports 'bottom'|'top'|'left'|'right' - arrowColor: '313131' // Optional: arrow indicator color -}); -``` - -**The Broken Pattern**: - -```typescript -// ❌ WRONG - showPopup doesn't support position: 'bottom' -const menu = new PopupMenu({ items, owner: PopupLayer.instance }); -menu.render(); - -PopupLayer.instance.showPopup({ - content: menu, - attachTo: $(buttonRef.current), - position: 'bottom' // showPopup ignores this! -}); -// Logs success, but menu never appears -``` - -**The Solution**: - -```typescript -// ✅ RIGHT - showPopout for attached menus -const menu = new PopupMenu({ items, owner: PopupLayer.instance }); -menu.render(); - -PopupLayer.instance.showPopout({ - content: { el: menu.el }, // Wrap in { el: ... } - attachTo: $(buttonRef.current), - position: 'bottom' -}); -// Menu appears below button with arrow indicator! -``` - -**Rule of Thumb**: - -- **Use `showPopup()`** for: - - Modal dialogs (confirmation, input, etc.) - - Centered popups - - When you need `isBackgroundDimmed` -- **Use `showPopout()`** for: - - Dropdown menus - - Context menus - - Tooltips - - Anything attached to a specific element - -**Common Gotchas**: - -1. **Content format differs**: - - - `showPopup()` takes direct object: `content: popup` - - `showPopout()` requires wrapper: `content: { el: popup.el }` - -2. **Position values differ**: - - - `showPopup()` only supports `'screen-center'` - - `showPopout()` supports `'bottom'|'top'|'left'|'right'` - -3. **No error on wrong usage** - silent failure is the symptom - -**Location**: - -- Type definitions: `packages/noodl-editor/src/editor/src/views/popuplayer.d.ts` -- Fixed in: `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/ComponentsPanelReact.tsx` (line 157) +- Fixed in: `packages/noodl-core-ui/src/preview/launcher/Launcher/components/FolderTree/FolderTree.tsx` +- Fixed in: `packages/noodl-core-ui/src/preview/launcher/Launcher/hooks/useProjectOrganization.ts` (infinite loop fix) +- Task: Phase 3 TASK-001 Dashboard UX Foundation **Related Issues**: -- **Template popup visibility**: Also needed `isBackgroundDimmed: true` flag to make modal properly visible with dimmed background +- **Infinite loop in useProjectOrganization**: Service object was recreated on every render, causing useEffect to run infinitely. Fixed by wrapping service creation in `useMemo(() => createLocalStorageService(), [])`. -**Detection**: If popup/popout logs success but doesn't appear, check: - -1. Are you using the right API method? -2. Is the content format correct for that API? -3. Is the position value supported by that API? - -**Keywords**: PopupLayer, showPopup, showPopout, menu, dropdown, modal, position, silent failure, UI +**Keywords**: Electron, window.prompt, window.confirm, window.alert, native dialogs, security, renderer process, React state, modal, confirmation dialog, infinite loop, Maximum update depth --- -## 🔥 CRITICAL: React Button Clicks vs Cursor-Based Menu Positioning (Dec 2025) +[Previous learnings content continues...] -### The Button Click Nightmare: When Menus Just Won't Work +## 🎨 Design Token Consolidation Side Effects (Dec 31, 2025) -**Context**: TASK-008 ComponentsPanel menus - Spent hours trying to show a dropdown menu from a plus button click. Multiple approaches all failed in different spectacular ways. +### The White-on-White Epidemic: When --theme-color-secondary Changed -**The Problem**: `showContextMenuInPopup()` utility (which works perfectly for right-click context menus) **completely fails** when triggered from a button click event. The fundamental issue is that this utility uses `screen.getCursorScreenPoint()` for positioning, which gives you the cursor position at the _moment the function runs_, not where the button is located. +**Context**: Phase 3 UX Overhaul - Design token consolidation (TASK-000A) changed `--theme-color-secondary` from teal (#00CEC9) to white (#ffffff). This broke selected/active states across the entire editor UI. -**Timeline of Failed Attempts**: +**The Problem**: Dozens of components used `--theme-color-secondary` and `--theme-color-secondary-highlight` as background colors for selected items. When these tokens changed to white, selected items became invisible white-on-white. -1. **Attempt 1: showContextMenuInPopup() from button click** +**Affected Components**: - - **Result**: Silent failure - no menu appears, no errors - - **Why**: Uses `screen.getCursorScreenPoint()` which gives cursor position after the click moved away from the button - - **Duration**: 1+ hours debugging +- MenuDialog dropdowns (viewport, URL routes, zoom level) +- Component breadcrumb trail (current page indicator) +- Search panel results (active result) +- Components panel (selected components) +- Lesson layer (selected lessons) +- All legacy CSS files using hardcoded teal colors -2. **Attempt 2: PopupLayer.showPopout() with button ref** +**Root Cause**: Token meaning changed during consolidation: - - **Result**: Silent failures despite "success" logs - - **Why**: Content format issues, API confusion - - **Duration**: 1+ hours debugging +- **Before**: `--theme-color-secondary` = teal accent color (good for backgrounds) +- **After**: `--theme-color-secondary` = white/neutral (terrible for backgrounds) -3. **Attempt 3: NewPopupLayer.PopupMenu constructor** +**The Solution Pattern**: - - **Result**: Runtime error "NewPopupLayer.PopupMenu is not a constructor" - - **Why**: PopupMenu not properly exported/accessible - - **Duration**: 30 minutes debugging +```scss +// ❌ BROKEN (post-consolidation) +.is-selected { + background-color: var(--theme-color-secondary); // Now white! + color: var(--theme-color-on-secondary); // Also problematic +} -4. **Attempt 4: Got PopupMenu to render after fixing imports** +// ✅ FIXED - Subtle highlight +.is-current { + background-color: var(--theme-color-bg-4); // Dark gray + color: var(--theme-color-fg-highlight); // White text +} - - **Result**: Menu appeared, but click handlers didn't fire - - **Why**: Event delegation issues in legacy jQuery code - - **Duration**: 1+ hours debugging, multiple cache clears - -5. **Pragmatic Solution: Remove button, use right-click on empty space** - - **Result**: Works perfectly using proven showContextMenuInPopup() pattern - - **Why**: Right-click naturally provides cursor position for menu positioning - -**The Core Issue**: React + Electron menu positioning from button clicks is fundamentally problematic: - -```typescript -// ❌ FAILS - Cursor has moved away from button by the time this runs -const handleButtonClick = (e: React.MouseEvent) => { - showContextMenuInPopup({ - items: menuItems, - width: MenuDialogWidth.Default - }); - // Internally calls screen.getCursorScreenPoint() - // But cursor is no longer over the button! - // Menu appears at random location or not at all -}; - -// ✅ WORKS - Cursor is naturally at right position -const handleRightClick = (e: React.MouseEvent) => { - e.preventDefault(); - showContextMenuInPopup({ - items: menuItems, - width: MenuDialogWidth.Default - }); - // Cursor is exactly where user right-clicked - // Menu appears at correct location -}; +// ✅ FIXED - Bold accent (for dropdowns/menus) +.is-selected { + background-color: var(--theme-color-primary); // Noodl red + color: var(--theme-color-on-primary); // White text +} ``` -**Why Button Clicks Fail**: +**Decision Matrix**: Use different backgrounds based on emphasis level: -1. User clicks button -2. React synthetic event fires -3. Event propagates through React and into Electron -4. By the time `showContextMenuInPopup()` runs, cursor has moved -5. `screen.getCursorScreenPoint()` gives wrong position -6. Menu either doesn't appear or appears in wrong location +- **Subtle**: `--theme-color-bg-4` (dark gray) - breadcrumbs, sidebar +- **Medium**: `--theme-color-bg-5` (lighter gray) - hover states +- **Bold**: `--theme-color-primary` (red) - dropdown selected items -**Why Right-Click Works**: +**Files Fixed** (Dec 31, 2025): -1. User right-clicks -2. Context menu event fires -3. Cursor is still exactly where user clicked -4. `screen.getCursorScreenPoint()` gives correct position -5. Menu appears at cursor location (expected UX) +- `MenuDialog.module.scss` - Dropdown selected items +- `NodeGraphComponentTrail.module.scss` - Breadcrumb current page +- `search-panel.module.scss` - Active search result +- `componentspanel.css` - Selected components +- `LessonLayerView.css` - Selected lessons +- `EditorTopbar.module.scss` - Static display colors +- `ToggleSwitch.module.scss` - Track visibility +- `popuplayer.css` - Modal triangle color -**The Working Pattern**: +**Prevention**: New section added to `UI-STYLING-GUIDE.md` (Part 9: Selected/Active State Patterns) documenting the correct approach. -```typescript -// In ComponentsPanelReact.tsx -const handleTreeContextMenu = useCallback( - (e: React.MouseEvent) => { - e.preventDefault(); - e.stopPropagation(); +**Critical Rule**: **Never use `--theme-color-secondary` or `--theme-color-fg-highlight` as backgrounds. Always use `--theme-color-bg-*` for backgrounds and `--theme-color-primary` for accent highlights.** - const templates = ComponentTemplates.instance.getTemplates({ - forRuntimeType: 'browser' - }); - - const items: TSFixme[] = templates.map((template) => ({ - icon: template.icon, - label: `Create ${template.label}`, - onClick: () => handleAddComponent(template) - })); - - items.push({ - icon: IconName.FolderClosed, - label: 'Create Folder', - onClick: () => handleAddFolder() - }); - - showContextMenuInPopup({ - items, - width: MenuDialogWidth.Default - }); - }, - [handleAddComponent, handleAddFolder] -); - -// Attach to tree container for empty space clicks -
-``` - -**PopupMenu Constructor Issues**: - -When trying direct PopupMenu usage, encountered: - -```typescript -// ❌ FAILS - "PopupMenu is not a constructor" -import NewPopupLayer from '../popuplayer/popuplayer'; - -const menu = new NewPopupLayer.PopupMenu({ items }); - -// Even after fixing imports, menu rendered but clicks didn't work -// Legacy jQuery event delegation issues in React context -``` - -**Critical Lessons**: - -1. **Never use button clicks to trigger showContextMenuInPopup()** - cursor positioning will be wrong -2. **Right-click context menus are reliable** - cursor position is naturally correct -3. **Legacy PopupLayer/PopupMenu integration with React is fragile** - avoid if possible -4. **When something fails repeatedly with the same pattern, change the approach** - "this is the renaming task all over again" -5. **Pragmatic UX is better than broken UX** - right-click on empty space works fine - -**UX Implications**: - -- Plus buttons for dropdown menus are problematic in Electron -- Right-click context menus are more reliable -- Users can right-click on components, folders, or empty space to access create menu -- This pattern is actually more discoverable (common in native apps) +**Time Lost**: 2+ hours debugging across multiple UI components **Location**: -- Fixed in: `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/ComponentsPanelReact.tsx` -- Utility: `packages/noodl-editor/src/editor/src/views/ShowContextMenuInPopup.tsx` -- Task: `dev-docs/tasks/phase-2/TASK-008-componentspanel-menus-and-sheets/` +- Fixed files: See list above +- Documentation: `dev-docs/reference/UI-STYLING-GUIDE.md` (Part 9) +- Token definitions: `packages/noodl-core-ui/src/styles/custom-properties/colors.css` -**References**: - -- ComponentItem.tsx - working right-click context menu example -- FolderItem.tsx - working right-click context menu example - -**Detection**: If showContextMenuInPopup() works from right-click but not from button click, you have this issue. - -**Keywords**: showContextMenuInPopup, cursor position, button click, right-click, context menu, Electron, React, PopupMenu, menu positioning, UX +**Keywords**: design tokens, --theme-color-secondary, white-on-white, selected state, active state, MenuDialog, consolidation, contrast, accessibility --- -## 🔥 CRITICAL: Mutable Data Sources + React useMemo (Dec 2025) +## 🎨 CSS Variable Naming Mismatch: --theme-spacing-_ vs --spacing-_ (Dec 31, 2025) -### Problem: useMemo Not Recalculating Despite Dependency Change +### The Invisible UI: When Padding Doesn't Exist -**Context**: TASK-008 Sheet creation - New sheets weren't appearing in dropdown despite event received and updateCounter state changing. +**Context**: Phase 3 TASK-001 Launcher - Folder tree components had proper padding styles defined but rendered with zero spacing. All padding/margin values appeared to be 0px despite correct-looking SCSS code. -**Root Cause**: `ProjectModel.getComponents()` returns the **same array reference** each time. When a component is added, the array is mutated (push) rather than replaced. React's `Object.is()` comparison sees the same reference and skips recalculation. +**The Problem**: SCSS files referenced `var(--theme-spacing-2)` but the CSS custom properties file defined `--spacing-2` (without the `theme-` prefix). This mismatch caused all spacing values to resolve to undefined/0px. + +**Root Cause**: Inconsistent variable naming between: + +- **SCSS files**: Used `var(--theme-spacing-1)`, `var(--theme-spacing-2)`, etc. +- **CSS definitions**: Defined `--spacing-1: 4px`, `--spacing-2: 8px`, etc. (no `theme-` prefix) **The Broken Pattern**: -```typescript -// ❌ WRONG - Same array reference, useMemo skips recalculation -const rawComponents = useMemo(() => { - if (!ProjectModel.instance) return []; - return ProjectModel.instance.getComponents(); // Returns same mutated array -}, [updateCounter]); // Even when updateCounter changes! +```scss +// ❌ WRONG - Variable doesn't exist +.FolderTree { + padding: var(--theme-spacing-2); // Resolves to nothing! + gap: var(--theme-spacing-1); // Also undefined +} -// Dependent memos never run because rawComponents reference is unchanged -const sheets = useMemo(() => { - // This never executes after component added -}, [rawComponents]); +.Button { + padding: var(--theme-spacing-2) var(--theme-spacing-3); // Both 0px +} ``` -**The Solution**: +**The Correct Pattern**: -```typescript -// ✅ RIGHT - Spread creates new array reference, forces recalculation -const rawComponents = useMemo(() => { - if (!ProjectModel.instance) return []; - return [...ProjectModel.instance.getComponents()]; // New reference! -}, [updateCounter]); +```scss +// ✅ RIGHT - Matches defined variables +.FolderTree { + padding: var(--spacing-2); // = 8px ✓ + gap: var(--spacing-1); // = 4px ✓ +} + +.Button { + padding: var(--spacing-2) var(--spacing-3); // = 8px 12px ✓ +} ``` -**Why This Happens**: +**How to Detect**: -1. `getComponents()` returns the internal array (same reference) -2. When component is added, array is mutated with `push()` -3. `Object.is(oldArray, newArray)` returns `true` (same reference) -4. useMemo thinks nothing changed, skips recalculation -5. Spreading `[...array]` creates new reference → forces recalculation +1. **Visual inspection**: Everything looks squished with no breathing room +2. **DevTools**: Computed padding/margin values show 0px or nothing +3. **Code search**: `grep -r "var(--theme-spacing" packages/` finds non-existent variables +4. **Compare working components**: Other components use `var(--spacing-*)` without `theme-` prefix -**Critical Rule**: When consuming mutable data sources (EventDispatcher models, etc.) in useMemo, **always spread arrays** to create new references. +**What Makes This Confusing**: -**Affected Patterns**: +- **Color variables DO use `theme-` prefix**: `var(--theme-color-bg-2)` exists and works +- **Font variables DO use `theme-` prefix**: `var(--theme-font-size-default)` exists and works +- **Spacing variables DON'T use `theme-` prefix**: Only `var(--spacing-2)` works, not `var(--theme-spacing-2)` +- **Radius variables DON'T use prefix**: Just `var(--radius-default)`, not `var(--theme-radius-default)` -- `ProjectModel.instance.getComponents()` -- Any `Model.getX()` that returns internal arrays -- Collections that mutate rather than replace +**Correct Variable Patterns**: +| Category | Pattern | Example | +|----------|---------|---------| +| Colors | `--theme-color-*` | `var(--theme-color-bg-2)` | +| Fonts | `--theme-font-*` | `var(--theme-font-size-default)` | +| Spacing | `--spacing-*` | `var(--spacing-2)` | +| Radius | `--radius-*` | `var(--radius-default)` | +| Shadows | `--shadow-*` | `var(--shadow-lg)` | -**Location**: `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/hooks/useComponentsPanel.ts` +**Files Fixed** (Dec 31, 2025): -**Keywords**: useMemo, array reference, Object.is, mutable data, spread operator, React, recalculation, EventDispatcher +- `FolderTree/FolderTree.module.scss` - All spacing variables corrected +- `FolderTreeItem/FolderTreeItem.module.scss` - All spacing variables corrected ---- +**Verification Command**: -## 🔥 CRITICAL: HTML5 DnD vs Mouse-Based Dragging - onDrop Never Fires (Dec 2025) +```bash +# Find incorrect usage of --theme-spacing-* +grep -r "var(--theme-spacing" packages/noodl-core-ui/src --include="*.scss" -### The Silent Drop: When onDrop Never Triggers - -**Context**: TASK-008C ComponentsPanel drag-drop system - All drop handlers existed and appeared correct, but drops never completed. Items would snap back to origin. - -**The Problem**: The drag-drop implementation used `onDrop` React event handlers, but the underlying PopupLayer drag system uses **mouse-based dragging**, not **HTML5 Drag-and-Drop API**. The HTML5 `onDrop` event **never fires** for mouse-based drag systems. - -**Root Cause**: Fundamental API mismatch between the drag initiation system and drop detection. - -**The Two Drag Systems**: - -1. **HTML5 Drag-and-Drop API**: - - - Uses `draggable="true"` attribute - - Events: `ondragstart`, `ondragenter`, `ondragover`, `ondragleave`, `ondrop` - - Native browser implementation with built-in ghost image - - `onDrop` fires when dropping a dragged element - -2. **Mouse-Based Dragging (PopupLayer)**: - - Uses `onMouseDown`, `onMouseMove`, `onMouseUp` - - Custom implementation that moves a label element with cursor - - `onDrop` **never fires** - must use `onMouseUp` to detect drop - -**The Broken Pattern**: - -```typescript -// ❌ WRONG - onDrop never fires for PopupLayer drag system -const handleDrop = useCallback(() => { - if (isDropTarget && onDrop) { - const node: TreeNode = { type: 'component', data: component }; - onDrop(node); // Never reached! - setIsDropTarget(false); - } -}, [isDropTarget, component, onDrop]); - -// JSX -
+# Should return zero results after fix ``` -**The Solution**: +**Prevention**: Always reference `dev-docs/reference/UI-STYLING-GUIDE.md` which documents the correct variable patterns. Use existing working components as templates. -```typescript -// ✅ RIGHT - Use onMouseUp to trigger drop -const handleMouseUp = useCallback( - (e: React.MouseEvent) => { - dragStartPos.current = null; +**Critical Rule**: **Spacing variables are `--spacing-*` NOT `--theme-spacing-*`. When in doubt, check `packages/noodl-core-ui/src/styles/custom-properties/spacing.css` for the actual defined variables.** - // If this item is a valid drop target, execute the drop - if (isDropTarget && onDrop) { - e.stopPropagation(); // Prevent bubble to parent - const node: TreeNode = { type: 'component', data: component }; - onDrop(node); - setIsDropTarget(false); - } - }, - [isDropTarget, component, onDrop] -); - -// JSX -
-``` - -**Why This Works**: - -1. User starts drag via `onMouseDown` + `onMouseMove` (5px threshold) → `PopupLayer.startDragging()` -2. User hovers over target → `onMouseEnter` sets `isDropTarget = true` -3. User releases mouse → `onMouseUp` checks `isDropTarget` and executes drop -4. `e.stopPropagation()` prevents event bubbling to parent containers - -**Root Drop Zone Pattern**: - -For dropping onto empty space (background), use event bubbling: - -```typescript -// In parent container -const handleTreeMouseUp = useCallback(() => { - const PopupLayer = require('@noodl-views/popuplayer'); - - // If we're dragging and no specific item claimed the drop, it's a root drop - if (draggedItem && PopupLayer.instance.isDragging()) { - handleDropOnRoot(draggedItem); - } -}, [draggedItem, handleDropOnRoot]); - -// JSX -
- {/* Tree items call e.stopPropagation() on valid drops */} - {/* If no item stops propagation, this handler catches it */} -
; -``` - -**Critical Rule**: **If using PopupLayer's drag system, always use `onMouseUp` for drop detection, never `onDrop`.** - -**Detection**: If drops appear to work (visual feedback shows) but never complete (items snap back), check if you're using `onDrop` instead of `onMouseUp`. +**Time Lost**: 30 minutes investigating "missing styles" before discovering the variable mismatch **Location**: -- Fixed in: `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/components/ComponentItem.tsx` -- Fixed in: `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/components/FolderItem.tsx` -- Fixed in: `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/ComponentsPanelReact.tsx` -- Task: `dev-docs/tasks/phase-2/TASK-008-componentspanel-menus-and-sheets/TASK-008C-drag-drop-system.md` +- Fixed files: `FolderTree.module.scss`, `FolderTreeItem.module.scss` +- Variable definitions: `packages/noodl-core-ui/src/styles/custom-properties/spacing.css` +- Documentation: `dev-docs/reference/UI-STYLING-GUIDE.md` -**Keywords**: onDrop, onMouseUp, HTML5 DnD, drag-and-drop, PopupLayer, mouse events, drop handler, snap back +**Keywords**: CSS variables, custom properties, --spacing, --theme-spacing, zero padding, invisible UI, variable mismatch, design tokens, spacing scale --- -## 🔥 CRITICAL: Runtime Node Creation - The Unholy Trinity of Silent Failures (Dec 2025) - -### The HTTP Node Debugging Saga: 3+ Hours of Silent Failures - -**Context**: Creating an HTTP node (httpnode.js) as a modern, no-code-friendly alternative to the script-based REST node. Everything looked correct but nothing worked. Signals never fired, config values never set, and no errors anywhere. - -**The Problems Found** (each took significant debugging time): - -#### Problem 1: Signal Input Using `set` Instead of `valueChangedToTrue` - -```javascript -// ❌ WHAT I WROTE (never triggers) -inputs: { - fetch: { - type: 'signal', - set: function(value) { - this.scheduleFetch(); // ☠️ Never called - } - } -} - -// ✅ WHAT IT SHOULD BE -inputs: { - fetch: { - type: 'signal', - valueChangedToTrue: function() { - this.scheduleFetch(); // ✓ Works! - } - } -} -``` - -**Why**: Signals use `EdgeTriggeredInput.createSetter()` which wraps the callback and only calls `valueChangedToTrue` when value transitions from falsy to truthy. The `set` callback is never used. - -#### Problem 2: Custom `setInputValue` Overriding Base Method - -```javascript -// ❌ WHAT I WROTE (breaks ALL inputs including signals) -prototypeExtensions: { - setInputValue: function(name, value) { - this._internal.inputValues[name] = value; // ☠️ Overrides Node.prototype.setInputValue - } -} - -// ✅ WHAT IT SHOULD BE -prototypeExtensions: { - _storeInputValue: function(name, value) { // Different name! - this._internal.inputValues[name] = value; - } -} -``` - -**Why**: `prototypeExtensions` methods are merged into node prototype. Defining `setInputValue` completely replaces the base implementation, which handles signal detection, input.set() calls, and event emission. - -#### Problem 3: Dynamic Ports Replacing Static Ports - -```javascript -// ❌ WHAT I WROTE (static ports disappear) -function updatePorts(nodeId, parameters, editorConnection) { - const ports = []; - // Only add dynamic header/query param ports... - if (parameters.headers) { /* add header ports */ } - editorConnection.sendDynamicPorts(nodeId, ports); // Static inputs GONE! -} - -// ✅ WHAT IT SHOULD BE -function updatePorts(nodeId, parameters, editorConnection) { - const ports = [ - // Re-add ALL static inputs - { name: 'url', displayName: 'URL', type: 'string', plug: 'input', group: 'Request' }, - { name: 'fetch', displayName: 'Fetch', type: 'signal', plug: 'input', group: 'Actions' }, - { name: 'cancel', displayName: 'Cancel', type: 'signal', plug: 'input', group: 'Actions' }, - // Then dynamic ports... - ]; - editorConnection.sendDynamicPorts(nodeId, ports); -} -``` - -**Why**: `sendDynamicPorts` REPLACES all ports, not merges. The `inputs` object in node definition is only for default setup - once dynamic ports are sent, they're the only ports. - -#### Problem 4: Config Inputs (StringList/Enum) Need Explicit Registration - -```javascript -// ❌ MISSING (config values never reach setters) -// StringList inputs like "headers", "queryParams" appear in editor but their -// values never reach the node because there's no registered input handler - -// ✅ WHAT IT NEEDS -registerInputIfNeeded: function(name) { - if (this.hasInput(name)) return; - - const configSetters = { - 'method': this.setMethod.bind(this), - 'headers': this.setHeaders.bind(this), - 'queryParams': this.setQueryParams.bind(this), - 'bodyType': this.setBodyType.bind(this), - // ... all config inputs - }; - - if (configSetters[name]) { - return this.registerInput(name, { set: configSetters[name] }); - } - - // Dynamic inputs (header-X, query-Y, etc.) - if (name.startsWith('header-')) { - return this.registerInput(name, { - set: this._storeInputValue.bind(this, name) - }); - } -} -``` - -**Why**: Inputs defined in the `inputs` object get registered automatically. Dynamic ports don't - they need `registerInputIfNeeded` to create runtime handlers. - -**Why This Was So Hard to Debug**: - -1. **No errors** - Everything appeared to work, logs said "success", but nothing happened -2. **Partial success** - Some things worked (node appeared in palette) making it seem close -3. **Multiple bugs** - Each fix revealed the next bug, each taking time to diagnose -4. **No TypeScript** - Runtime code is JS, no compile-time checking -5. **Unfamiliar patterns** - `valueChangedToTrue`, `registerInputIfNeeded`, etc. aren't obvious - -**Time Lost**: 3+ hours debugging what should have been a straightforward node - -**Prevention**: - -- Created `dev-docs/reference/LEARNINGS-NODE-CREATION.md` with Critical Gotchas section -- Added Node Creation Checklist to `.clinerules` Section 14 -- These gotchas are now documented with THE BUG / WHY IT BREAKS / THE FIX patterns - -**Location**: - -- `packages/noodl-runtime/src/nodes/std-library/data/httpnode.js` (fixed) -- `dev-docs/reference/LEARNINGS-NODE-CREATION.md` (new documentation) - -**Keywords**: node creation, signal, valueChangedToTrue, setInputValue, prototypeExtensions, sendDynamicPorts, registerInputIfNeeded, stringlist, enum, config inputs, HTTP node, runtime - ---- - -## 🔥 CRITICAL: PopupLayer.dragCompleted() - Not endDrag() (Dec 2025) - -### Wrong Method Name Causes TypeError - -**Context**: TASK-008C ComponentsPanel drag-drop system - After fixing the onDrop→onMouseUp issue, drops still failed with a TypeError. - -**The Error**: - -``` -TypeError: PopupLayer.instance.endDrag is not a function -``` - -**The Problem**: Code was calling `PopupLayer.instance.endDrag()`, but this method **doesn't exist**. The correct method is `dragCompleted()`. - -**Root Cause**: Method name was guessed or hallucinated rather than verified against the actual PopupLayer source code. - -**The Broken Pattern**: - -```typescript -// ❌ WRONG - endDrag() doesn't exist -const handleDrop = () => { - // ... do drop operation - PopupLayer.instance.endDrag(); // TypeError! -}; -``` - -**The Solution**: - -```typescript -// ✅ RIGHT - Use dragCompleted() -const handleDrop = () => { - // ... do drop operation - PopupLayer.instance.dragCompleted(); // Works! -}; -``` - -**PopupLayer Drag API Reference** (from `popuplayer.js`): - -```javascript -// Start drag with label that follows cursor -PopupLayer.prototype.startDragging = function (args) { - // args: { label: string } - // Creates .popup-layer-dragger element -}; - -// Check if currently dragging -PopupLayer.prototype.isDragging = function () { - return this.dragItem !== undefined; -}; - -// Show cursor feedback during drag -PopupLayer.prototype.indicateDropType = function (droptype) { - // droptype: 'move' | 'copy' | 'none' -}; - -// ✅ CORRECT: Complete the drag operation -PopupLayer.prototype.dragCompleted = function () { - this.$('.popup-layer-dragger').css({ opacity: '0' }); - this.dragItem = undefined; -}; -``` - -**Critical Rule**: **Always verify method names against actual source code. Never guess or assume API method names.** - -**Why This Matters**: - -- Silent TypeErrors at runtime -- Method names like `endDrag` vs `dragCompleted` are easy to confuse -- Legacy JavaScript codebase has no TypeScript definitions to catch this - -**Detection**: If you get `X is not a function` error on PopupLayer, check `popuplayer.js` for the actual method name. - -**Location**: - -- PopupLayer source: `packages/noodl-editor/src/editor/src/views/popuplayer/popuplayer.js` -- Fixed in: `packages/noodl-editor/src/editor/src/views/panels/ComponentsPanelNew/hooks/useComponentActions.ts` - -**Keywords**: PopupLayer, dragCompleted, endDrag, TypeError, drag-and-drop, method name, API - ---- - -## NoodlRuntime.instance.getMetaData() Pattern for Project Data (Dec 2025) - -### How Runtime Nodes Access Project Metadata - -**Context**: BYOB Query Data node needed to access backend services configuration (URLs, tokens, schema) from runtime code. - -**The Pattern**: Use `NoodlRuntime.instance.getMetaData(key)` to access project metadata stored in graphModel. - -**Working Example** (from byob-query-data.js): - -```javascript -const NoodlRuntime = require('../../../../noodl-runtime'); - -resolveBackend: function() { - // Get metadata - same pattern as cloudstore.js uses for cloudservices - const backendServices = NoodlRuntime.instance.getMetaData('backendServices'); - - if (!backendServices || !backendServices.backends) { - console.log('[BYOB Query Data] No backend services metadata found'); - console.log('[BYOB Query Data] Available metadata keys:', - Object.keys(NoodlRuntime.instance.metadata || {})); - return null; - } - - // Access the data - const backends = backendServices.backends || []; - const activeBackendId = backendServices.activeBackendId; - - // Find and use the backend config... -} -``` - -**Reference Implementation** (from `src/api/cloudstore.js`): - -```javascript -const NoodlRuntime = require('../../noodl-runtime'); - -// Access cloud services config -const cloudServices = NoodlRuntime.instance.getMetaData('cloudservices'); -``` - -**How It Works**: - -- `NoodlRuntime.prototype.getMetaData(key)` delegates to `this.graphModel.getMetaData(key)` -- Metadata is stored in the project file and loaded into graphModel -- Editor components set metadata via `graphModel.setMetaData(key, value)` - -**Available Metadata Keys** (varies by project): - -- `cloudservices` - Parse/Firebase cloud settings -- `backendServices` - BYOB backend configurations -- Project-specific settings - -**Location**: - -- NoodlRuntime API: `packages/noodl-runtime/noodl-runtime.js` (line 299) -- Pattern reference: `packages/noodl-runtime/src/api/cloudstore.js` -- BYOB usage: `packages/noodl-runtime/src/nodes/std-library/data/byob-query-data.js` - -**Keywords**: NoodlRuntime, getMetaData, project metadata, runtime, backend config, cloudservices - ---- - -## 🔴 Runtime Nodes Must Be in coreNodes Index (Dec 2025) - -### Problem: Node Module Loads But Doesn't Appear in Node Picker - -**Context**: TASK-002 BYOB Data Nodes - Created `byob-query-data.js`, registered in `register-nodes.js`, console showed "Module loaded" but node never appeared in Node Picker. - -**Root Cause**: Runtime nodes need to be registered in THREE places: - -1. ✅ Node file created (`noodl-runtime/src/nodes/std-library/data/byob-query-data.js`) -2. ✅ Registered in `register-nodes.js` via `require()` -3. ❌ **MISSING** - Added to `coreNodes` index in `nodelibraryexport.js` - -**The Hidden Requirement**: - -```javascript -// In nodelibraryexport.js, the coreNodes array determines Node Picker organization -const coreNodes = [ - { - name: 'Read & Write Data', - subCategories: [ - { - name: 'External Data', - items: ['net.noodl.HTTP', 'REST2'] // HTTP appears because it's HERE - } - // Node not in this array = not in Node Picker! - ] - } -]; -``` - -**The Fix**: - -```javascript -// Add node to appropriate category in coreNodes -{ - name: 'BYOB Data', - items: ['noodl.byob.QueryData'] // Now appears in Node Picker! -} -``` - -**Why This Is Easy to Miss**: - -- Module loads fine (console log appears) -- No errors anywhere -- Node IS registered in `nodeRegister._constructors` -- Node IS in `nodetypes` array exported to editor -- But Node Picker uses `coreNodes` index for organization - -**Critical Rule**: After creating a node, ALWAYS add it to `nodelibraryexport.js` coreNodes array. - -**Location**: - -- `packages/noodl-runtime/src/nodelibraryexport.js` (coreNodes array) -- Documented in: `dev-docs/reference/LEARNINGS-NODE-CREATION.md` (Step 3) - -**Keywords**: node picker, coreNodes, nodelibraryexport, runtime node, silent failure, node not appearing - ---- - -## 🔥 CRITICAL: LocalProjectsModel.loadProject() Doesn't Navigate to Editor (Dec 2025) - -### The Silent Success: Projects Load But Don't Open - -**Context**: Phase 3 Launcher integration - Implemented all project management buttons (Open Project, Launch Project, Create Project). LocalProjectsModel.loadProject() succeeded and returned a ProjectModel, but the editor never opened. - -**The Problem**: `LocalProjectsModel.loadProject()` **only loads the project into memory**. It does NOT navigate to the editor. You must explicitly call the router after loading. - -**Discovery Timeline**: - -``` -🔵 [handleOpenProject] Starting... -🔵 [handleOpenProject] Selected folder: /Users/.../MAU chatbot -🔵 [handleOpenProject] Calling openProjectFromFolder... -🔵 [handleOpenProject] Got project: ProjectModel -🔵 [handleOpenProject] Loading project... -🔵 [handleOpenProject] Project loaded: ProjectModel -✅ [handleOpenProject] Success! Project should open now -// But no project opens 🤔 -``` - -**Root Cause**: Missing router navigation call after successful project load. - -**The Broken Pattern**: - -```typescript -// ❌ WRONG - Project loads but editor never opens -const handleOpenProject = async () => { - const direntry = await filesystem.openDialog({ ... }); - const project = await LocalProjectsModel.instance.openProjectFromFolder(direntry); - const projectEntry = LocalProjectsModel.instance.getProjects().find(p => p.id === project.id); - const loaded = await LocalProjectsModel.instance.loadProject(projectEntry); - // ☠️ Returns ProjectModel but nothing happens - // User stays on dashboard, confused -}; -``` - -**The Solution**: - -```typescript -// ✅ RIGHT - Navigate to editor after loading -const handleOpenProject = useCallback(async () => { - const direntry = await filesystem.openDialog({ ... }); - const project = await LocalProjectsModel.instance.openProjectFromFolder(direntry); - const projectEntry = LocalProjectsModel.instance.getProjects().find(p => p.id === project.id); - const loaded = await LocalProjectsModel.instance.loadProject(projectEntry); - - if (loaded) { - // Navigate to editor with loaded project - props.route.router.route({ to: 'editor', project: loaded }); - } -}, [props.route]); -``` - -**Why This Pattern Exists**: - -1. `loadProject()` is a data operation - it loads project files, initializes ProjectModel, sets up modules -2. Router navigation is a separate concern - handled by the Router component -3. In Phase 3, `ProjectsPage` (React) must explicitly trigger navigation -4. In legacy code, this was handled by imperative routing in Backbone views - -**The Router Navigation Pattern** (Phase 3): - -```typescript -// Access router through props.route.router -interface ProjectsPageProps extends IRouteProps { - from: TSFixme; - // route.router is the Router instance -} - -// Navigate with project data -props.route.router.route({ - to: 'editor', // Target route - project: loaded // ProjectModel instance -}); - -// Navigate without project (back to dashboard) -props.route.router.route({ - to: 'projects', - from: 'editor' -}); -``` - -**Applies to All Project Opening Scenarios**: - -1. **Create Project**: After `newProject()` callback → navigate to editor -2. **Open Project**: After `openProjectFromFolder()` + `loadProject()` → navigate to editor -3. **Launch Project**: After `loadProject()` from list → navigate to editor - -**Critical Rule**: **`loadProject()` only loads data. Always call `props.route.router.route()` to actually open the editor.** - -**Location**: - -- Fixed in: `packages/noodl-editor/src/editor/src/pages/ProjectsPage/ProjectsPage.tsx` -- Router source: `packages/noodl-editor/src/editor/src/router.tsx` -- Phase 3 context: DASH-001 Tabbed Navigation integration - -**Detection**: If you see success logs but the editor doesn't open, you're missing the router navigation call. - -**Keywords**: LocalProjectsModel, loadProject, router, navigation, Phase 3, Launcher, ProjectsPage, silent success, openProjectFromFolder - ---- +[Rest of the previous learnings content continues...] diff --git a/dev-docs/reference/UI-STYLING-GUIDE.md b/dev-docs/reference/UI-STYLING-GUIDE.md index 3793121..1f389c2 100644 --- a/dev-docs/reference/UI-STYLING-GUIDE.md +++ b/dev-docs/reference/UI-STYLING-GUIDE.md @@ -290,6 +290,135 @@ Before completing any UI task, verify: --- +## Part 9: Selected/Active State Patterns + +### Decision Matrix: Which Background to Use? + +When styling selected or active items, choose based on the **level of emphasis** needed: + +| Context | Background Token | Text Color | Use Case | +| -------------------- | ----------------------- | --------------------------------------- | ---------------------------------------------- | +| **Subtle highlight** | `--theme-color-bg-4` | `--theme-color-fg-highlight` | Breadcrumb current page, sidebar selected item | +| **Medium highlight** | `--theme-color-bg-5` | `--theme-color-fg-highlight` | Hovered list items, tabs | +| **Bold accent** | `--theme-color-primary` | `var(--theme-color-on-primary)` (white) | Dropdown selected item, focused input | + +### Common Pattern: Dropdown/Menu Selected Items + +```scss +.MenuItem { + padding: 8px 12px; + cursor: pointer; + + // Default state + color: var(--theme-color-fg-default); + background-color: transparent; + + // Hover state (if not selected) + &:hover:not(.is-selected) { + background-color: var(--theme-color-bg-3); + color: var(--theme-color-fg-highlight); + } + + // Selected state - BOLD accent for visibility + &.is-selected { + background-color: var(--theme-color-primary); + color: var(--theme-color-on-primary); + + // Icons and child elements also need white + svg path { + fill: var(--theme-color-on-primary); + } + } + + // Disabled state + &:disabled { + opacity: 0.5; + cursor: not-allowed; + } +} +``` + +### Common Pattern: Navigation/Breadcrumb Current Item + +```scss +.BreadcrumbItem { + padding: 6px 12px; + border-radius: 4px; + color: var(--theme-color-fg-default); + + // Current/active page - SUBTLE highlight + &.is-current { + background-color: var(--theme-color-bg-4); + color: var(--theme-color-fg-highlight); + } +} +``` + +### ⚠️ CRITICAL: Never Use These for Backgrounds + +**DO NOT use these tokens for selected/active backgrounds:** + +```scss +/* ❌ WRONG - These are now WHITE after token consolidation */ +background-color: var(--theme-color-secondary); +background-color: var(--theme-color-secondary-highlight); +background-color: var(--theme-color-fg-highlight); + +/* ❌ WRONG - Poor contrast on dark backgrounds */ +background-color: var(--theme-color-bg-1); /* Too dark */ +background-color: var(--theme-color-bg-2); /* Too dark */ +``` + +### Visual Hierarchy Example + +```scss +// List with multiple states +.ListItem { + // Normal + background: transparent; + color: var(--theme-color-fg-default); + + // Hover (not selected) + &:hover:not(.is-selected) { + background: var(--theme-color-bg-3); // Subtle lift + } + + // Selected + &.is-selected { + background: var(--theme-color-primary); // Bold, can't miss it + color: white; + } + + // Selected AND hovered + &.is-selected:hover { + background: var(--theme-color-primary-highlight); // Slightly lighter red + } +} +``` + +### Accessibility Checklist for Selected States + +- [ ] Selected item is **immediately visible** (high contrast) +- [ ] Color is not the **only** indicator (use icons/checkmarks too) +- [ ] Keyboard focus state is **distinct** from selection +- [ ] Text contrast meets **WCAG AA** (4.5:1 minimum) + +### Real-World Examples + +✅ **Good patterns** (fixed December 2025): + +- `MenuDialog.module.scss` - Uses `--theme-color-primary` for selected dropdown items +- `NodeGraphComponentTrail.module.scss` - Uses `--theme-color-bg-4` for current breadcrumb +- `search-panel.module.scss` - Uses `--theme-color-bg-4` for active search result + +❌ **Anti-patterns** (to avoid): + +- Using `--theme-color-secondary` as background (it's white now!) +- No visual distinction between selected and unselected items +- Low contrast text on selected backgrounds + +--- + ## Quick Grep Commands ```bash @@ -301,8 +430,11 @@ grep -rE '#[0-9a-fA-F]{3,6}' packages/noodl-editor/src/editor/src/styles/ # Find usage of a specific token grep -r "theme-color-primary" packages/ + +# Find potential white-on-white issues +grep -r "theme-color-secondary" packages/ --include="*.scss" --include="*.css" ``` --- -_Last Updated: December 2024_ +_Last Updated: December 2025_ diff --git a/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-000-styles-overhaul/TASK-000I-node-graph-visual-improvements/CHANGELOG.md b/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-000-styles-overhaul/TASK-000I-node-graph-visual-improvements/CHANGELOG.md new file mode 100644 index 0000000..662dbe9 --- /dev/null +++ b/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-000-styles-overhaul/TASK-000I-node-graph-visual-improvements/CHANGELOG.md @@ -0,0 +1,158 @@ +# TASK-000I Changelog + +## Overview + +This changelog tracks the implementation of Node Graph Visual Improvements, covering visual polish, node comments, and port organization features. + +### Implementation Sessions + +1. **Session 1**: Sub-Task A - Rounded Corners & Colors +2. **Session 2**: Sub-Task A - Connection Points & Label Truncation +3. **Session 3**: Sub-Task B - Comment Data Layer & Icon +4. **Session 4**: Sub-Task B - Hover Preview & Edit Modal +5. **Session 5**: Sub-Task C - Port Grouping System +6. **Session 6**: Sub-Task C - Type Icons & Connection Preview +7. **Session 7**: Integration & Polish + +--- + +## [Date TBD] - Task Created + +### Summary + +Task documentation created for Node Graph Visual Improvements based on product planning discussion. + +### Files Created + +- `dev-docs/tasks/phase-3/TASK-010-node-graph-visual/README.md` - Full task specification +- `dev-docs/tasks/phase-3/TASK-010-node-graph-visual/CHECKLIST.md` - Implementation checklist +- `dev-docs/tasks/phase-3/TASK-010-node-graph-visual/CHANGELOG.md` - This file +- `dev-docs/tasks/phase-3/TASK-010-node-graph-visual/NOTES.md` - Working notes + +### Context + +Discussion identified three key areas for improvement: + +1. Nodes look dated (sharp corners, flat colors) +2. No way to document individual nodes with comments +3. Dense nodes with many ports become hard to read + +Decision made to implement as three sub-tasks that can be tackled incrementally. + +--- + +## Progress Summary + +| Sub-Task | Status | Date Started | Date Completed | +| ---------------------- | ----------- | ------------ | -------------- | +| A1: Rounded Corners | Not Started | - | - | +| A2: Color Palette | Not Started | - | - | +| A3: Connection Points | Not Started | - | - | +| A4: Label Truncation | Not Started | - | - | +| B1: Comment Data Layer | Not Started | - | - | +| B2: Comment Icon | Not Started | - | - | +| B3: Hover Preview | Not Started | - | - | +| B4: Edit Modal | Not Started | - | - | +| B5: Click Integration | Not Started | - | - | +| C1: Port Grouping | Not Started | - | - | +| C2: Type Icons | Not Started | - | - | +| C3: Connection Preview | Not Started | - | - | + +--- + +## Template for Session Entries + +```markdown +## [YYYY-MM-DD] - Session N: [Sub-Task Name] + +### Summary + +[Brief description of what was accomplished] + +### Files Created + +- `path/to/file.ts` - [Purpose] + +### Files Modified + +- `path/to/file.ts` - [What changed and why] + +### Technical Notes + +- [Key decisions made] +- [Patterns discovered] +- [Gotchas encountered] + +### Visual Changes + +- [Before/after description] +- [Screenshot references] + +### Testing Notes + +- [What was tested] +- [Edge cases discovered] + +### Next Steps + +- [What needs to be done next] +``` + +--- + +## Blockers Log + +| Date | Blocker | Resolution | Time Lost | +| ---- | ------- | ---------- | --------- | +| - | - | - | - | + +--- + +## Performance Notes + +| Scenario | Before | After | Notes | +| -------------------- | ------ | ----- | ------------ | +| Render 50 nodes | - | - | Baseline TBD | +| Render 100 nodes | - | - | Baseline TBD | +| Pan/zoom performance | - | - | Baseline TBD | + +--- + +## Design Decisions Log + +| Decision | Options Considered | Choice Made | Rationale | +| ------------------- | ------------------------------- | ----------- | ------------------------------ | +| Corner radius | 4px, 6px, 8px | TBD | - | +| Comment icon | Speech bubble, Note icon, "i" | TBD | - | +| Preview delay | 200ms, 300ms, 500ms | 300ms | Balance responsiveness vs spam | +| Port group collapse | Remember state, Reset on reload | Reset | Simpler, no persistence needed | + +--- + +## Screenshots + +_Add before/after screenshots as implementation progresses_ + +### Before (Baseline) + +- [ ] Capture current node appearance +- [ ] Capture dense node example +- [ ] Capture current colors + +### After Sub-Task A + +- [ ] New rounded corners +- [ ] Updated colors +- [ ] Refined connection points + +### After Sub-Task B + +- [ ] Comment icon on node +- [ ] Hover preview +- [ ] Edit modal + +### After Sub-Task C + +- [ ] Grouped ports example +- [ ] Type icons +- [ ] Connection preview highlight diff --git a/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-000-styles-overhaul/TASK-000I-node-graph-visual-improvements/CHECKLIST.md b/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-000-styles-overhaul/TASK-000I-node-graph-visual-improvements/CHECKLIST.md new file mode 100644 index 0000000..77d5566 --- /dev/null +++ b/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-000-styles-overhaul/TASK-000I-node-graph-visual-improvements/CHECKLIST.md @@ -0,0 +1,224 @@ +# TASK-000I Implementation Checklist + +## Pre-Implementation + +- [ ] Review `NodeGraphEditorNode.ts` paint() method thoroughly +- [ ] Review `colors.css` current color definitions +- [ ] Review `NodeGraphNode.ts` metadata structure +- [ ] Test Canvas roundRect() browser support +- [ ] Set up test project with complex node graphs + +--- + +## Sub-Task A: Visual Polish + +### A1: Rounded Corners + +- [ ] Create `canvasHelpers.ts` with roundRect utility +- [ ] Replace background `fillRect` with roundRect in paint() +- [ ] Update border drawing to use roundRect +- [ ] Update selection highlight to use roundRect +- [ ] Update error/annotation borders to use roundRect +- [ ] Handle title bar corners (top only vs all) +- [ ] Test at various zoom levels +- [ ] Verify no visual artifacts + +### A2: Color Palette Update + +- [ ] Document current color values +- [ ] Design new palette following design system +- [ ] Update `--theme-color-node-data-*` variables +- [ ] Update `--theme-color-node-visual-*` variables +- [ ] Update `--theme-color-node-logic-*` variables +- [ ] Update `--theme-color-node-custom-*` variables +- [ ] Update `--theme-color-node-component-*` variables +- [ ] Update connection colors if needed +- [ ] Verify contrast ratios (WCAG AA minimum) +- [ ] Test in dark theme +- [ ] Get feedback on new colors + +### A3: Connection Point Styling + +- [ ] Identify all port indicator drawing code +- [ ] Increase hit area size (4px → 6px?) +- [ ] Add subtle inner highlight effect +- [ ] Improve anti-aliasing +- [ ] Test connection dragging still works +- [ ] Verify hover states visible + +### A4: Port Label Truncation + +- [ ] Create truncateText utility function +- [ ] Integrate into drawPlugs() function +- [ ] Calculate available width correctly +- [ ] Add ellipsis character (…) +- [ ] Verify tooltip shows full name on hover +- [ ] Test with various label lengths +- [ ] Test with RTL text (if applicable) + +### A: Integration & Polish + +- [ ] Full visual review of all node types +- [ ] Performance profiling +- [ ] Update any hardcoded colors +- [ ] Screenshots for documentation + +--- + +## Sub-Task B: Node Comments System + +### B1: Data Layer + +- [ ] Add `comment?: string` to NodeMetadata interface +- [ ] Implement `getComment()` method +- [ ] Implement `setComment()` method with undo support +- [ ] Implement `hasComment()` helper +- [ ] Add 'commentChanged' event emission +- [ ] Verify comment persists in project JSON +- [ ] Verify comment included in node copy/paste +- [ ] Write unit tests for data layer + +### B2: Comment Icon Rendering + +- [ ] Design/source comment icon (speech bubble) +- [ ] Add icon drawing in paint() after title +- [ ] Show solid icon when comment exists +- [ ] Show faded icon on hover when no comment +- [ ] Calculate correct icon position +- [ ] Store hit bounds for click detection +- [ ] Test icon visibility at all zoom levels + +### B3: Hover Preview + +- [ ] Add hover state tracking for comment icon +- [ ] Implement 300ms debounce timer +- [ ] Create preview content formatter +- [ ] Position preview near icon, not obscuring node +- [ ] Set max dimensions (250px × 150px) +- [ ] Add scroll for long comments +- [ ] Clear preview on mouse leave +- [ ] Clear preview on pan/zoom start +- [ ] Test rapid mouse movement (no spam) + +### B4: Edit Modal + +- [ ] Create `NodeCommentEditor.tsx` component +- [ ] Create `NodeCommentEditor.module.scss` styles +- [ ] Implement draggable header +- [ ] Implement textarea with auto-focus +- [ ] Handle Save button click +- [ ] Handle Cancel button click +- [ ] Handle Cmd+Enter to save +- [ ] Handle Escape to cancel +- [ ] Show node name in header +- [ ] Position modal near node initially +- [ ] Prevent duplicate modals for same node + +### B5: Click Handler Integration + +- [ ] Add comment icon click detection +- [ ] Open modal on icon click +- [ ] Prevent node selection on icon click +- [ ] Handle modal close callback +- [ ] Update node display after comment change + +### B: Integration & Polish + +- [ ] End-to-end test: create, edit, delete comment +- [ ] Test with very long comments +- [ ] Test with special characters +- [ ] Test undo/redo flow +- [ ] Test save/load project +- [ ] Test export behavior +- [ ] Accessibility review (keyboard nav) + +--- + +## Sub-Task C: Port Organization & Smart Connections + +### C1: Port Grouping - Data Model + +- [ ] Define PortGroup interface +- [ ] Add portGroups to node type schema +- [ ] Create port group configuration for HTTP node +- [ ] Create port group configuration for Object node +- [ ] Create port group configuration for Function node +- [ ] Create auto-grouping logic for unconfigured nodes +- [ ] Store group expand state in view + +### C1: Port Grouping - Rendering + +- [ ] Modify measure() to account for groups +- [ ] Implement group header drawing +- [ ] Implement expand/collapse chevron +- [ ] Draw ports within expanded groups +- [ ] Skip ports in collapsed groups +- [ ] Update connection positioning for grouped ports +- [ ] Handle click on group header + +### C1: Port Grouping - Testing + +- [ ] Test grouped node rendering +- [ ] Test collapse/expand toggle +- [ ] Test connections to grouped ports +- [ ] Test node without groups (unchanged) +- [ ] Test dynamic ports (wildcard matching) +- [ ] Verify no regression on existing projects + +### C2: Port Type Icons + +- [ ] Design icon set (signal, string, number, boolean, object, array, color, any) +- [ ] Create icon paths/characters in `portIcons.ts` +- [ ] Integrate icon drawing into port rendering +- [ ] Size icons appropriately (10-12px) +- [ ] Match icon color to port type +- [ ] Test visibility at minimum zoom +- [ ] Ensure icons don't interfere with labels + +### C3: Connection Preview on Hover + +- [ ] Add highlightedPort state to NodeGraphEditor +- [ ] Detect port hover in mouse event handling +- [ ] Implement `getPortCompatibility()` method +- [ ] Highlight compatible ports (glow effect) +- [ ] Dim incompatible ports (reduce opacity) +- [ ] Draw preview line from source to cursor +- [ ] Clear highlight on mouse leave +- [ ] Test with various type combinations +- [ ] Performance test with many visible nodes + +### C: Integration & Polish + +- [ ] Full interaction test +- [ ] Performance profiling +- [ ] Edge case testing +- [ ] Documentation for port group configuration + +--- + +## Final Verification + +- [ ] All three sub-tasks complete +- [ ] No console errors +- [ ] No TypeScript errors +- [ ] Performance acceptable +- [ ] Existing projects load correctly +- [ ] All node types render correctly +- [ ] Copy/paste works +- [ ] Undo/redo works +- [ ] Save/load works +- [ ] Export works +- [ ] Screenshots captured for changelog +- [ ] CHANGELOG.md updated +- [ ] LEARNINGS.md updated with discoveries + +--- + +## Sign-off + +| Sub-Task | Completed | Date | Notes | +| -------------------- | --------- | ---- | ----- | +| A: Visual Polish | ☐ | - | - | +| B: Node Comments | ☐ | - | - | +| C: Port Organization | ☐ | - | - | +| Final Integration | ☐ | - | - | diff --git a/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-000-styles-overhaul/TASK-000I-node-graph-visual-improvements/NOTES.md b/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-000-styles-overhaul/TASK-000I-node-graph-visual-improvements/NOTES.md new file mode 100644 index 0000000..bd92e49 --- /dev/null +++ b/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-000-styles-overhaul/TASK-000I-node-graph-visual-improvements/NOTES.md @@ -0,0 +1,306 @@ +# TASK-000I Working Notes + +## Key Code Locations + +### Node Rendering + +``` +packages/noodl-editor/src/editor/src/views/nodegrapheditor/NodeGraphEditorNode.ts + +Key methods: +- paint() - Main render function (~line 200-400) +- drawPlugs() - Port indicator rendering +- measure() - Calculate node dimensions +- handleMouseEvent() - Click/hover handling +``` + +### Colors + +``` +packages/noodl-core-ui/src/styles/custom-properties/colors.css + +Node colors section (~line 30-60): +- --theme-color-node-data-* +- --theme-color-node-visual-* +- --theme-color-node-logic-* +- --theme-color-node-custom-* +- --theme-color-node-component-* +``` + +### Node Model + +``` +packages/noodl-editor/src/editor/src/models/nodegraphmodel/NodeGraphNode.ts + +- metadata object already exists +- Add comment storage here +``` + +### Node Type Definitions + +``` +packages/noodl-editor/src/editor/src/models/nodelibrary/ + +- Port groups would be defined in node type registration +``` + +--- + +## Canvas API Notes + +### roundRect Support + +- Native `ctx.roundRect()` available in modern browsers +- Fallback for older browsers: + +```javascript +function roundRect(ctx, x, y, w, h, r) { + ctx.beginPath(); + ctx.moveTo(x + r, y); + ctx.lineTo(x + w - r, y); + ctx.arcTo(x + w, y, x + w, y + r, r); + ctx.lineTo(x + w, y + h - r); + ctx.arcTo(x + w, y + h, x + w - r, y + h, r); + ctx.lineTo(x + r, y + h); + ctx.arcTo(x, y + h, x, y + h - r, r); + ctx.lineTo(x, y + r); + ctx.arcTo(x, y, x + r, y, r); + ctx.closePath(); +} +``` + +### Text Measurement + +```javascript +const width = ctx.measureText(text).width; +``` + +### Hit Testing + +Currently done manually by checking bounds - no need to change pattern. + +--- + +## Color Palette Ideas + +### Current (approximate from inspection) + +```css +/* Data nodes - current olive green */ +--base-color-node-green-700: #4a5d23; +--base-color-node-green-600: #5c7029; + +/* Visual nodes - current muted blue */ +--base-color-node-blue-700: #2d4a6d; +--base-color-node-blue-600: #3a5f8a; + +/* Logic nodes - current grey */ +--base-color-node-grey-700: #3d3d3d; +--base-color-node-grey-600: #4a4a4a; + +/* Custom nodes - current pink/magenta */ +--base-color-node-pink-700: #7d3a5d; +--base-color-node-pink-600: #9a4872; +``` + +### Proposed Direction + +```css +/* Data nodes - richer emerald */ +--base-color-node-green-700: #166534; +--base-color-node-green-600: #15803d; + +/* Visual nodes - cleaner slate */ +--base-color-node-blue-700: #334155; +--base-color-node-blue-600: #475569; + +/* Logic nodes - warmer charcoal */ +--base-color-node-grey-700: #3f3f46; +--base-color-node-grey-600: #52525b; + +/* Custom nodes - refined rose */ +--base-color-node-pink-700: #9f1239; +--base-color-node-pink-600: #be123c; +``` + +_Need to test contrast ratios and get visual feedback_ + +--- + +## Port Type Icons + +### Character-based approach (simpler) + +```typescript +const PORT_TYPE_ICONS = { + signal: '⚡', // or custom glyph + string: 'T', + number: '#', + boolean: '◐', + object: '{}', + array: '[]', + color: '●', + any: '◇' +}; +``` + +### SVG path approach (more control) + +```typescript +const PORT_TYPE_PATHS = { + signal: 'M4 0 L8 4 L4 8 L0 4 Z' // lightning bolt + // ... etc +}; +``` + +_Need to evaluate which looks better at 10-12px_ + +--- + +## Port Grouping Logic + +### Auto-grouping heuristics + +```typescript +function autoGroupPorts(ports: Port[]): PortGroup[] { + const signals = ports.filter((p) => isSignalType(p.type)); + const dataInputs = ports.filter((p) => p.direction === 'input' && !isSignalType(p.type)); + const dataOutputs = ports.filter((p) => p.direction === 'output' && !isSignalType(p.type)); + + const groups: PortGroup[] = []; + + if (signals.length > 0) { + groups.push({ name: 'Events', ports: signals, expanded: true }); + } + if (dataInputs.length > 0) { + groups.push({ name: 'Inputs', ports: dataInputs, expanded: true }); + } + if (dataOutputs.length > 0) { + groups.push({ name: 'Outputs', ports: dataOutputs, expanded: true }); + } + + return groups; +} + +function isSignalType(type: string): boolean { + return type === 'signal' || type === '*'; // Check actual type names +} +``` + +### Explicit group configuration example (HTTP node) + +```typescript +{ + portGroups: [ + { + name: 'Request', + ports: ['url', 'method', 'body', 'headers-*'], + defaultExpanded: true + }, + { + name: 'Response', + ports: ['status', 'response', 'responseHeaders'], + defaultExpanded: true + }, + { + name: 'Control', + ports: ['send', 'success', 'failure', 'error'], + defaultExpanded: true + } + ]; +} +``` + +--- + +## Connection Compatibility + +### Existing type checking + +```typescript +// Check NodeLibrary for existing type compatibility logic +NodeLibrary.instance.canConnect(sourceType, targetType); +``` + +### Visual feedback states + +1. **Source port** - Normal rendering (this is what user is hovering) +2. **Compatible** - Brighter, subtle glow, maybe pulse animation +3. **Incompatible** - Dimmed to 50% opacity, greyed connection point + +--- + +## Comment Modal Positioning + +### Algorithm + +```typescript +function calculateModalPosition(node: NodeGraphEditorNode): { x: number; y: number } { + const nodeScreenPos = canvasToScreen(node.global.x, node.global.y); + const nodeWidth = node.nodeSize.width * currentScale; + const nodeHeight = node.nodeSize.height * currentScale; + + // Position to the right of the node + let x = nodeScreenPos.x + nodeWidth + 20; + let y = nodeScreenPos.y; + + // Check if off-screen right, move to left + if (x + MODAL_WIDTH > window.innerWidth) { + x = nodeScreenPos.x - MODAL_WIDTH - 20; + } + + // Check if off-screen bottom + if (y + MODAL_HEIGHT > window.innerHeight) { + y = window.innerHeight - MODAL_HEIGHT - 20; + } + + return { x, y }; +} +``` + +--- + +## Learnings to Add to LEARNINGS.md + +_Add these after implementation:_ + +- [ ] Canvas roundRect browser support findings +- [ ] Performance impact of rounded corners +- [ ] Comment storage in metadata - any gotchas +- [ ] Port grouping measurement calculations +- [ ] Connection preview performance considerations + +--- + +## Questions to Resolve + +1. ~~Should rounded corners apply to title bar only or whole node?~~ + + - Decision: Whole node with consistent radius + +2. What happens to comments when node is copied to different project? + + - Need to test metadata handling in import/export + +3. Should port groups be user-customizable or only defined in node types? + + - Start with node type definitions, user customization is future enhancement + +4. How to handle groups for Component nodes (user-defined ports)? + - Auto-group based on port direction (input/output) + +--- + +## Reference Screenshots + +_Add reference screenshots here during implementation for comparison_ + +### Design References + +- [ ] Modern node-based tools (Unreal Blueprints, Blender Geometry Nodes) +- [ ] Other low-code tools for comparison + +### OpenNoodl Current State + +- [ ] Capture before screenshots +- [ ] Note specific problem areas diff --git a/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-000-styles-overhaul/TASK-000I-node-graph-visual-improvements/README.md b/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-000-styles-overhaul/TASK-000I-node-graph-visual-improvements/README.md new file mode 100644 index 0000000..36eacf5 --- /dev/null +++ b/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-000-styles-overhaul/TASK-000I-node-graph-visual-improvements/README.md @@ -0,0 +1,786 @@ +# TASK-000I: Node Graph Visual Improvements + +## Overview + +Modernize the visual appearance of the node graph canvas, add a node comments system, and improve port label handling. This is a high-impact visual refresh that maintains backward compatibility while significantly improving the user experience for complex node graphs. + +**Phase:** 3 (Visual Improvements) +**Priority:** High +**Estimated Time:** 35-50 hours total +**Risk Level:** Low-Medium + +--- + +## Background + +The node graph is the heart of OpenNoodl's visual programming experience. While functionally solid, the current visual design shows its age: + +- Nodes have sharp corners and flat colors that feel dated +- No way to attach documentation/comments to individual nodes +- Port labels overflow on nodes with many connections +- Dense nodes (Object, State, Function) become hard to read + +This task addresses these pain points through three sub-tasks that can be implemented incrementally. + +### Current Architecture + +The node graph uses a **hybrid rendering approach**: + +1. **HTML5 Canvas** (`NodeGraphEditorNode.ts`) - Renders: + + - Node backgrounds via `ctx.fillRect()` + - Borders via `ctx.rect()` and `ctx.strokeRect()` + - Port indicators (dots/arrows) via `ctx.arc()` and triangle paths + - Connection lines via bezier curves + - Text labels via `ctx.fillText()` + +2. **DOM Layer** (`domElementContainer`) - Renders: + + - Comment layer (existing, React-based) + - Some overlays and tooltips + +3. **Color System** - Node colors come from: + - `NodeLibrary.instance.colorSchemeForNodeType()` + - Maps to CSS variables in `colors.css` + - Already abstracted - we can update colors without touching Canvas code + +### Key Files + +``` +packages/noodl-editor/src/editor/src/views/ +├── nodegrapheditor.ts # Main editor, paint loop +├── nodegrapheditor/ +│ ├── NodeGraphEditorNode.ts # Node rendering (PRIMARY TARGET) +│ ├── NodeGraphEditorConnection.ts # Connection line rendering +│ └── ... +├── commentlayer.ts # Existing comment system + +packages/noodl-core-ui/src/styles/custom-properties/ +├── colors.css # Design tokens (color updates) + +packages/noodl-editor/src/editor/src/models/ +├── nodegraphmodel/NodeGraphNode.ts # Node data model (metadata storage) +├── nodelibrary/ # Node type definitions, port groups +``` + +--- + +## Sub-Tasks + +### Sub-Task A: Visual Polish (8-12 hours) + +Modernize node appearance without changing functionality. + +### Sub-Task B: Node Comments System (12-18 hours) + +Add ability to attach documentation to individual nodes. + +### Sub-Task C: Port Organization & Smart Connections (15-20 hours) + +Improve port label handling and add connection preview on hover. + +--- + +## Sub-Task A: Visual Polish + +### Scope + +1. **Rounded corners** on all node rectangles +2. **Updated color palette** following design system +3. **Refined connection points** (port dots/arrows) +4. **Port label truncation** with ellipsis for overflow + +### Implementation + +#### A1: Rounded Corners (2-3 hours) + +**Current code** in `NodeGraphEditorNode.ts`: + +```typescript +// Background +ctx.fillRect(x, y, this.nodeSize.width, this.nodeSize.height); + +// Border +ctx.rect(x, y, this.nodeSize.width, this.nodeSize.height); +``` + +**New approach** - Create helper function: + +```typescript +function roundRect(ctx: CanvasRenderingContext2D, x: number, y: number, width: number, height: number, radius: number) { + ctx.beginPath(); + ctx.roundRect(x, y, width, height, radius); // Native Canvas API + ctx.closePath(); +} +``` + +**Apply to:** + +- Node background fill +- Node border stroke +- Selection highlight +- Error/annotation borders +- Title bar area (top corners only, or clip) + +**Radius recommendation:** 6-8px for nodes, 4px for smaller elements + +#### A2: Color Palette Update (2-3 hours) + +Update CSS variables in `colors.css` to use more modern, saturated colors while maintaining the existing semantic meanings: + +| Node Type | Current | Proposed Direction | +| ------------------ | ------------ | -------------------------------- | +| Data (green) | Olive/muted | Richer emerald green | +| Visual (blue) | Muted blue | Cleaner slate blue | +| Logic (grey) | Flat grey | Warmer charcoal with subtle tint | +| Custom (pink) | Magenta-pink | Refined rose/coral | +| Component (purple) | Muted purple | Cleaner violet | + +**Also update:** + +- `--theme-color-signal` (connection lines) +- `--theme-color-data` (connection lines) +- Background contrast between header and body + +**Constraint:** Keep changes within design system tokens, ensure sufficient contrast. + +#### A3: Connection Point Styling (2-3 hours) + +Current port indicators are simple: + +- **Dots** (`ctx.arc`) for data sources +- **Triangles** (manual path) for signals/targets + +**Improvements:** + +- Slightly larger hit areas (currently 4px radius) +- Subtle inner highlight or ring effect +- Smoother anti-aliasing +- Consider pill-shaped indicators for "connected" state + +**Files:** `NodeGraphEditorNode.ts` - `drawPlugs()` function + +#### A4: Port Label Truncation (2-3 hours) + +**Problem:** Long port names overflow the node boundary. + +**Solution:** + +```typescript +function truncateText(ctx: CanvasRenderingContext2D, text: string, maxWidth: number): string { + const ellipsis = '…'; + let truncated = text; + + while (ctx.measureText(truncated + ellipsis).width > maxWidth && truncated.length > 0) { + truncated = truncated.slice(0, -1); + } + + return truncated.length < text.length ? truncated + ellipsis : text; +} +``` + +**Apply in** `drawPlugs()` before `ctx.fillText()`. + +**Tooltip:** Full port name should show on hover (existing tooltip system). + +### Success Criteria - Sub-Task A + +- [ ] All nodes render with rounded corners (radius configurable) +- [ ] Color palette updated, passes contrast checks +- [ ] Connection points are visually refined +- [ ] Long port labels truncate with ellipsis +- [ ] Full port name visible on hover +- [ ] No visual regressions in existing projects +- [ ] Performance unchanged (canvas render time) + +--- + +## Sub-Task B: Node Comments System + +### Scope + +Allow users to attach plain-text comments to any node, with: + +- Small indicator icon when comment exists +- Hover preview (debounced to avoid bombardment) +- Click to open edit modal +- Comments persist with project + +### Design Decisions + +**Storage:** `node.metadata.comment: string` + +- Already have `metadata` object on NodeGraphNode +- Persists with project JSON +- No schema changes needed + +**UI Pattern:** Icon + Hover Preview + Modal + +- Comment icon in title bar (only shows if comment exists OR on hover) +- Hover over icon shows preview tooltip (300ms delay) +- Click opens sticky modal for editing +- Modal can be dragged, stays open while working + +**Why not inline expansion?** + +- Would affect node measurement/layout calculations +- Creates cascade effects on connections +- More invasive to existing code + +### Implementation + +#### B1: Data Layer (1-2 hours) + +**Add to `NodeGraphNode.ts`:** + +```typescript +// In metadata interface +interface NodeMetadata { + // ... existing fields + comment?: string; +} + +// Helper methods +getComment(): string | undefined { + return this.metadata?.comment; +} + +setComment(comment: string | undefined, args?: { undo?: boolean }) { + if (!this.metadata) this.metadata = {}; + + const oldComment = this.metadata.comment; + this.metadata.comment = comment || undefined; // Remove if empty + + this.notifyListeners('commentChanged', { comment }); + + if (args?.undo) { + UndoQueue.instance.push({ + label: 'Edit comment', + do: () => this.setComment(comment), + undo: () => this.setComment(oldComment) + }); + } +} + +hasComment(): boolean { + return !!this.metadata?.comment?.trim(); +} +``` + +#### B2: Comment Icon Rendering (2-3 hours) + +**In `NodeGraphEditorNode.ts` paint function:** + +```typescript +// After drawing title, before drawing ports +if (this.model.hasComment() || this.isHovered) { + this.drawCommentIcon(ctx, x, y, titlebarHeight); +} + +private drawCommentIcon( + ctx: CanvasRenderingContext2D, + x: number, y: number, + titlebarHeight: number +) { + const iconX = x + this.nodeSize.width - 24; // Right side of title + const iconY = y + titlebarHeight / 2; + const hasComment = this.model.hasComment(); + + ctx.save(); + ctx.globalAlpha = hasComment ? 1 : 0.4; + ctx.fillStyle = hasComment ? '#ffffff' : nc.text; + + // Draw speech bubble icon (simple path or loaded SVG) + // ... icon drawing code + + ctx.restore(); + + // Store hit area for click detection + this.commentIconBounds = { x: iconX - 8, y: iconY - 8, width: 16, height: 16 }; +} +``` + +#### B3: Hover Preview (3-4 hours) + +**Requirements:** + +- 300ms delay before showing (avoid bombardment on pan/scroll) +- Cancel if mouse leaves before delay +- Position near node but not obscuring it +- Max width ~250px, max height ~150px with scroll + +**Implementation approach:** + +- Track mouse position in `NodeGraphEditorNode.handleMouseEvent` +- Use `setTimeout` with cleanup for debounce +- Render preview using existing `PopupLayer.showTooltip()` or custom + +```typescript +// In handleMouseEvent, on 'move-in' to comment icon area: +this.commentPreviewTimer = setTimeout(() => { + if (this.model.hasComment()) { + PopupLayer.instance.showTooltip({ + content: this.model.getComment(), + position: { x: iconX, y: iconY + 20 }, + maxWidth: 250 + }); + } +}, 300); + +// On 'move-out': +clearTimeout(this.commentPreviewTimer); +PopupLayer.instance.hideTooltip(); +``` + +#### B4: Edit Modal (4-6 hours) + +**Create new component:** `NodeCommentEditor.tsx` + +```typescript +interface NodeCommentEditorProps { + node: NodeGraphNode; + initialPosition: { x: number; y: number }; + onClose: () => void; +} + +export function NodeCommentEditor({ node, initialPosition, onClose }: NodeCommentEditorProps) { + const [comment, setComment] = useState(node.getComment() || ''); + const [position, setPosition] = useState(initialPosition); + + const handleSave = () => { + node.setComment(comment.trim() || undefined, { undo: true }); + onClose(); + }; + + return ( + +
+
+ Comment: {node.label} + +
+