mirror of
https://github.com/The-Low-Code-Foundation/OpenNoodl.git
synced 2026-01-11 23:02:56 +01:00
Fixed click handling bug with editor node canvas
This commit is contained in:
Richard Osborne
@@ -0,0 +1,61 @@
|
||||
# TASK-002: React 19 UI Fixes - Changelog
|
||||
|
||||
## 2025-12-08
|
||||
|
||||
### Investigation
|
||||
- Identified root cause: Legacy React 17 APIs still in use after Phase 1 migration
|
||||
- Found 3 files requiring migration:
|
||||
- `nodegrapheditor.debuginspectors.js` - Uses `ReactDOM.render()` and `unmountComponentAtNode()`
|
||||
- `commentlayer.ts` - Creates new `createRoot()` on every render
|
||||
- `TextStylePicker.jsx` - Uses `ReactDOM.render()` and `unmountComponentAtNode()`
|
||||
- Confirmed these errors cause all reported UI bugs (node picker, config panel, wire connectors)
|
||||
|
||||
### Changes Made
|
||||
|
||||
#### nodegrapheditor.debuginspectors.js
|
||||
- **Before**: Used `ReactDOM.render()` at line 60, `ReactDOM.unmountComponentAtNode()` at line 64
|
||||
- **After**: Migrated to React 18+ `createRoot()` API with proper root management
|
||||
|
||||
#### commentlayer.ts
|
||||
- **Before**: Created new roots on every `_renderReact()` call, causing React warnings
|
||||
- **After**: Check if roots exist before creating, reuse existing roots
|
||||
|
||||
#### TextStylePicker.jsx
|
||||
- **Before**: Used `ReactDOM.render()` and `unmountComponentAtNode()` in useEffect
|
||||
- **After**: Migrated to `createRoot()` API with proper cleanup
|
||||
|
||||
### Testing Notes
|
||||
- [ ] Verified right-click node picker works
|
||||
- [ ] Verified plus icon node picker positions correctly
|
||||
- [ ] Verified node config panel appears
|
||||
- [ ] Verified wire connectors can be dragged
|
||||
- [ ] Verified no more React 19 API errors in console
|
||||
|
||||
### Code Changes Summary
|
||||
|
||||
**nodegrapheditor.debuginspectors.js:**
|
||||
- Changed import from `require('react-dom')` to `require('react-dom/client')`
|
||||
- Added `this.root` property to store React root reference
|
||||
- `render()`: Now creates root only once with `createRoot()`, reuses for subsequent renders
|
||||
- `dispose()`: Uses `this.root.unmount()` instead of `ReactDOM.unmountComponentAtNode()`
|
||||
|
||||
**commentlayer.ts:**
|
||||
- `_renderReact()`: Now checks if roots exist before calling `createRoot()`
|
||||
- `renderTo()`: Properly resets roots to `null` after unmounting when switching divs
|
||||
- `dispose()`: Added null checks before unmounting
|
||||
|
||||
**TextStylePicker.jsx:**
|
||||
- Changed import from `ReactDOM from 'react-dom'` to `{ createRoot } from 'react-dom/client'`
|
||||
- `useEffect`: Creates local root with `createRoot()`, renders popup, unmounts in cleanup
|
||||
|
||||
**nodegrapheditor.ts:**
|
||||
- Added `toolbarRoots: Root[]` array to store toolbar React roots
|
||||
- Added `titleRoot: Root | null` for the title bar root
|
||||
- Toolbar rendering now creates roots only once and reuses them
|
||||
- `reset()`: Properly unmounts all toolbar roots and title root
|
||||
|
||||
**createnewnodepanel.ts:**
|
||||
- Added explicit `width: 800px; height: 600px` on container div before React renders
|
||||
- This fixes popup positioning since React 18's `createRoot()` is async
|
||||
- PopupLayer measures dimensions immediately after appending, but async render hasn't finished
|
||||
- With explicit dimensions, PopupLayer calculates correct centered position
|
||||
@@ -0,0 +1,67 @@
|
||||
# TASK-002: React 19 UI Fixes - Checklist
|
||||
|
||||
## Pre-Flight Checks
|
||||
- [x] Confirm on correct branch
|
||||
- [x] Review current error messages in devtools
|
||||
- [x] Understand existing code patterns in each file
|
||||
|
||||
## File Migrations
|
||||
|
||||
### 1. nodegrapheditor.debuginspectors.js (Critical)
|
||||
- [x] Replace `require('react-dom')` with `require('react-dom/client')`
|
||||
- [x] Add `root` property to store React root reference
|
||||
- [x] Update `render()` method:
|
||||
- Create root only once (if not exists)
|
||||
- Use `this.root.render()` instead of `ReactDOM.render()`
|
||||
- [x] Update `dispose()` method:
|
||||
- Use `this.root.unmount()` instead of `ReactDOM.unmountComponentAtNode()`
|
||||
- [ ] Test: Right-click on canvas should show node picker
|
||||
- [ ] Test: Debug inspector popups should work
|
||||
|
||||
### 2. commentlayer.ts (High Priority)
|
||||
- [x] Update `_renderReact()` to check if roots already exist before creating
|
||||
- [x] Only call `createRoot()` if `this.backgroundRoot` is null/undefined
|
||||
- [x] Only call `createRoot()` if `this.foregroundRoot` is null/undefined
|
||||
- [ ] Test: No warnings about "container already passed to createRoot"
|
||||
- [ ] Test: Comment layer renders correctly
|
||||
|
||||
### 3. TextStylePicker.jsx (Medium Priority)
|
||||
- [x] Replace `import ReactDOM from 'react-dom'` with `import { createRoot } from 'react-dom/client'`
|
||||
- [x] Update popup rendering logic to use `createRoot()`
|
||||
- [x] Store root reference for cleanup
|
||||
- [x] Update cleanup to use `root.unmount()` instead of `unmountComponentAtNode()`
|
||||
- [ ] Test: Text style popup opens and closes correctly
|
||||
|
||||
### 4. nodegrapheditor.ts (Additional - Found During Work)
|
||||
- [x] Add `toolbarRoots: Root[]` array for toolbar React roots
|
||||
- [x] Add `titleRoot: Root | null` for title bar root
|
||||
- [x] Update toolbar rendering to reuse roots
|
||||
- [x] Update `reset()` to properly unmount all roots
|
||||
- [ ] Test: Toolbar buttons render correctly
|
||||
|
||||
### 5. createnewnodepanel.ts (Additional - Popup Positioning Fix)
|
||||
- [x] Add explicit dimensions (800x600) to container div
|
||||
- [x] Compensates for React 18's async createRoot() rendering
|
||||
- [ ] Test: Node picker popup appears centered
|
||||
|
||||
## Post-Migration Verification
|
||||
|
||||
### Console Errors
|
||||
- [ ] No `ReactDOM.render is not a function` errors
|
||||
- [ ] No `ReactDOM.unmountComponentAtNode is not a function` errors
|
||||
- [ ] No `createRoot() on a container already passed` warnings
|
||||
|
||||
### UI Functionality
|
||||
- [ ] Right-click on canvas → Node picker appears (not grab hand)
|
||||
- [ ] Click plus icon → Node picker appears in correct position
|
||||
- [ ] Click visual node → Config panel appears on left
|
||||
- [ ] Click logic node → Config panel appears on left
|
||||
- [ ] Drag wire connectors → Connection can be made between nodes
|
||||
- [ ] Debug inspectors → Show values on connections
|
||||
- [ ] Text style picker → Opens and edits correctly
|
||||
- [ ] Comment layer → Comments can be added and edited
|
||||
|
||||
## Final Steps
|
||||
- [x] Update CHANGELOG.md with changes made
|
||||
- [x] Update LEARNINGS.md if new patterns discovered
|
||||
- [ ] Commit changes with descriptive message
|
||||
85
dev-docs/tasks/phase-2/TASK-002-react19-ui-fixes/README.md
Normal file
85
dev-docs/tasks/phase-2/TASK-002-react19-ui-fixes/README.md
Normal file
@@ -0,0 +1,85 @@
|
||||
# TASK-002: React 19 UI Fixes
|
||||
|
||||
## Overview
|
||||
|
||||
This task addresses critical React 19 API migration issues that were not fully completed during Phase 1. These issues are causing multiple UI bugs in the node graph editor.
|
||||
|
||||
## Problem Statement
|
||||
|
||||
After the React 19 migration in Phase 1, several legacy React 17 APIs are still being used in the codebase:
|
||||
- `ReactDOM.render()` - Removed in React 18+
|
||||
- `ReactDOM.unmountComponentAtNode()` - Removed in React 18+
|
||||
- Incorrect `createRoot()` usage (creating new roots on every render)
|
||||
|
||||
These errors crash the node graph editor's mouse event handlers, causing:
|
||||
- Right-click shows 'grab' hand instead of node picker
|
||||
- Plus icon node picker appears at wrong position and overflows
|
||||
- Node config panel doesn't appear when clicking nodes
|
||||
- Wire connectors don't respond to clicks
|
||||
|
||||
## Error Messages
|
||||
|
||||
```
|
||||
ReactDOM.render is not a function
|
||||
at DebugInspectorPopup.render (nodegrapheditor.debuginspectors.js:60)
|
||||
|
||||
ReactDOM.unmountComponentAtNode is not a function
|
||||
at DebugInspectorPopup.dispose (nodegrapheditor.debuginspectors.js:64)
|
||||
|
||||
You are calling ReactDOMClient.createRoot() on a container that has already
|
||||
been passed to createRoot() before.
|
||||
at _renderReact (commentlayer.ts:145)
|
||||
```
|
||||
|
||||
## Affected Files
|
||||
|
||||
| File | Issue | Priority |
|
||||
|------|-------|----------|
|
||||
| `nodegrapheditor.debuginspectors.js` | Uses legacy `ReactDOM.render()` & `unmountComponentAtNode()` | **Critical** |
|
||||
| `commentlayer.ts` | Creates new `createRoot()` on every render | **High** |
|
||||
| `TextStylePicker.jsx` | Uses legacy `ReactDOM.render()` & `unmountComponentAtNode()` | **Medium** |
|
||||
|
||||
## Solution
|
||||
|
||||
### Pattern 1: Replace ReactDOM.render() / unmountComponentAtNode()
|
||||
|
||||
```javascript
|
||||
// Before (React 17):
|
||||
const ReactDOM = require('react-dom');
|
||||
ReactDOM.render(<Component />, container);
|
||||
ReactDOM.unmountComponentAtNode(container);
|
||||
|
||||
// After (React 18+):
|
||||
import { createRoot } from 'react-dom/client';
|
||||
const root = createRoot(container);
|
||||
root.render(<Component />);
|
||||
root.unmount();
|
||||
```
|
||||
|
||||
### Pattern 2: Reuse Existing Roots
|
||||
|
||||
```typescript
|
||||
// Before (Wrong):
|
||||
_renderReact() {
|
||||
this.root = createRoot(this.div);
|
||||
this.root.render(<Component />);
|
||||
}
|
||||
|
||||
// After (Correct):
|
||||
_renderReact() {
|
||||
if (!this.root) {
|
||||
this.root = createRoot(this.div);
|
||||
}
|
||||
this.root.render(<Component />);
|
||||
}
|
||||
```
|
||||
|
||||
## Related Tasks
|
||||
|
||||
- TASK-001B-react19-migration (Phase 1) - Initial React 19 migration
|
||||
- TASK-006-typescript5-upgrade (Phase 1) - TypeScript 5 upgrade
|
||||
|
||||
## References
|
||||
|
||||
- [React 18 Migration Guide](https://react.dev/blog/2022/03/08/react-18-upgrade-guide)
|
||||
- [createRoot API](https://react.dev/reference/react-dom/client/createRoot)
|
||||
Reference in New Issue
Block a user