ui/Drawer: Polish open/close animation, fix swipe on content padding#77800
Open
ui/Drawer: Polish open/close animation, fix swipe on content padding#77800
Conversation
ciampo
changed the title
|
Size Change: 0 B Total Size: 7.82 MB ℹ️ View Unchanged
|
ciampo
changed the title
ciampo
changed the title
|
Flaky tests detected in 8737a2c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/25137388975
|
ciampo
changed the title
The popup `transform` transition was nested inside `&[data-open], &[data-swiping]`, so it only applied while the drawer was open or being swiped. Base UI removes `[data-open]` at the start of the close sequence, so non-swipe dismissals (close button, Escape, backdrop click) had nothing to interpolate and the popup snapped to its `[data-ending-style]` offset instead of sliding out. Move the transition (and its `[data-ending-style]` / `[data-swiping]` duration overrides) to the `.popup` top level — the same shape Base UI documents and our `Dialog` already uses. The `will-change` hint stays scoped to the in-motion states (now including `[data-ending-style]`), so we still avoid retaining a transform layer while the drawer is fully idle.
The removed block read like a commit message rather than a code comment. The adjacent `will-change` block already names the in-motion states (opening, swiping, closing); `Dialog` follows the same shape without commentary; and the `[data-starting-style]` / `[data-ending-style]` contract is documented Base UI API.
Paint the popup `box-shadow` only while the popup body is on screen. At `[data-starting-style]` and `[data-ending-style]` the popup is translated fully past the viewport edge, but its shadow blur still bleeds back into the viewport — most noticeable in non-modal mode, where there is no backdrop to mask it. Set `box-shadow: none` at those two states and add `box-shadow` to the existing transition. `none` interpolates against the resolved token value as a list of zero-shadow layers per the CSS Backgrounds spec, so we don't need to know how many layers `--wpds-elevation-lg` expands to.
Note that `will-change` is intentionally limited to `transform` (the only animated property the compositor can take off the main thread), and that `box-shadow: none` for the offscreen states is intentionally left outside the `prefers-reduced-motion` guard so the popup mounts / unmounts without a stray shadow when transitions are disabled. No behavior change.
Base UI's drawer carves out `[data-drawer-content]` from mouse-drag swipe-dismiss so text selection inside the body keeps working. Before this change, our `Drawer.Content` wrapper *was* the marker, so the visual popup-edge gutter (`Drawer.Content`'s padding) inherited that carve-out and could not be grabbed with the mouse on desktop — leaving only the chrome (`Drawer.Header` / `Drawer.Footer`) draggable in practice. Most of the visible drawer surface looked draggable but silently no-oped under a mouse pointerdown. Scope the marker tightly around the body children: `Drawer.Content` now renders an outer styled `<div>` (the actual scroll container, keeping its `data-wp-ui-overlay-scroll-container` attribute and the forwarded `ref`/`onScroll`/className/...) and wraps Base UI's `_Drawer.Content` as a plain inner `<div>`. The padding gutter falls outside the marker, so a pointerdown there resolves to the popup ancestor and engages swipe; pointerdowns on real body content still match `[data-drawer-content]` via `closest()` and bail out, preserving text selection. The shared `overlay-chrome.module.css` non-pinned-chrome rules switch from `>` to descendant matching to traverse the new wrapper. `Dialog` and `AlertDialog` don't render the wrapper, so the change is a no-op for direct nested chrome there; consumers who already wrap nested chrome in custom intermediate elements inside `*.Content` will now see the documented padding-collapse behavior apply (previously only matched direct children). The marker is rendered with the default `<div>` (display: block), not `display: contents`. A boxless marker would silently neuter `useOverlayScrollStateAttributes`'s `ResizeObserver` for dynamic body growth (the hook observes the scroll container's direct children, and RO does not fire entries for boxless elements). Keeping the marker as a real auto-height block lets the resize signal propagate as the hook's existing JSDoc invariant promises. Also adds the missing PR-link suffix to the prior two `Drawer` animation entries in the CHANGELOG.
The previous commit relaxed the shared `.content > .header` / `.content > .footer` rules in `overlay-chrome.module.css` to descendant matching so they would traverse `Drawer.Content`'s new outer wrapper. That relaxation was too coarse: it also fired for `Dialog` and `AlertDialog` consumers who wrap chrome in arbitrary intermediate elements inside `*.Content`, collapsing both the chrome's inline padding and (worse) its `:first-child` / `:last-child` block-padding — breaking the visual rhythm of legitimate consumer-defined sections. Tighten by pairing each direct-child rule with an explicit one-level- deeper variant through `[data-drawer-content]`. Drawer's marker is a transparent wrapper we control, so mirroring through it preserves the intent (chrome at the scroll body's true visual edge collapses its popup-edge padding); arbitrary consumer-supplied wrappers no longer match for `Dialog` / `AlertDialog`, restoring pre-PR behavior there. Updates the CHANGELOG entry to reflect the byte-identical layout for `Dialog` / `AlertDialog`.
Asserts the load-bearing invariants of the wrapper introduced two commits ago: - The forwarded ref / scroll listener / overlay-chrome class land on the visible scroll container (no `[data-drawer-content]` on the outer element). - Base UI's `[data-drawer-content]` marker is the only direct child of that scroll container — so the popup-edge padding gutter falls outside the marker and stays mouse-draggable for swipe-dismiss. - The marker renders as a real block-level element rather than `display: contents`, which would silently neuter `useOverlayScrollStateAttributes`'s `ResizeObserver` for dynamic body growth. Catches both a future Base UI release that changes the marker's default tag and a regression that re-introduces `display: contents` or removes the wrapper.
Drop references to Base UI's `_Drawer.Content` marker, the `[data-drawer-content]` selector, and the wrapper-vs-marker layering from the public JSDoc on `Drawer.Content` and `ContentProps.children`. Replace them with user-facing descriptions of the swipe-to-dismiss behavior (mouse drag preserved in the gutter and chrome but not over the body; touch swipe engages from anywhere, gated by the scroll edge on vertical drawers). The internal structure remains an implementation detail.
ciampo
force-pushed
the
fix/drawer-close-animation
branch
from
736c681 to
8737a2c
Compare
|
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. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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?
Drawerpopup polish, in three pieces:[data-drawer-content]from mouse-drag swipe so text selection inside the body keeps working; before this PR, ourDrawer.Contentwas the marker, so the visual padding gutter inherited that carve-out and could not be grabbed with the mouse. Most of the drawer surface looked draggable but silently no-oped under pointer down on desktop.Why the close animation broke
The popup
transformtransition was nested inside&[data-open], &[data-swiping]. Base UI removes[data-open]at the start of the close sequence, so on a non-swipe dismissal neither selector matched, thetransitiondeclaration disappeared, and the per-direction&[data-ending-style] { transform: translate(±100%) }rule applied with nothing to interpolate.Moving the transition to the
.popuptop level inside the existing@media not (prefers-reduced-motion)guard mirrors whatDialogalready does and what the Base UI animation handbook and drawer docs prescribe for[data-starting-style]/[data-ending-style]based transitions.will-change: transformstays scoped to the in-motion states (now including[data-ending-style]), so we still avoid retaining a transform layer while idle.The original gating was intentional (
Drawer: Scope motion layers and horizontal safe-area) — the optimization is fine, the over-scoping was the regression.Why the elevation fade
At
[data-starting-style]/[data-ending-style]the popup is translated fully past the viewport edge, but itsbox-shadowblur still bleeds back into the viewport, appearing as a faint halo at mount/unmount. Most visible inmodal={ false }andmodal="trap-focus"modes where there is no backdrop to mask it.Setting
box-shadow: noneat those two states and addingbox-shadowto the existing transition makes the shadow grow in alongside the slide.noneinterpolates against the resolved token value as a list of zero-shadow layers per the CSS Backgrounds spec, so we don't need to know how many layers--wpds-elevation-lgexpands to.The other overlay primitives (
Dialog,AlertDialog,Popover,Tooltip,Select,Autocomplete) don't need this — they either have no entry/exit animation or already animateopacitythrough 0, which fades the shadow with the rest of the popup. The drawer is the only one that intentionally keeps the popup opaque while it slides.Why scope the swipe carve-out tightly
In
DrawerViewport.tsx, Base UI'sonPointerDownhandler bails out whengetElementAtPoint(...).closest('[data-drawer-content]')matches — so mouse-drag never engages over the body, on purpose, so that text selection there keeps working.When
Drawer.Contentis[data-drawer-content], that carve-out covers the visible padding gutter too, even though the gutter is just empty space the user perceives as part of the panel chrome. Scoping the marker tightly around the body children —Drawer.Contentnow renders an outer styled<div>(the actual scroll container, withdata-wp-ui-overlay-scroll-containerand the forwardedref/onScroll/className/ props) and wraps Base UI's_Drawer.Contentinside it — leaves the padding gutter outside the carve-out. Pointer-downs there resolve to the popup ancestor and engage swipe; pointer-downs on real body content still match[data-drawer-content]viaclosest()and bail, preserving text selection.Two consequences worth noting:
>to descendant matching for the non-pinned.content > .header/.content > .footerrules inoverlay-chrome.module.css, so they traverse the new wrapper.DialogandAlertDialogdon't render the wrapper — direct nested chrome matches identically — but consumers who already wrap nested chrome in custom intermediate elements inside*.Contentwill now see the documented padding-collapse apply.<div>(block), notdisplay: contents. A boxless marker would silently neuteruseOverlayScrollStateAttributes'sResizeObserverfor dynamic body growth (the hook observes the scroll container's direct children, andResizeObserverdoes not fire for boxless elements). Keeping the marker as a real auto-height block lets the resize signal propagate as the hook's JSDoc invariant promises.Testing Instructions
In Storybook, open
Design System / Components / Drawerand try:swipeDirectionset to each ofup,down,left,right(useAll Sidesor the controls).Drawer.Content. Swipe should not engage; selection / click should behave normally.Non Modalstory is the clearest test (no backdrop). Open and close: the shadow should ramp in alongside the slide rather than appearing instantly. In modal mode (Default) the effect is subtler but should still look smooth.tabindex="0"toggles correctly when content stops or starts overflowing (both initially and as content mutates at runtime).prefers-reduced-motion: reduce) and close the drawer. It should close instantly with no transition. The shadow should also not animate.Testing Instructions for Keyboard
Screenshots or screencast
Kapture.2026-04-29.at.17.30.29.mp4
Kapture.2026-04-29.at.17.27.59.mp4
Use of AI Tools
Authored with the help of an AI assistant (Cursor); diagnosis, code changes, and PR text were reviewed by a human. Per the WordPress AI Guidelines.