Restores setting zoom out mode to useZoomOut hook #65999
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
|
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. |
jeryj
added
[Type] Bug
|
Size Change: +30 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
|
I thought we'd decided not to use that hook in the Patterns inserter? As I understood it, that hook was "zoom out" so should only handle zooming. But if it's being used everywhere as a proxy for toggling:
...then yes there will be bugs. Ideally this hook would have no existed because it exposes internal "modes" which can only be set by experimental actions. But if we are set on using it then it will need fixes here. |
I think that was decided before the requirements were clearer. I think the required behavior will need a hook or useEffect.
The intention of the hook was originally to set and unset modes, which is what is required to set zoom out mode when entering patterns and restoring the previous mode when exiting the inserter.
I think it should exist to support the desired UX, just not have been made public. |
Yes correct. This is what I meant to say 👍 |
|
If this is/was a public API then the hook should continue to do what it did in 6.6. So let's try to get this merged so we can have a stable experience. |
draganescu
approved these changes
Oct 11, 2024
| @@ -1077,11 +1077,11 @@ _Parameters_ | |||
|
|
|||
| ### useZoomOut | |||
|
|
|||
| A hook used to set the zoomed out view, invoking the hook sets the mode. | |||
| A hook used to set the editor mode to zoomed out mode, invoking the hook sets the mode. | |||
There was a problem hiding this comment.
To note that the "zoom out mode" is already replaced by edit mode.
draganescu
added
the
Backport to WP 6.7 Beta/RC
kevin940726
force-pushed
the
fix/zoom-out-from-pattern
branch
from
d4dbf14 to
7930be8
Compare
|
e2e test failure was showing Zoom Out mode being active on the parsing patterns tests. Assume this is a bug with this PR. Trying a rebase. If this doesn't resolve then this PR won't make Beta 3. |
The test opens the patterns inserter tab, so I think it's correct that zoomed out is enabled. I'm not sure why the pattern parses incorrectly 🤔 |
|
Figured out the issue with the test and pushed a fix. The test relies on an existing Now that this PR enables Zoomed Out mode when the patterns tab is selected, the existing The fix disables zoom out mode before trying to insert the pattern. |
|
Flaky tests detected in 80a63cb. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11327046863
|
github-actions
bot
removed
the
Backport to WP 6.7 Beta/RC
gutenbergplugin
pushed a commit
that referenced
this pull request
Oct 14, 2024
* Restores setting zoom out mode to useZoomOut hook * Fix test --------- Co-authored-by: jeryj <[email protected]> Co-authored-by: talldan <[email protected]> Co-authored-by: draganescu <[email protected]> Co-authored-by: getdave <[email protected]>
github-actions
bot
added
the
Backported to WP Core
|
I just cherry-picked this PR to the wp/6.7 branch to get it included in the next release: 2a28b0d |
ciampo
pushed a commit
that referenced
this pull request
Oct 14, 2024
* Restores setting zoom out mode to useZoomOut hook * Fix test --------- Co-authored-by: jeryj <[email protected]> Co-authored-by: talldan <[email protected]> Co-authored-by: draganescu <[email protected]> Co-authored-by: getdave <[email protected]>
t-hamano
reviewed
Oct 17, 2024
| }, [] ); | ||
|
|
||
| // The effect opens the zoom-out view if we want it open and it's not currently in zoom-out mode. | ||
| useEffect( () => { |
There was a problem hiding this comment.
I came across this PR while searching for the cause of #66205. I feel like we need to check that it's not a mobile layout here, otherwise the width won't be set correctly when we open the Patterns tab in mobile view.
There was a problem hiding this comment.
I think we'll need to wait and see if #66141 gets backported. However, @PARTHVATALIYA's idea to add it to the inserter useZoomOut conditional should work well for either case. If that route will fix the issue, I think that would work well.
There was a problem hiding this comment.
Confirming that we are not intending to backport #66141
2 tasks
karthick-murugan
pushed a commit
to karthick-murugan/gutenberg
that referenced
this pull request
Nov 13, 2024
* Restores setting zoom out mode to useZoomOut hook * Fix test --------- Co-authored-by: jeryj <[email protected]> Co-authored-by: talldan <[email protected]> Co-authored-by: draganescu <[email protected]> Co-authored-by: getdave <[email protected]>
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?
The useZoomOut hook was only setting zoom level, not entering the zoom out mode, which gets it into odd scenarios.
Why?
The bug on trunk:
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Before
https://github.com/user-attachments/assets/dad48d4b-8b56-4d91-bc9e-515f0c5b579b
After
https://github.com/user-attachments/assets/39f08ead-22dc-4c46-b034-fa9385f3e5f1