Zoom Out: When double clicking a template while zoomed out , reset zoom level instead of showing dialog #65963
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
…m level instead of showing template editing dialog
andrewserong
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. |
2 tasks
|
Size Change: +34 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
|
👋🏻 Excellent. How similar is this to #65565 ? I wonder if we could make it so that the notification component never exists if we're zoomed out? I don't know if that component should check and set editor modes. |
|
Oh, thank you, I hadn't seen that one! 🤦 It's very close to #65565, so happy to close this one out.
Good question. It seemed slightly more straightforward to co-locate the code so we've got the one double click handler to me, but I don't feel strongly about it 🙂 |
|
Flaky tests detected in 56ddd4e. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11249312334
|
getdave
reviewed
Oct 9, 2024
Comment on lines
64
to
65
| // If the editor is zoomed out, reset the zoom level and switch to | ||
| // edit mode. The dialog will not be shown in this case. |
There was a problem hiding this comment.
Is it worth adding a link to the related Issue? Or perhaps describing that we already have a "double click" handler for exiting Zoom Out so we want to ensure the same behaviour on template parts?
|
Thanks for the feedback @getdave and @draganescu! Your comments reminded me to take another look at the existing hook for handling double click for switching off the zoom out mode. It turns out, I think we can safely re-use that hook in the editor canvas as it checks the editor mode. The result is a simpler changeset IMO, and means that we can consolidate the double-click-to-zoom-out logic in the one place. What do you think? In my manual testing so far, it still seems to work nicely, with the edit template dialog still showing when expected while not zoomed out. Happy to keep iterating on this of course if you have more feedback or ideas! |
andrewserong
added
the
Backport to WP 6.7 Beta/RC
ramonjd
approved these changes
Oct 10, 2024
There was a problem hiding this comment.
Resetting the zoom level on doubleclick feels more natural to me. Furthermore, that double clicking allows me to zoom in on the thing I double clicked on 😄
That the canvas "zooms in" is feedback enough — at least for my brain — to warrant removing the confirmation modal as well.
I can confirm that the modal still appears when double clicking on a non-zoomed out canvas.
Before
2024-10-10.15.24.19.mp4
After
2024-10-10.15.25.29.mp4
As a side note, I don't expect it to be possible, but it would be sweet if double click also recalculated the scroll value so that I'm taken to the y position relative to the place I double clicked.
2024-10-10.15.35.19.mp4
|
Thanks for reviewing, Ramon!
Indeed. There's another PR over in #61465 that seeks to maintain the scroll position, so 🤞 the approach there should get that working. I'll leave this PR open for now, in case there's any more feedback from others. If not, I'll merge this in tomorrow and I'll be sure to add props for @PARTHVATALIYA 's work on #65565. |
Oh from May too! Nice, thanks for linking to it. |
draganescu
approved these changes
Oct 10, 2024
| @@ -61,7 +61,11 @@ export default function EditTemplateBlocksNotification( { contentRef } ) { | |||
| ) { | |||
| return; | |||
| } | |||
| setIsDialogOpen( true ); | |||
|
|
|||
| if ( ! event.defaultPrevented ) { | |||
getdave
deleted the
try/reset-zoom-level-when-dbl-clicking-template-while-zoomed-out
branch
github-actions
bot
removed
the
Backport to WP 6.7 Beta/RC
gutenbergplugin
pushed a commit
that referenced
this pull request
Oct 10, 2024
…om level instead of showing dialog (#65963) * Zoom Out: When double clicking a template while zoomed out, reset zoom level instead of showing template editing dialog * Try reusing useZoomOutModeExit hook
|
I just cherry-picked this PR to the wp/6.7 branch to get it included in the next release: 20a3808 |
github-actions
bot
added
the
Backported to WP Core
|
Thanks for the reviews and for landing this one! 🙏 |
|
Hey @andrewserong I think you forgot me to add to the props. |
|
Apologies @PARTHVATALIYA, it looks like @getdave merged this PR overnight and it didn't include the I'm very sorry your contribution wasn't included here, and thank you so much again for your work on this feature! |
|
This was a bad oversight on my part. I apologise. I'll speak to @desrosj who knows how the props collection works to see if there's something we can do to get your props included. Maybe amending the commit message to include you on the Again, my apologies for the oversight. |
|
Hey @PARTHVATALIYA! Our tooling is not perfect, and there are occasional instances where props are missed. I did check out your w.org profile though, and it seems you have received other props for the 6.7 release cycle, so you will be included in the release credits! In the end, the actual count is not important in the end because we don't rank contributors by contribution numbers. Hope that context helps! And thank you for all of your contributions! |
ciampo
pushed a commit
that referenced
this pull request
Oct 14, 2024
…om level instead of showing dialog (#65963) * Zoom Out: When double clicking a template while zoomed out, reset zoom level instead of showing template editing dialog * Try reusing useZoomOutModeExit hook
karthick-murugan
pushed a commit
to karthick-murugan/gutenberg
that referenced
this pull request
Nov 13, 2024
…om level instead of showing dialog (WordPress#65963) * Zoom Out: When double clicking a template while zoomed out, reset zoom level instead of showing template editing dialog * Try reusing useZoomOutModeExit hook
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 #65505
When double clicking areas of a template while zoomed out (i.e. while template preview is switched on while editing a page or post), reset zoom level instead of showing a dialog.
Props to @PARTHVATALIYA for the original work on this fix over in #65565 🙇
Why?
While zoomed out, the idea in #65505 is that it'll feel more intuitive to cancel out of zoom out mode when double clicking areas of the template, rather than showing the dialog to switch to editing the template. As the issue described, when double clicking outside of the content area, the desire appears to be to get out of zoom out mode as the first priority.
How?
useZoomOutModeExithook, and attach it to the editor canvas so that double clicks on the canvas will result in deactivating the zoom out mode.EditTemplateBlocksNotificationcomponent so that it only handles a double click event if theevent.defaultPreventedhas not be set yet.Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast
Post editor:
2024-10-09.15.26.57.mp4
Editing a page in the site editor:
2024-10-09.15.32.17.mp4