Block Editor: Fix 'useZoomOut' hook conflicts #67033
Open
+10
−17
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
Mamaduka
added
[Type] Bug
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
6 tasks
Mamaduka
added
[Feature] Zoom Out
and removed
[Feature] Inserter
|
Size Change: +2 B (0%) Total Size: 1.82 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in d654e70. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11856899124
|
|
I definitely appreciate how much simpler this solution is! I added e2e tests in this PR to make it easier to run through all the scenarios. If this PR gets approved, I think we should move over the relevant tests. There is one design divergence with this PR: if you are in zoom out, the blocks tab does not reset the zoom level.
@richtabor wanted the blocks tab to reset zoom level as well. But, maybe if you're starting in zoom out, then it's OK to not reset when going to the blocks tab? If the design team is OK with not disengaging zoom out when switching to the blocks tab in this case, then it does make things simpler. If that's OK, then the only scenario I tested that this fails in is when zoom state is manually toggled while the inserter is open:
FWIW - I like this concept of "If you start from zoom out, don't have the hook automatically do anything. Assume the user knows what they're doing." That's what I also went with in this approach. Your method is much simpler though 😅 |
|
@Mamaduka I do really like this direction. I opened #67040 with this as the base and added a commit to introduce the "controlled mode" concept that fixes the bug about manually disengaging/reengaging zoom out. Also added the relevant e2e tests. Feel free to merge into this or cherry pick if you think they're useful. Thanks for taking a look at this bug! |
|
Thank you, @jeryj! I'll have a look on Monday. With the current number of logical paths, we might end up baking a small state machine into this hook 😄 P.S. I would rather see this inserter behavior removed and let users decide if they want to edit in zoom-out mode. |
|
FWIW, I'm also supportive of removing this behavior entirely. I think we can get into trouble when we assume the user wants to be in a certain mode. |
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.
What?
Fixes #66328.
Alternative to #66381 and #66574.
PR fixes a bug when Zoom out mode is incorrectly enabled/disabled based on the inserter's mounting state and if the user has already manually enabled it.
Testing Instructions
Borrowed from #66381.
From full screen mode
From full screen mode, exit zoom out manually
From zoom out, re-engage after unmountThe fix doesn't handle this case. If the user enables zoom-out mode manually, the visual editor should stay in zoom-out mode until it's disabled again.
Testing Instructions for Keyboard
Same.