fix: space bar panning over Vue nodes in standard nav mode#8998
Draft
christian-byrne wants to merge 2 commits intomainfrom
Draft
fix: space bar panning over Vue nodes in standard nav mode#8998christian-byrne wants to merge 2 commits intomainfrom
christian-byrne wants to merge 2 commits intomainfrom
Conversation
dosubot
bot
added
the
size:L
|
Playwright: ✅ 528 passed, 0 failed · 3 flaky 📊 Browser Reports
|
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 02/22/2026, 05:41:53 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe changes implement a callback-based synchronization mechanism for read-only state in the canvas. The LGraphCanvas now exposes an optional Changes
Sequence Diagram(s)sequenceDiagram
participant Canvas as LGraphCanvas
participant CB as Callback
participant Store as Canvas Store
participant Comp as Components
Note over Canvas,Comp: Initial Setup
Canvas->>Store: canvas.value assigned
Store->>Canvas: Subscribe to onReadOnlyChanged
Canvas->>Store: isReadOnly.value = canvas.read_only
Store->>Comp: isReadOnly exposed in store
Note over Canvas,Comp: Runtime State Change
Comp->>Canvas: set read_only = true
Canvas->>CB: onReadOnlyChanged(true)
CB->>Store: isReadOnly.value = true
Store->>Comp: Reactive update triggers
Comp->>Comp: Re-evaluate (shouldHandleNodePointerEvents)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
📦 Bundle: 4.37 MB gzip 🟢 -146 BDetailsSummary
Category Glance App Entry Points — 21.5 kB (baseline 21.5 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 942 kB (baseline 942 kB) • 🟢 -77 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 68.8 kB (baseline 68.8 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 436 kB (baseline 436 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 738 B (baseline 738 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 43.2 kB (baseline 43.2 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 2.51 MB (baseline 2.51 MB) • 🟢 -123 BStores, services, APIs, and repositories
Status: 13 added / 13 removed Utilities & Hooks — 58 kB (baseline 58.3 kB) • 🟢 -279 BHelpers, composables, and utility bundles
Status: 12 added / 12 removed Vendor & Third-Party — 8.83 MB (baseline 8.83 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.61 MB (baseline 7.61 MB) • ⚪ 0 BBundles that do not match a named category
Status: 47 added / 47 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/core/canvas/useCanvasInteractions.ts`:
- Around line 67-71: The panning branch is bypassed when
wheelCapturedByFocusedElement prevents handling, so modify handleWheel (and
forwardEventToCanvas if needed) to prioritize space‑panning: move the panning
check (shouldHandleNodePointerEvents) before the wheelCapturedByFocusedElement
early return or call forwardEventToCanvas with a force/bypass flag when
shouldHandleNodePointerEvents is false; update forwardEventToCanvas
signature/usage (and any callers/tests) to accept and honor a force boolean so
focused wheel‑capture elements are ignored during space‑panning while normal
wheel capture behavior remains unchanged.
In
`@src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.test.ts`:
- Around line 12-21: The test currently exposes a global mutable ref
(shouldHandleNodePointerEventsRef) used inside the useCanvasInteractions mock
which can leak state between tests; replace that global with a hoisted value via
vi.hoisted(() => ref(true)) and have the mocked useCanvasInteractions return
that hoisted ref and forwardEventToCanvasMock, so each test can reset or modify
shouldHandleNodePointerEvents via the hoisted factory without global mutation;
update the mock declaration that returns useCanvasInteractions to reference the
hoisted variable instead of the top-level ref.
In `@src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.ts`:
- Around line 68-72: When onPointermove and onPointerup detect panning mode
(shouldHandleNodePointerEvents.value is false) you must first end any active
node drag before forwarding events to the canvas; call the existing
safeDragEnd() (or equivalent cleanup) when hasDraggingStarted or
layoutStore.isDraggingVueNodes is true, then forwardEventToCanvas(event) and
return. Update the onPointermove and onPointerup handlers (names shown in the
diff) to perform this conditional cleanup so hasDraggingStarted and
layoutStore.isDraggingVueNodes are cleared before early-returning.
src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.test.ts
Show resolved
Hide resolved
src/renderer/extensions/vueNodes/composables/useNodePointerInteractions.ts
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/renderer/core/canvas/useCanvasInteractions.ts`:
- Around line 65-69: handleWheel currently returns early when
shouldForwardWheelEvent is false, which still allows focused wheel‑capture
elements to block space‑panning; ensure the panning branch runs regardless of
shouldForwardWheelEvent by checking the space‑panning condition
(shouldHandleNodePointerEvents.value) before or independent of the
shouldForwardWheelEvent check so forwardEventToCanvas(event) is invoked when in
panning mode. Update the control flow in handleWheel to evaluate
shouldHandleNodePointerEvents.value (and call forwardEventToCanvas) before doing
the early return based on shouldForwardWheelEvent, keeping existing behavior for
non‑panning cases.
christian-byrne
force-pushed
the
fix/space-var
branch
from
92da196 to
790c64f
Compare
christian-byrne
commented
Feb 20, 2026
Comment on lines
+119
to
+122
| isReadOnly.value = newCanvas.read_only | ||
| newCanvas.onReadOnlyChanged = (value: boolean) => { | ||
| isReadOnly.value = value | ||
| } |
Contributor
Author
There was a problem hiding this comment.
This cannot be the best way to do this.
Bridge LGraphCanvas.read_only to Vue reactivity via onReadOnlyChanged callback so the existing CSS pointer-events-auto/none toggle on LGraphNode.vue and NodeWidgets.vue re-evaluates when space key toggles panning mode. Events then fall through to the LiteGraph canvas naturally — no per-handler forwarding or force flags needed. Fixes #7806 Amp-Thread-ID: https://ampcode.com/threads/T-019c796c-e83c-769d-85f4-20a349994bad
christian-byrne
force-pushed
the
fix/space-var
branch
from
be2b66d to
b812888
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/litegraph/src/LGraphCanvas.ts (1)
401-407: Guard the read-only callback to avoid redundant notifications.
onReadOnlyChangedwill fire even when the value hasn’t changed (e.g., repeated space keydown). Consider short‑circuiting to reduce unnecessary reactive updates while still refreshing the cursor.♻️ Suggested refinement
set read_only(value: boolean) { - this.state.readOnly = value - this._updateCursorStyle() - this.onReadOnlyChanged?.(value) + if (this.state.readOnly === value) return this._updateCursorStyle() + this.state.readOnly = value + this._updateCursorStyle() + this.onReadOnlyChanged?.(value) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/litegraph/src/LGraphCanvas.ts` around lines 401 - 407, The read_only setter currently always calls onReadOnlyChanged even when the value hasn't changed—modify the setter for read_only to compare the incoming value to this.state.readOnly and only invoke the onReadOnlyChanged callback when they differ, while still always assigning this.state.readOnly and calling this._updateCursorStyle(); use the existing onReadOnlyChanged? guard when invoking the callback to avoid redundant notifications.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/lib/litegraph/src/LGraphCanvas.ts`:
- Around line 401-407: The read_only setter currently always calls
onReadOnlyChanged even when the value hasn't changed—modify the setter for
read_only to compare the incoming value to this.state.readOnly and only invoke
the onReadOnlyChanged callback when they differ, while still always assigning
this.state.readOnly and calling this._updateCursorStyle(); use the existing
onReadOnlyChanged? guard when invoking the callback to avoid redundant
notifications.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix space bar panning mode so it correctly pans the canvas (instead of dragging nodes or scrolling widgets) when the user holds space and drags/scrolls over Vue nodes in standard (scroll-to-pan) navigation mode.
Changes
LGraphCanvas.read_onlyto Vue reactivity via anonReadOnlyChangedcallback invoked from theread_onlysetter.canvasStorestores a reactiveisReadOnlyref synced by this callback.shouldHandleNodePointerEventsnow reads from this ref instead of the non-reactivecanvas?.read_only. Also addspointermoveforwarding to canvas during panning mode and wheel event forwarding in standard mode when space is held.Review Focus
onReadOnlyChangedcallback pattern mirrors the existingonChangedcallback used byDragAndScale. All 3 call sites forread_onlygo through the setter (verified by grep).handlePointerinuseCanvasInteractionsstill readscanvas.read_onlyimperatively for legacy mode — intentionally unchanged since legacy mode doesn't use the computed guard.Fixes #7806
┆Issue is synchronized with this Notion page by Unito