Conversation
zoltanhosszu
force-pushed
the
history-merging
branch
from
9ef9c14 to
d3dad56
Compare
zoltanhosszu
force-pushed
the
history-merging
branch
from
b418f2f to
a96e8b5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
zoltanhosszu
force-pushed
the
history-merging
branch
from
a96e8b5 to
8142f44
Compare
samuelpecher
left a comment
samuelpecher
left a comment
There was a problem hiding this comment.
I think there has to be a different approach than complex surgery on the history stack. The root issue is that we're modifying the node too much in the background. If we completely bypass that, then there's no need for the surgery.
| this.editor.getEditorState().read(() => { | ||
| this.#syncSelectedClasses() | ||
| }) |
There was a problem hiding this comment.
This would force a render as it's called within a command handler. The better move is unwrapping since the caller will need to call $getSelection and be in a read/update context
| this.editor.getEditorState().read(() => { | |
| this.#syncSelectedClasses() | |
| }) | |
| this.#syncSelectedClasses() |
| // the undo history via HISTORIC_TAG. | ||
| get #transientUpdateTags() { | ||
| if (this.#editorHasFocus) { | ||
| return [ HISTORIC_TAG ] |
There was a problem hiding this comment.
HISTORIC_TAG is intended for use when history operations are applied (undo/redo). Feels dangerous.
This PR fixes 2 issues with regards to editor history:
ActionTextAttachmentUploadNodeDOM with a stale progressbar in the editor.