Site Editor: Fix broken 'Redo' by removing faulty logic for discarding unsaved Logo changes #37895
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
This effect ran on unmount and used `getGlobalBlockCount` to determine if all Site Logo blocks have been removed, then discards any unsaved changes to the Logo and Icon. This does not work correctly in the Site Editor context, and would fire erroneously when switching between template parts, breaking the ability to 'Undo'/'Redo'.
stacimc
added
[Feature] History
|
Size Change: -77 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
andrewserong
approved these changes
Jan 12, 2022
There was a problem hiding this comment.
Thanks for putting together this fix @stacimc! I had a little trouble replicating the issue at first (it seemed like the first time I do undo/redo it worked on trunk but if I do undo/redo a second time after removing all site logo blocks, I could replicate it — or, if I edit outside of a template part I could replicate it more reliably).
Before: redo stops working
Kapture.2022-01-12.at.11.23.13.mp4
After: redo works reliably
Kapture.2022-01-12.at.11.27.58.mp4
This removal looks good to me to restore the redo behaviour in the site editor. 🤞 we can come up with another idea for how to preserve discarding changes when removing the last site logo, but that sounds like a lower priority task than getting this fix in. LGTM!
glendaviesnz
approved these changes
Jan 12, 2022
|
Opened #37900 to explore a different way to discard unsaved changes when removing the final Site Logo block. For now this should fix the regression. |
23 tasks
youknowriad
added
the
Backport to WP Minor Release
|
Backported to the minor. |
Mamaduka
removed
the
Backport to WP Minor Release
Mamaduka
pushed a commit
that referenced
this pull request
Feb 16, 2022
…37895) This effect ran on unmount and used `getGlobalBlockCount` to determine if all Site Logo blocks have been removed, then discards any unsaved changes to the Logo and Icon. This does not work correctly in the Site Editor context, and would fire erroneously when switching between template parts, breaking the ability to 'Undo'/'Redo'.
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.
Fixes #37760
Description
#35892 introduced some changes to the Site Logo block, including an effect which was intended to discard any unsaved changes to the logo/icon if a user removes all their Site Logo blocks. This effect relied on
getGlobalBlockCountto determine the existence of any Site Logo blocks, but does not work reliably in the Site Editor. If it fires erroneously, it breaks theRedobutton. (See testing instructions).This PR just removes that logic entirely in order to fix the Redo bug. Other options for discarding unsaved Site Logo changes can be explored separately at a much lower priority.
How has this been tested?
Verify you can reproduce the bug on trunk:
Now checkout this branch and repeat the testing instructions. This time, the "Redo" should be available and should re-insert the block removed by "Undo".
Screenshots
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).