Fix undo/redo in site editor code editor's mode #52695
Merged
Conversation
This file contains 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
youknowriad
added
[Type] Bug
|
Size Change: -76 B (0%) Total Size: 1.43 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in a3b3b27. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5577462729
|
Mamaduka
reviewed
Jul 18, 2023
There was a problem hiding this comment.
A general question: Do you think we should retain validation through parsing on blur? It sort of gives you immediate feedback if you mess up the block markup.
With updated behavior (which I really like, btw), you'll only see parse errors after switching to the visual editor.
P.S. I'm unsure if people (except me) find original behavior helpful. We can always land as it is and return the previous onBlur handler later.
Screencast
CleanShot.2023-07-18.at.12.31.32.mp4
|
Myself too, I grew accustumated to the fact that on blur, we parse and serialize again but to be honest I feel like this was just a habit and a wrong behavior. If someone edits HTML, he wants that HTML to persist. |
There was a problem hiding this comment.
Hi @youknowriad, with these changes the undo seems to be character by character, which makes it a little hard to use (each key press creates an undo level). On the edit post, the undo seems to not be happening character by character.
Mamaduka
approved these changes
Jul 18, 2023
That's because in edit-post, it's not relying on the "undo" of the input and not the actual undo/redo stack we have. and sometimes it's very broken in edit-post today as well. So I think we should both update them to use the current PR behavior instead. and for the character per character, that's something separate IMO, we should solve that by implementing a RichText like behavior and this is something that is being explored (reusable hook) in #52375 |
|
I took another look at the post editor and in my test undo/redo there creates an undo stop when you focus out, it's something we can do in the site editor easily too if we wanted but I still feel that it's wrong but it could undo a very long change if you write something very long in the code editor. Instead we should bring RichText's way of coalescing edits. So I'm going to merge this, and I'll be following-up with the same simplification for the post editor too. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
closes #43277
What?
This PR fixes the undo/redo of the site editor's code editor mode but also simplifies the code editor a lot.
How?
The main complexity in the previous implementation is that it was actually making the assumption that when you edit the "content" of a record, you also need to edit the "blocks" property to keep it in sync. The reality is that we don't have to keep these two in sync, we can instead just "unset" the blocks which means whenever the blocks will be needed again, they'll just parse again from the edited "content".
Ideally this PR is also merged in coordination with #52417 which simplifies the blocks parsing logic from content too.
I think we can probably do the same simplification to the code editor of edit-post too.
Testing Instructions
1- Open the site editor
2- Switch to code editor mode
3- Edit the content and use undo/redo
4- Everything should work perfectly (undo/redo is per character but that's a separate issue anyway).