-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Drop patterns and blocks between sections only in zoom out mode #60828
Drop patterns and blocks between sections only in zoom out mode #60828
Conversation
Size Change: +77 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, works as expected.
I'm curious: is there a way/should we consider only limiting patterns to the top-level while in zoom out, not blocks and patterns?
if ( __unstableGetEditorMode() === 'zoom-out' ) { | ||
const { sectionRootClientId } = unlock( getSettings() ); | ||
const sectionsClientIds = getBlockOrder( sectionRootClientId ); | ||
_isDropZoneDisabled = sectionsClientIds?.includes( clientId ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we changed the block editing mode to account for this? Why do we need to change it here as well? Is this a bug? https://github.com/WordPress/gutenberg/pull/59249/files#diff-aa92c48095c35f17feb834dd919baf52274a73c7b1180ff5acce7b6087b126cfR2923
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wondered the same.
In that file we disable all blocks that are NOT sections, NOT the container (content only), not the block root (disabled). The sections are default. So that means, we need to manually disable the drop zone for each section because the mode is default.
It's easy to see in trunk: one can drop just at immediate 1st level of a pattern but not in nested block levels.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we disable the section blocks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can't select them if they're disabled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 if you disable a block you should still be able to select it? For what use case does it behave that way? If you want to disable selection, you should disable the whole block list imo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do disable the whole block list if there is a section root.
So the case is:
- the elements outside the section root should not be selectable
- the section root should not be selectable
- the 1st children (sections) of the section root should be selectable
- anything inside the sections should not be selectable
This is what this code accomplishes.
if you disable a block you should still be able to select it?
See #51148 how it specifically "Disable selection (using user-select: none) in disabled blocks."
There is always a way. We could check if the transfer data contains one block or multiple blocks and assume multiple blocks means a pattern. But, as @ellatrix mentioned the block editing mode set to disabled for the sections themselves should prevent drop zone indifferent of what we drag. |
So just to bubble up this conversation and for reference:
That means we still need to remove the drop zone for sections themselves since they are "whatever normal mode logic says" which is usually default, so drop works. For now I think this PR makes sense. |
e7738d8
to
440e289
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Agreed. Not a huge deal, but perhaps worth a follow-up to clean up. |
Co-authored-by: Ben Dwyer <[email protected]>
Co-authored-by: Ben Dwyer <[email protected]>
Co-authored-by: Ben Dwyer <[email protected]>
What?
When zoom out mode is engaged the dragging operations from the inserter need to only allow dropping in between whatever we consider sections (1st children of the detected section root).
Why?
To avoid a bad experience considering the scaled down visuals, and the fact that once something is created via drag and drop inside a section it won't be selectable, because only sections are selectable, not their contents.
How?
Disable the drop zone in zoom out mode for the section containers.
Testing Instructions
main
tagTesting Instructions for Keyboard
N/A
Screenshots or screencast
Before / Trunk
drop-before.mp4
After / This PR
drop-after.mp4