From a64e1131893d8e098a8a404f437642f1866db905 Mon Sep 17 00:00:00 2001 From: Richard Osborne Date: Sun, 11 Jan 2026 14:51:35 +0100 Subject: [PATCH] docs(blockly): Document integration bugs and create TASK-012B Session 3: Bug Investigation & Documentation Discovered critical issues during user testing: - Canvas rendering broken (DOM conflict with React) - Logic Builder button crashes (model API error) - CSS positioning issues Root Cause: - Attempted to wrap legacy canvas in React tabs - Canvas is vanilla JS/jQuery, not React-compatible - Created duplicate DOM containers causing conflicts Resolution: - Created TASK-012B with detailed fix plan - Approach: Separate canvas and Logic Builder completely - Use visibility toggle instead of tab replacement - Canvas = Desktop, Logic Builder = Windows overlay Files Created: - TASK-012B-integration-bugfixes.md (complete task doc) Files Updated: - CHANGELOG.md (Session 3, status update) Key Learning: Don't try to wrap legacy jQuery/vanilla JS in React. Keep them completely separate with event coordination. Next: Implement TASK-012B fixes (~1 hour) --- .../TASK-012-blockly-integration/CHANGELOG.md | 79 ++++++ .../TASK-012B-integration-bugfixes.md | 262 ++++++++++++++++++ 2 files changed, 341 insertions(+) create mode 100644 dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-012-blockly-integration/TASK-012B-integration-bugfixes.md diff --git a/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-012-blockly-integration/CHANGELOG.md b/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-012-blockly-integration/CHANGELOG.md index 74f8f7f..98933fa 100644 --- a/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-012-blockly-integration/CHANGELOG.md +++ b/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-012-blockly-integration/CHANGELOG.md @@ -184,3 +184,82 @@ User clicks "Edit Blocks" ``` ### Ready for Production Testing! ๐Ÿš€ + +--- + +### Session 3: 2026-01-11 (Bug Investigation) + +**Duration:** ~30 minutes + +**Phase:** Investigation & Documentation + +**Discovered Issues:** + +During user testing, discovered critical integration bugs: + +**Bug #1-3, #5: Canvas Not Rendering** + +- Opening project shows blank canvas +- First component click shows nothing +- Second component works normally +- Root cause: CanvasTabs tried to "wrap" canvas in React tab system +- Canvas is rendered via vanilla JS/jQuery, not React +- DOM ID conflict between React component and legacy canvas +- **Resolution:** Created TASK-012B to fix with separation of concerns + +**Bug #4: Logic Builder Button Crash** + +- `this.parent.model.getDisplayName is not a function` +- Root cause: Incorrect assumption about model API +- **Resolution:** Documented fix in TASK-012B + +**Bug #6: Floating "Workspace" Label** + +- CSS positioning issue in property panel +- **Resolution:** Documented fix in TASK-012B + +**Key Learning:** + +- Don't try to wrap legacy jQuery/vanilla JS in React +- Keep canvas and Logic Builder completely separate +- Use visibility toggle instead of replacement +- Canvas = Desktop, Logic Builder = Windows on desktop + +**Files Created:** + +- `TASK-012B-integration-bugfixes.md` - Complete bug fix task documentation + +**Next Steps:** + +- โœ… **Phase A-C Implementation COMPLETE!** +- ๐Ÿ› TASK-012B needed to fix integration issues +- ๐Ÿงช After fixes: Full production testing + +--- + +## Status Update + +### What Works โœ… + +- Blockly workspace component +- Custom Noodl blocks (20+ blocks) +- Code generation system +- Logic Builder runtime node +- Dynamic port registration +- Property panel button (when model API fixed) +- IODetector and code generation pipeline + +### What's Broken ๐Ÿ› + +- Canvas rendering on project open +- Logic Builder button crashes (model API error) +- Canvas/Logic Builder visibility coordination + +### Architecture Decision + +- **Original Plan:** Canvas and Logic Builder as sibling tabs +- **Reality:** Canvas is legacy vanilla JS, can't be React-wrapped +- **Solution:** Keep them separate, use visibility toggle +- **Status:** Documented in TASK-012B for implementation + +### Ready for Production Testing! ๐Ÿš€ (After TASK-012B) diff --git a/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-012-blockly-integration/TASK-012B-integration-bugfixes.md b/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-012-blockly-integration/TASK-012B-integration-bugfixes.md new file mode 100644 index 0000000..efa3da8 --- /dev/null +++ b/dev-docs/tasks/phase-3-editor-ux-overhaul/TASK-012-blockly-integration/TASK-012B-integration-bugfixes.md @@ -0,0 +1,262 @@ +# TASK-012B: Logic Builder Integration Bug Fixes + +**Status:** Ready to Start +**Priority:** High (Blocking) +**Estimated Time:** 1 hour +**Dependencies:** TASK-012 Phase A-C Complete + +--- + +## Overview + +Fix critical integration bugs discovered during testing of the Logic Builder feature. The main issue is an architectural conflict between the canvas rendering system and the React-based CanvasTabs component. + +--- + +## Bugs to Fix + +### ๐Ÿ”ด Critical + +**Bug #1-3, #5: Canvas Not Rendering** + +- Opening a project shows blank canvas initially +- First component click shows nothing +- Second component click works normally +- Tabs not visible (only back/forward buttons) + +**Root Cause:** CanvasTabs tries to "wrap" the canvas in a React tab system, but the canvas is rendered via vanilla JS/jQuery with its own lifecycle. This creates a DOM conflict where the canvas renders into the wrong container. + +**Bug #4: Logic Builder Button Crash** + +``` +this.parent.model.getDisplayName is not a function +``` + +**Root Cause:** Incorrect assumption about model API. Method doesn't exist. + +### ๐ŸŸข Low Priority + +**Bug #6: Floating "Workspace" Label** + +- Label floats in middle of property panel over button +- CSS positioning issue + +--- + +## Root Cause Analysis + +### The Architecture Conflict + +**What Was Attempted:** + +```tsx +// CanvasTabs.tsx - INCORRECT APPROACH +{ + activeTab?.type === 'canvas' &&
{/* Tried to render canvas here */}
; +} +``` + +**The Problem:** + +1. The canvas is **NOT a React component** +2. It's rendered by `nodegrapheditor.ts` using a template +3. My CanvasTabs created a **duplicate** container with same ID +4. This causes DOM conflicts and canvas rendering failures + +**Key Insight:** + +- Canvas = The desktop (always there) +- Logic Builder tabs = Windows on the desktop (overlay) +- NOT: Canvas and Logic Builder as sibling tabs + +--- + +## Fix Strategy (Option 1: Minimal Changes) + +### Principle: Separation of Concerns + +**Don't wrap the canvas. Keep them completely separate:** + +1. CanvasTabs manages **Logic Builder tabs ONLY** +2. Canvas rendering completely untouched +3. Use visibility toggle instead of replacement +4. Coordinate via EventDispatcher + +--- + +## Implementation Plan + +### Step 1: Remove Canvas Tab Logic (20 min) + +**File:** `CanvasTabs.tsx` + +- Remove `type === 'canvas'` condition +- Remove canvas tab rendering +- Only render Logic Builder tabs +- Simplify component to single responsibility + +**File:** `CanvasTabsContext.tsx` + +- Remove canvas tab from initial state +- Only manage Logic Builder tabs +- Add event emission for visibility coordination + +### Step 2: Add Visibility Coordination (10 min) + +**File:** `nodegrapheditor.ts` + +- Listen for `LogicBuilder.TabOpened` event +- Hide canvas elements when Logic Builder opens +- Listen for `LogicBuilder.TabClosed` event +- Show canvas elements when Logic Builder closes + +**CSS Approach:** + +```typescript +// When Logic Builder opens +this.el.find('#nodegraphcanvas').css('display', 'none'); +this.el.find('.other-canvas-elements').css('display', 'none'); + +// When Logic Builder closes (or no tabs) +this.el.find('#nodegraphcanvas').css('display', 'block'); +this.el.find('.other-canvas-elements').css('display', 'block'); +``` + +### Step 3: Fix Logic Builder Button (5 min) + +**File:** `LogicBuilderWorkspaceType.ts` (line ~81) + +```typescript +// BEFORE (broken): +const nodeName = this.parent.model.label || this.parent.model.getDisplayName() || 'Logic Builder'; + +// AFTER (fixed): +const nodeName = this.parent.model.label || this.parent.model.type?.displayName || 'Logic Builder'; +``` + +### Step 4: Fix CSS Label (10 min) + +**File:** `LogicBuilderWorkspaceType.ts` + +Properly structure the HTML with correct CSS classes: + +```typescript +return ` +
+ + +
+`; +``` + +### Step 5: Testing (15 min) + +**Test Scenarios:** + +1. โœ… Open project โ†’ Canvas renders immediately +2. โœ… Click component โ†’ Canvas shows nodes +3. โœ… Add Logic Builder node โ†’ Node appears +4. โœ… Click "Edit Blocks" โ†’ Logic Builder opens, canvas hidden +5. โœ… Close Logic Builder tab โ†’ Canvas shows again +6. โœ… Multiple Logic Builder tabs work +7. โœ… No console errors + +--- + +## Technical Details + +### Event Flow + +``` +User clicks "Edit Blocks" + โ†“ +LogicBuilderWorkspaceType emits: LogicBuilder.OpenTab + โ†“ +CanvasTabsContext creates Logic Builder tab + โ†“ +CanvasTabsContext emits: LogicBuilder.TabOpened + โ†“ +NodeGraphEditor hides canvas + โ†“ +CanvasTabs renders Logic Builder workspace +``` + +``` +User closes Logic Builder tab + โ†“ +CanvasTabsContext removes tab + โ†“ +If no more tabs: CanvasTabsContext emits: LogicBuilder.AllTabsClosed + โ†“ +NodeGraphEditor shows canvas + โ†“ +Canvas resumes normal rendering +``` + +### Files to Modify + +**Major Changes:** + +1. `CanvasTabs.tsx` - Remove canvas logic +2. `CanvasTabsContext.tsx` - Simplify, add events +3. `nodegrapheditor.ts` - Add visibility coordination + +**Minor Fixes:** 4. `LogicBuilderWorkspaceType.ts` - Fix getDisplayName, CSS 5. `CanvasTabs.module.scss` - Remove canvas-specific styles + +--- + +## Success Criteria + +- [x] Canvas renders immediately on project open +- [ ] Canvas shows on first component click +- [ ] Logic Builder button works without errors +- [ ] Logic Builder opens in separate view (canvas hidden) +- [ ] Closing Logic Builder returns to canvas view +- [ ] Multiple Logic Builder tabs work correctly +- [ ] No floating labels or CSS issues +- [ ] No console errors + +--- + +## Risks & Mitigation + +### Risk: Breaking Canvas Rendering + +**Mitigation:** Don't modify canvas rendering code at all. Only add/remove CSS display properties. + +### Risk: Event Coordination Timing + +**Mitigation:** Use EventDispatcher (already proven pattern in codebase). + +### Risk: Edge Cases with Multiple Tabs + +**Mitigation:** Comprehensive testing of tab opening/closing sequences. + +--- + +## Future Improvements (Not in Scope) + +- [ ] Smooth transitions between canvas and Logic Builder +- [ ] Remember last opened Logic Builder tabs +- [ ] Split-screen view (canvas + Logic Builder) +- [ ] Breadcrumb integration for Logic Builder +- [ ] Proper tab bar UI (currently just overlays) + +--- + +## References + +- Original issue: User testing session 2026-01-11 +- Root cause investigation: TASK-012 CHANGELOG Session 2 +- Canvas rendering: `packages/noodl-editor/src/editor/src/views/nodegrapheditor.ts` +- CanvasTabs: `packages/noodl-editor/src/editor/src/views/CanvasTabs/` + +--- + +## Notes + +This is a **debugging and refactoring task**, not new feature development. The goal is to make the already-implemented Logic Builder actually work correctly by fixing the architectural mismatch discovered during testing. + +**Key Learning:** When integrating React with legacy jQuery/vanilla JS code, keep them completely separate rather than trying to wrap one in the other.