Repository navigation
fix(graph): cancel port connection drags when the layer is removed - #452
Conversation
|
Preview is ready. |
Reviewer's GuideImplementationCancel pending and active gestures when the layer is disabled or unmounted, clear drag listeners when a gesture ends, and prevent queued iterations after final unmount.
Track settings identity and route changed settings through enable or disable so reattachment restores observation without overriding explicit enablement when settings are unchanged.
TestsCover cancellation of pending and active gestures across disable, detach, and unmount, plus settings reattachment, queued iterations, and preserved explicit enablement.
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/components/canvas/layers/portConnectionLayer/PortConnectionLayer.ts" line_range="220" />
<code_context>
+ protected afterInit(): void {
this.onSignal(this.props.graph.rootStore.settings.$settings, (value) => {
- this.enabled = Boolean(value.canCreateNewConnections);
+ // Subscribing again must preserve explicit enable/disable calls until the settings change.
+ if (value === this.lastSettings) {
</code_context>
<issue_to_address>
**Connection drags ignore current setting**
When the setting changes while the layer is detached and remains unchanged after reattachment, `afterInit` only registers `onSignal` for future changes, so `enabled` remains stale; connection drags are accepted or rejected contrary to `canCreateNewConnections`.
On reattachment, reconcile `enabled` with the current setting when it differs from `lastSettings`, without overriding explicit enable/disable calls when the setting is unchanged.
Also at `src/components/canvas/layers/portConnectionLayer/PortConnectionLayer.ts:225-229`.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 1 finding to address first, and if cancellation or enablement handling is wrong, a drag could create an unintended graph connection that remains after reverting and may need to be removed manually. The impact is bounded to affected graph data and is repairable.
Blocking findings: src/components/canvas/layers/portConnectionLayer/PortConnectionLayer.ts:220
|
|
||
| protected afterInit(): void { | ||
| this.onSignal(this.props.graph.rootStore.settings.$settings, (value) => { | ||
| this.enabled = Boolean(value.canCreateNewConnections); |
There was a problem hiding this comment.
🟡 Medium · Connection drags ignore current setting
When the setting changes while the layer is detached and remains unchanged after reattachment, afterInit only registers onSignal for future changes, so enabled remains stale; connection drags are accepted or rejected contrary to canCreateNewConnections.
On reattachment, reconcile enabled with the current setting when it differs from lastSettings, without overriding explicit enable/disable calls when the setting is unchanged.
Also at src/components/canvas/layers/portConnectionLayer/PortConnectionLayer.ts:225-229.
Prompt for AI agents
In `src/components/canvas/layers/portConnectionLayer/PortConnectionLayer.ts` at line 220:
**Connection drags ignore current setting**
When the setting changes while the layer is detached and remains unchanged after reattachment, `afterInit` only registers `onSignal` for future changes, so `enabled` remains stale; connection drags are accepted or rejected contrary to `canCreateNewConnections`.
On reattachment, reconcile `enabled` with the current setting when it differs from `lastSettings`, without overriding explicit enable/disable calls when the setting is unchanged.
Also at `src/components/canvas/layers/portConnectionLayer/PortConnectionLayer.ts:225-229`.
Problem
PortConnectionLayerinstalls document drag listeners on port press, before the pointer crosses the drag threshold. Disabling or removing the layer in that interval lets the old gesture start on the next mouse move. Removing a layer during an active drag can also leave it handling the subsequent mouseup.Changes
Verification
npm run test:unit -- --runInBand(497 passed)npm run typechecknpm run lintSummary by Sourcery
Ensure port connection drags are safely cancelled across layer lifecycle changes and remain consistent after reattachment.
Bug Fixes:
Enhancements:
Tests: