-
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
Block editor: iframe: remove event bubbling and improve draggable chip #34819
base: trunk
Are you sure you want to change the base?
Conversation
Size Change: +365 B (0%) Total Size: 1.06 MB
ℹ️ View Unchanged
|
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 like the idea here! However I did spot in manual testing that we're missing the drag chip in list view:
CleanShot.2021-09-28.at.15.49.45.mp4
@@ -51,6 +51,7 @@ $z-layers: ( | |||
|
|||
// The draggable element should show up above the entire UI | |||
".components-draggable__clone": 1000000000, | |||
".block-editor-block-draggable-chip-wrapper": 1000000000, |
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.
It might be worth using a decimal literal for readability 1_000_000_000
Not sure how important this is, but it looks like the parent z-index stacking context for the chip wrapper is .interface-interface-skeleton__content
. This means that despite the high z-index here items in sidebar and actions will be on top of the chip from natural stacking order.
CleanShot.2021-09-28.at.15.44.25.mp4
@gwwar Judging by that video, it looks like the chips aren't missing from List View, they're just rendering behind it. |
Good call @ZebulanStanphill that was it: CleanShot.2021-09-30.at.09.49.04.mp4One way of working around this is appending the chip wrapper as a child of body, as we probably want to keep the existing ordering for content vs sidebars and actions. |
Description
dragover
event on the iframe.BlockTools
next to the block toolbar (these components are anyway related).__experimentalDragComponent
prop can now be removed.Thoughts
While the change requires document level event listeners to be added, we could potentially avoid this with purely React events if we handle it higher up the tree, though document level event handlers might still be required to handle drag end. I thought the block tools would be a more appropriate place for rendering the drag chip.
How has this been tested?
Screenshots
Types of changes
Checklist:
*.native.js
files for terms that need renaming or removal).