Repository navigation
Fix drawer backdrop exit transition on close - #43028
Open
xiao115255 wants to merge 1 commit into
Open
xiao115255 wants to merge 1 commit into
xiao115255 wants to merge 1 commit into
Conversation
Drawer closed the <dialog> synchronously, so it left the top layer before the exit transition ran and the browser dropped its ::backdrop instantly. Dialog already defers close() until the exit transition ends and carries the styles that go with it; Drawer did neither. - Override _shouldDeferClose() in Drawer, as Dialog does. - Qualify the Drawer open-state selectors with :not(.hiding) so the exit falls back to the off-screen placement transform while [open] is still present, and add the .hiding::backdrop exit state. - Clear .hiding in DialogBase.dispose() and always call _closeAndCleanup() in the hide completion callback: the deferred close makes both paths reachable, and a stranded .hiding or a skipped cleanup would leave the element invisible or the page scroll locked. Fixes twbs#43024
This branch has not been deployed
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
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 & Why
Fixes #43024.
A closing Drawer dropped its native
::backdropinstantly.DialogBasecloses the<dialog>synchronously, and leaving the top layer is what removes::backdrop, so the backdrop never got a chance to transition.Dialogavoids this by overriding_shouldDeferClose()to keep the dialog in the top layer until the exit transition ends;Drawerdid not override it, and was also missing the CSS that goes with it.This makes Drawer mirror Dialog:
js/src/drawer.ts— override_shouldDeferClose()(same asDialog).scss/_drawer.scss— qualify the open-state selectors with:not(.hiding)so the exit falls back to the off-screen placement transform while[open]is still present, and add the.hiding::backdropexit state so the backdrop fades out alongside the panel.Two supporting fixes in
js/src/dialog-base.ts, both reachable only because the close is now deferred:dispose()clears.hiding._queueCallback()skips its completion callback once the instance is disposed (an SPA tearing the drawer down mid-navigation, or re-instantiating on the same element), which would strand the exit state. With[open]:not(.hiding)a stranded.hidingmakes the next instance's drawer invisible while<html>stays scroll locked._closeAndCleanup()unconditionally. If something else closes the dialog during the exit (a<form method="dialog">submit, app code callingdialog.close()), the previousif (element.open)guard skipped the cleanup and leftdialog-openon<html>permanently._closeAndCleanup()is already documented as safe on an already-closed dialog.Note that Drawer now matches Dialog's exit behavior exactly, including that the dialog stays modal (and keeps swallowing pointer input) until the exit transition finishes.
How Tested
npm run js-test-unit— 1355 tests pass across 33 files, coverage above the configured thresholds.npm run js-typecheck,npm run js-lint,npm run css-lint-stylelint,npm run css-test(47 specs),npm run dist— all pass.[open]and.hidingwhile the exit runs (fails without the_shouldDeferClose()override); instant drawers still close in the same task;dispose()mid-transition clears.hiding(fails without the base-class fix).dist/build, using the reproducer from the issue with--drawer-backdrop-bgoverridden to make the backdrop visible, samplinggetComputedStyle(dialog, '::backdrop')on the close click:openand:modalbothfalse2 ms after the click, one distinct backdrop sample, no::backdroptransition events;[open]and:modalhold for the whole exit,background-colorandbackdrop-filterfiretransitionrun/transitionendon::backdrop, the alpha steps 0.25 → 0.14 → 0.07 → … → ~0 (10 distinct samples), andclose()runs at ~298 ms.AI Disclosure
This PR was prepared with AI assistance (Claude Code), reviewed and submitted by @xiao115255.