fix(editor): clear rich text editor when editing shape is deleted#8050
fix(editor): clear rich text editor when editing shape is deleted#8050MitjaBezensek merged 3 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
5 Skipped Deployments
|
| if (prevPageState.editingShapeId && shapesNoLongerInPage.has(prevPageState.editingShapeId)) { | ||
| if (!nextPageState) nextPageState = { ...prevPageState } | ||
| nextPageState.editingShapeId = null | ||
| this._currentRichTextEditor.set(null) |
There was a problem hiding this comment.
good catch! we should make this logic change, however, in setEditingShape, not in this side effect path.
i think this used to maybe work but we added if (!this.canEditShape(id)) return this in setEditingShape so maybe the cleanup code isn't running.
there's some lifecycle stuff there (onEditEnd/onEditStart) that i think might get skipped (potentially) if we don't make the logic check there
There was a problem hiding this comment.
Addressed this in the latest commit. To me that looked the right place to do the fix that will trigger the correct clean up, but I may be missing something so I'd love if you could take another look 🙌
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| const { editingShapeId } = this.editor.getCurrentPageState() | ||
| if (!editingShapeId) return | ||
|
|
||
| // Clear the editing shape |
There was a problem hiding this comment.
Unconditional edit cleanup triggers redundant updates
Low Severity
EditingShape.onExit() now calls this.editor.setEditingShape(null) unconditionally, even in flows where editingShapeId was already cleared before the transition. This forces an extra Editor.run plus _updateCurrentPageState / _currentRichTextEditor.set(null) updates, potentially causing avoidable reactive work and re-renders during state transitions.
There was a problem hiding this comment.
Yeah, think that's correct. We do want to run all the cleanup logic for cases when we are no longer editing a shape.
| // Clear the editing shape | ||
| this.editor.setEditingShape(null) | ||
|
|
||
| updateHoveredShapeId.cancel() |
There was a problem hiding this comment.
It also felt like this additional cleanup should also run?
MitjaBezensek
temporarily deployed
to
bemo-canary
GitHub Actions
Inactive
MitjaBezensek
temporarily deployed
to
e2e-dotcom
GitHub Actions
Inactive
Add 12 new entries from PRs merged since v4.4.0: - Featured: click-through on transparent image pixels (#7942) - API: enum-to-const-object refactor (#8084) - Improvements: SVG sanitizer (#7896), save-on-blur (#8037) - Bug fixes: cross-origin download (#8090), zero-size draw (#8067), rich text toolbar cleanup (#8050), zoom threshold (#8040), selection foreground fallback (#8011), sticky note SVG shadow (#7934), arrow frame clamping (#7932), zero pressure draw (#5693)
2 participants
Closes #8049
See the issue for how to repro.
When a remote user deletes a shape that's being edited,
cleanupInstancePageStatesetseditingShapeId = nulldirectly. ThedefaultSideEffectshandler then transitions toselect.idle, which firesEditingShape.onExit(). ButonExit()had an early return (if (!editingShapeId) return) that bailed because the id was already cleared — sosetEditingShape(null)never ran and_currentRichTextEditorwas never cleared. The rich text toolbar (and link editor) stayed visible.Fix
EditingShape.onExit(). The guard originally protected code that usededitingShapeIdafter callingsetEditingShape(null), but that logic has since moved intosetEditingShapeitself. NowsetEditingShape(null)always runs on exit, clearing_currentRichTextEditor._currentRichTextEditor.set(null)added in the initial commit insidecleanupInstancePageState— it's no longer needed since the state machine exit path handles it.Change type
bugfixTest plan
Release notes
Note
Low Risk
Small, localized change to
EditingShape.onExit()cleanup logic; primary risk is unintended tool switching when leaving edit mode, but the newhadEditingShapeguard narrows that behavior.Overview
Fixes a state-machine edge case where leaving
select.editing_shapecould skip cleanup ifeditingShapeIdhad already been cleared externally (e.g. by collaborative deletion).EditingShape.onExit()now always callseditor.setEditingShape(null)and only reverts the tool-lock-to-textflow when an editing shape actually existed, preventing stale rich-text UI (toolbar/link editor) from remaining visible.Written by Cursor Bugbot for commit 30daedb. This will update automatically on new commits. Configure here.