mirror of
https://github.com/The-Low-Code-Foundation/OpenNoodl.git
synced 2026-01-12 23:32:55 +01:00
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)
This commit is contained in:
Richard Osborne
@@ -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)
|
||||
|
||||
@@ -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' && <div id="nodegraph-canvas-container">{/* Tried to render canvas here */}</div>;
|
||||
}
|
||||
```
|
||||
|
||||
**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 `
|
||||
<div class="property-panel-input-wrapper">
|
||||
<label class="property-label">Workspace</label>
|
||||
<button id="edit-logic-blocks-btn" class="edit-blocks-button">
|
||||
✨ Edit Logic Blocks
|
||||
</button>
|
||||
</div>
|
||||
`;
|
||||
```
|
||||
|
||||
### 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.
|
||||
Reference in New Issue
Block a user