Remove viewport check from useZoomOut hook #66341
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
[Feature] Zoom Out
[Type] Code Quality
|
Size Change: +6 B (0%) Total Size: 1.81 MB
ℹ️ View Unchanged
|
The useZoomOut hook should not be responsible for checking viewport width. It should respect the desired zoom out level passed by the condition. We should decide if we want to enter zoom out or not where we implement the zoom out hook (the inserter menu).
jeryj
force-pushed
the
refactor/remove-viewport-use-zoom-out
branch
from
0bf119a to
4e2af1e
Compare
|
Flaky tests detected in e95a1e7. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11488132224
|
t-hamano
reviewed
Oct 24, 2024
There was a problem hiding this comment.
I saw this comment. I updated the useZoomOut hook to always disable zoom out mode on small viewports, but if that's not ideal I'd like to update #66330.
I think zoom out can still be valid for smaller screens, so I'd rather not force |
2 tasks
|
Am I correct that #66330 is only needed if this is not backported in time? |
|
The reason I submitted #66330 is because #66308 cannot be backported to the And since this PR is a follow-up to #66308, I don't think this PR can be backported to wp 6.7. I think #66330 should be merged into the |
|
IIUC, should we remove the backport label on this PR? I'll remove it. Feel free to add it back if I'm misunderstanding something! 🙇 |
kevin940726
removed
the
Backport to WP 6.7 Beta/RC
Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com>
karthick-murugan
pushed a commit
to karthick-murugan/gutenberg
that referenced
this pull request
Nov 13, 2024
The useZoomOut hook should not be responsible for checking viewport width. It should respect the desired zoom out level passed by the condition. We should decide if we want to enter zoom out or not where we implement the useZoomOut hook (the inserter menu). --------- Co-authored-by: jeryj <jeryj@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: kevin940726 <kevin940726@git.wordpress.org> Co-authored-by: PARTHVATALIYA <parthvataliya@git.wordpress.org> Co-authored-by: getdave <get_dave@git.wordpress.org>
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.
Follow up to #66308
What?
Moves the viewport check to the inserter menu instead of within the useZoomOut hook.
Why?
The useZoomOut hook should not be responsible for checking viewport width. It should respect the desired zoom out level passed by the condition. We should decide if we want to enter zoom out or not where we implement the zoom out hook (the inserter menu).
How?
Moves the viewport check to the inserter menu instead of within the useZoomOut hook. Also, rather than matching on is wide, we're checking for the condition of "larger than medium"
Testing Instructions
On large viewport
Open inserter
Click patterns
Zoom out should initialize
Close inserter
Zoom out should exit
On small/medium viewport
Open inserter
Click patterns
Zoom out should not be active
Inserter should be full width
Testing Instructions for Keyboard
Screenshots or screencast