[components] Draggable: Migrate clone wrapper to wp compat overlay slot#78183
Conversation
|
Size Change: +591 B (+0.01%) Total Size: 7.97 MB 📦 View Changed
ℹ️ View Unchanged
|
|
Flaky tests detected in 9d9fe4b. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/25865528178
|
195b35f to
d5db842
Compare
98e4910 to
81353ca
Compare
81353ca to
72360a4
Compare
72360a4 to
0a25743
Compare
mirka
left a comment
There was a problem hiding this comment.
Did you notice that the default story on the docs page was kind of broken to begin with? It's fixed on this branch 😄
I haven't done any testing of actual in-app behavior yet, I'll do it in the second round 🙏
cfd150a to
12a138f
Compare
It was fixed thanks to After some investigation, my hypothesis is that the story was not working as expected because some styles in the Storybook docs page (I believe a Marking the story as "framed" (ie. not inlined, via |
Lock in the structural guarantee that underpins the stacking claim in #78183: when `@wordpress/components`'s `Draggable` runs in a WordPress environment, the drag chip is rendered inside the body-level `[data-wp-compat-overlay-slot]`. That single structural assertion subsumes the visual stacking contract — the slot creates an isolated stacking context with `z-index: 1000000003`, so anything appended into it stacks above any `@wordpress/components` overlay opened mid-drag (which live outside the slot at lower `z-index`s). Asserting structure rather than visual layering keeps the test robust against unrelated overlay z-index churn, and avoids a brittle `elementFromPoint`-style probe across the parent-doc/canvas-iframe boundary.
|
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. |
…ries The `source-link` Storybook addon already derives the GitHub source path from `storyData.importPath` when no explicit `parameters.sourceLink` is provided (see `storybook/addons/source-link/manager.ts`). For stories living under `storybook/stories/playground/`, that fallback resolves to the same value the explicit `sourceLink` was hard-coding, so the declaration is pure duplication. Per mirka's review on PR #78183 (empty-suggestion blocks covering the `parameters: { sourceLink: ... }` literal).
a390bf3 to
32ea5f8
Compare
Move the (already small) Draggable stylesheet to a CSS module so its rules travel via `@wordpress/style-runtime` (and therefore into any iframe wrapped in `<StyleProvider>` — e.g. the block-editor canvas) without needing the package-level `build-style/style.css` bundle. Drops the `@use` line from `packages/components/src/style.scss`, following the same shape as the `AlignmentMatrixControl` (#73714/#73757) and `AnglePickerControl` (#73786) migrations. The CSS-module class names are standard (hashed). The legacy `components-draggable__*` / `is-dragging-components-draggable` class names are kept by adding them alongside the hashed ones in the JS `classList.add(...)` calls, since several other Gutenberg packages reference them in their own stylesheets (block-editor's `list-view`, `block-tools`, `block-library`'s `navigation` editor, `edit-widgets`' `widget-area` editor) and block-editor runtime JS reads `is-dragging-components-draggable` off `document.body`. Dropping those names would silently break those consumers. Per mirka's review on PR #78183 (CSS-module option for the iframe story); the corresponding Storybook simplification follows in a separate commit.
Now that Draggable's styles ship as a CSS module routed through
`@wordpress/style-runtime`, the cross-document fallback story no
longer needs to manually `?inline`-import and inject the whole
`components-ltr` SCSS bundle into the iframe's `<head>`.
Wrap the portaled iframe content in
`<StyleProvider document={iframeDoc}>` from
`@wordpress/components` instead — `StyleProvider` calls
`registerDocument()` on the iframe document, and the style
registry replays every registered CSS module (Draggable
included) into that document. The visible behavior is
unchanged: the orange clone still tracks the cursor inside the
iframe, demonstrating the cross-document fallback.
Per mirka's review on PR #78183.
32ea5f8 to
300cbb7
Compare
Move the (already small) Draggable stylesheet to a CSS module so its rules travel via `@wordpress/style-runtime` (and therefore into any iframe wrapped in `<StyleProvider>` — e.g. the block-editor canvas) without needing the package-level `build-style/style.css` bundle. Drops the `@use` line from `packages/components/src/style.scss`, following the same shape as the `AlignmentMatrixControl` (#73714/#73757) and `AnglePickerControl` (#73786) migrations. The CSS-module class names are standard (hashed). The legacy `components-draggable__*` / `is-dragging-components-draggable` class names are kept by adding them alongside the hashed ones in the JS `classList.add(...)` calls, since several other Gutenberg packages reference them in their own stylesheets (block-editor's `list-view`, `block-tools`, `block-library`'s `navigation` editor, `edit-widgets`' `widget-area` editor) and block-editor runtime JS reads `is-dragging-components-draggable` off `document.body`. Dropping those names would silently break those consumers. Per mirka's review on PR #78183 (CSS-module option for the iframe story); the corresponding Storybook simplification follows in a separate commit.
Now that Draggable's styles ship as a CSS module routed through
`@wordpress/style-runtime`, the cross-document fallback story no
longer needs to manually `?inline`-import and inject the whole
`components-ltr` SCSS bundle into the iframe's `<head>`.
Wrap the portaled iframe content in
`<StyleProvider document={iframeDoc}>` from
`@wordpress/components` instead — `StyleProvider` calls
`registerDocument()` on the iframe document, and the style
registry replays every registered CSS module (Draggable
included) into that document. The visible behavior is
unchanged: the orange clone still tracks the cursor inside the
iframe, demonstrating the cross-document fallback.
Per mirka's review on PR #78183.
300cbb7 to
e12ba9f
Compare
Now that Draggable's styles ship as a CSS module routed through
`@wordpress/style-runtime`, the cross-document fallback story no
longer needs to manually `?inline`-import and inject the whole
`components-ltr` SCSS bundle into the iframe's `<head>`.
Wrap the portaled iframe content in
`<StyleProvider document={iframeDoc}>` from
`@wordpress/components` instead — `StyleProvider` calls
`registerDocument()` on the iframe document, and the style
registry replays every registered CSS module (Draggable
included) into that document. The visible behavior is
unchanged: the orange clone still tracks the cursor inside the
iframe, demonstrating the cross-document fallback.
Per mirka's review on PR #78183.
e12ba9f to
5931ca7
Compare
Move the (already small) Draggable stylesheet to a CSS module so its rules travel via `@wordpress/style-runtime` (and therefore into any iframe wrapped in `<StyleProvider>` — e.g. the block-editor canvas) without needing the package-level `build-style/style.css` bundle. Drops the `@use` line from `packages/components/src/style.scss`, following the same shape as the `AlignmentMatrixControl` (#73714/#73757) and `AnglePickerControl` (#73786) migrations. The CSS-module class names are standard (hashed). The legacy `components-draggable__*` / `is-dragging-components-draggable` class names are kept by adding them alongside the hashed ones in the JS `classList.add(...)` calls, since several other Gutenberg packages reference them in their own stylesheets (block-editor's `list-view`, `block-tools`, `block-library`'s `navigation` editor, `edit-widgets`' `widget-area` editor) and block-editor runtime JS reads `is-dragging-components-draggable` off `document.body`. Dropping those names would silently break those consumers. Per mirka's review on PR #78183 (CSS-module option for the iframe story); the corresponding Storybook simplification follows in a separate commit.
Move the (already small) Draggable stylesheet to a CSS module so its rules travel via `@wordpress/style-runtime` (and therefore into any iframe wrapped in `<StyleProvider>` — e.g. the block-editor canvas) without needing the package-level `build-style/style.css` bundle. Drops the `@use` line from `packages/components/src/style.scss`, following the same shape as the `AlignmentMatrixControl` (#73714/#73757) and `AnglePickerControl` (#73786) migrations. The CSS-module class names are standard (hashed). The legacy `components-draggable__*` / `is-dragging-components-draggable` class names are kept by adding them alongside the hashed ones in the JS `classList.add(...)` calls, since several other Gutenberg packages reference them in their own stylesheets (block-editor's `list-view`, `block-tools`, `block-library`'s `navigation` editor, `edit-widgets`' `widget-area` editor) and block-editor runtime JS reads `is-dragging-components-draggable` off `document.body`. Dropping those names would silently break those consumers. Per mirka's review on PR #78183 (CSS-module option for the iframe story); the corresponding Storybook simplification follows in a separate commit.
Now that Draggable's styles ship as a CSS module routed through
`@wordpress/style-runtime`, the cross-document fallback story no
longer needs to manually `?inline`-import and inject the whole
`components-ltr` SCSS bundle into the iframe's `<head>`.
Wrap the portaled iframe content in
`<StyleProvider document={iframeDoc}>` from
`@wordpress/components` instead — `StyleProvider` calls
`registerDocument()` on the iframe document, and the style
registry replays every registered CSS module (Draggable
included) into that document. The visible behavior is
unchanged: the orange clone still tracks the cursor inside the
iframe, demonstrating the cross-document fallback.
Per mirka's review on PR #78183.
The Enhancements entry for this PR ended up rolled into the already-cut `33.1.0` release section during an earlier rebase, and had grown to a 700-character paragraph spelling out every edge case (cross-document fallback, `appendToOwnerDocument` semantics, in-flow ancestor migration hints). Move it back to `## Unreleased` and trim to a two-sentence summary in line with the surrounding entries. The dropped detail still lives in the JSDoc, the code comments, and the PR description's <details> blocks.
Sweep across the comments added by this PR, dropping
redundant duplication, narration of self-evident code, and
prose that already lives in the PR description / JSDoc:
- Drop the duplicate compat-slot note from the
`AppendElementToOwnerDocument` story JSDoc (the
interaction is already described on the prop's TS
JSDoc in `types.ts`).
- Tighten the prop JSDoc for `appendToOwnerDocument` to a
single short paragraph.
- Slim the same-document-only slot guard comment in
`Draggable.start()` (the conditional itself reads as
"slot if same document").
- Compact the rationale comment for
`parameters.docs.story.inline: false` in the Draggable
autodocs config to a single explanation.
- Trim the structural-stacking assertion comment in the
Playwright `draggable-blocks` spec.
- Drop the forward-looking "can be removed on a future
Stylelint upgrade" note from `CSS_BASELINE_2024_FUNCTIONS`.
No behavior change.
The `getWpCompatOverlaySlot()` export bullet was left inside the already-released `## 0.13.0` section when the parallel `@wordpress/components` entry was moved to `## Unreleased`.
Mirroring this offscreen stand-in in RTL has no benefit — either side hides it equally — so revert to the original physical property and silence the logical-properties lint with a targeted comment.
The cursor flip is also triggered by external code (block-editor keyboard drag, etc.) that toggles `is-dragging-components-draggable` directly. Targeting the legacy class globally preserves that flow, which a module-hashed selector silently broke.
`jest-preset-default`'s style mock returns `undefined` for any class, which `classList.add()` would coerce to a literal "undefined" token. Filter falsy entries to keep test DOM clean.
- Note why the invisible drag image bypasses the compat slot. - Drop a redundant chip-count assertion in the e2e spec. - Flag the SCSS-only stylelint override pattern explicitly.
Consolidates the manual verification stories (`WP Compat Overlay Slot`, `Popover with SlotFill`) alongside the existing Draggable fixture.
@mirka I'm not sure we can, because any usage of the component that doesn't use the wp compat overlay slot may still rely on that
|
Ok, then I think that means we should only set the z-index when it's not in the compat slot? Otherwise we'll have an explicit z-index element in the compat slot. |
Good point — addressing in #78354 |
What?
Migrate
.components-draggable__cloneplacement onto the@wordpress/uicompat overlay slot (#77851). When the slot is available in the same document as the dragged element, the drag clone joins the slot's body-level stacking context.Alternative to #78177 — once this lands, the z-index bump in #78177 isn't needed and that PR can be closed.
Why?
Once
@wordpress/uioverlays start defaulting to the slot (#78095), the clone needs to share stacking with them. Migrating consolidates the legacyz-index: 1000000000under the slot's stacking context (slot atz-index: 1000000003) instead of bumping the clone's own value one rung higher, and sets the pattern for any further@wordpress/components→ slot migrations (Popover, Modal, Tooltip, …). The clone's ownz-index: 1000000000is kept as a defensive fallback for the two paths that still skip the slot: hosts that never opt in (nowindow.wp.components, nouseEnableWpCompatOverlaySlot()call), and cross-document drags where the slot lives in a different document than the dragged element.How?
DraggableimportsgetWpCompatOverlaySlotfrom@wordpress/uiand routes the clone into the slot when present. In WordPress environments the slot auto-enables on first call (viawindow.wp.components), so the clone reliably lands inside the slot regardless of which@wordpress/uioverlay rendered first.The slot is only used when its owner document matches the dragged element's owner document. Cross-document drags (e.g. drag inside an iframe while the slot lives in the parent) fall back to the legacy placement so the clone's viewport-relative geometry stays in one coordinate space.
Behavior change: default placement and
appendToOwnerDocumentsemanticsWhen the slot is in effect (same-document case), all three
Draggableplacement modes route the clone into the slot:__experimentalDragComponent(was:ownerDocument.body) → slotappendToOwnerDocument: true(was:ownerDocument.body) → slotappendToOwnerDocument: false(was: dragged element's parent) → slotThe
appendToOwnerDocumentprop therefore has no effect in the same-document slot case — the clone always lives in the slot, regardless of the prop. Its JSDoc has been updated to document this and to call out the cross-document fallback. Hosts that don't opt into the slot, or whose drags cross document boundaries, continue to see the prop's original semantics.The default-mode shift (clone leaving the dragged element's parent) is the only one that changes the clone's stacking ancestor. External consumers that relied on that in-flow ancestry (e.g. for scoped CSS or event delegation) must either keep the slot disabled or migrate their scoping onto the slot itself. See the in-tree audit below for the Gutenberg side.
In-tree audit: no Gutenberg-app refactor needed
Every potential coupling to the clone's previous DOM position is unaffected:
packages/components/src/draggable/style.scssand the entry inpackages/base-styles/_z-index.scss. Both are top-level class selectors with no parent dependency.cloneClassnameconsumers in-tree.list-view/block-contents.jspassesblock-editor-list-view-draggable-chip, styled only as.block-editor-list-view-draggable-chip { opacity: 0.8; }(top-level).BlockMoverandInserterDraggableBlocksdon't pass a clone class.is-dragging-components-draggableselectors. All occurrences (innavigation,block-tools,list-view,edit-widgets, draggable's ownstyle.scss) scope off the body class, which still applies identically. They target the original DOM (dragged block, toolbars, etc.), never the clone.pointer-events: none, so it never receives pointer events. HTML5 drag events fire on the dragged element / drop targets, never on the clone — so no parent listener was relying on bubbling through it.querySelector/closest()anywhere in the repo targets.components-draggable__clone.Draggable/BlockDraggableconsumers.block-editor/block-mover(default placement → previously elementWrapper, now slot; visual identical:position: fixed,pointer-events: none),block-editor/list-view/block-contents(appendToOwnerDocument: true→ previously body, now slot; stacking-equivalent),block-editor/inserter-draggable-blocks(__experimentalDragComponent→ previously body, now slot; stacking-equivalent).test/e2e/specs/editor/various/draggable-blocks.spec.js,inserting-blocks.spec.js) locate it by globaldata-testid=block-draggable-chip >> visible=true, not anchored to a parent.Styles migrated from SCSS to a CSS module
Draggable's small stylesheet now ships as a CSS module so its rules travel via@wordpress/style-runtime(rather than only through the package-levelbuild-style/style.css). For any document wrapped in<StyleProvider>from@wordpress/components— including the block-editor canvas iframe — the registry replays the styles automatically. This follows the same pattern as the existingAlignmentMatrixControl(#73714/#73757) andAnglePickerControl(#73786) migrations.The CSS-module class names are standard (hashed); the legacy
components-draggable__*/is-dragging-components-draggablenames are kept alongside the hashed ones in the JSclassList.add(...)calls. Several other packages still rely on them — block-editor'slist-view,block-tools,block-library'snavigationeditor,edit-widgets'widget-area, plus a block-editor runtime check that readsis-dragging-components-draggabledirectly offdocument.body. Dropping the legacy names would silently break those consumers.As a side benefit, the Storybook cross-document fallback story can now wrap its iframe portal in
<StyleProvider document={iframeDoc}>and drop the previous manual style injection.Testing Instructions
This PR has no new unit tests; the migration relies on the slot helper's existing test coverage (24 tests in
packages/ui/src/utils/test/wp-compat-overlay-slot.test.ts), a new Storybook playground story that exercises the cross-document fallback path (Playground / Debug fixtures / Draggable cross-document fallback), and a new Playwright regression intest/e2e/specs/editor/various/draggable-blocks.spec.jsthat asserts the drag chip is rendered inside the body-level[data-wp-compat-overlay-slot](which transitively guarantees the stacking contract — the slot creates an isolated stacking context atz-index: 1000000003).Manual smoke:
BlockPopover(@wordpress/componentstooltips). Clone stays above the tooltip — now backed by the slot's isolated stacking context (z-index: 1000000003withisolation: isolate) rather than the clone's ownz-index: 1000000000, which now only matters in the fallback paths.@wordpress/uiTooltip(per [ui] Tooltip: Default portal container to the wp compat overlay slot #78095) over the canvas mid-drag. Tooltip and clone share the slot's stacking context, so neither is hidden behind unrelated@wordpress/componentsoverlays. The clone keeps its legacyz-index: 1000000000inside the slot, so it also stays visible above any mid-drag tooltip — preserving the pre-migration UX of the dragged item being the active manipulation target.Draggableautodocs page, bothDefaultandAppend element to owner documentshow the clone tracking the cursor as you drag. Each story renders in its own iframe on the docs page (viaparameters.docs.story.inline: false) so the clone'sposition: fixedresolves against the viewport instead of docs-page wrappers (which applytransforms and would otherwise pin the clone in the wrong place).Playground / Debug fixtures / Draggable cross-document fallback. Drag the blue handle inside the iframe; the orange clone tracks the cursor inside the iframe (no offset toward the Storybook canvas top-left), confirming the cross-document fallback. The story lives understorybook/stories/playground/(not under theDraggablecomponent) because the slot opt-in is a window-level flag and would otherwise leak into sibling Draggable stories on the same autodocs page.Testing instructions for keyboard
Drag-and-drop is pointer-only in the affected code path; no keyboard testing applies. The hosting components (
BlockDraggableetc.) provide their own keyboard alternatives.Use of AI Tools
Authored with Cursor (Claude). Author reviewed all changes.