[Bug Fix] Overlays: play the exit animation before hiding

Merged
TV
2mo ago
+556-63

Related issue

No existing issue — happy to open one if you'd prefer to track it separately.

Description

Every overlay that ships data-[state=closed]:animate-out cut it: each controller set the closed state and applied hidden (display: none) — or called element.remove() — in the same frame, so the enter animation ran and the exit was a hard cut. Three of them never set data-state at all.

ComponentBefore
Popover, HoverCard, ContextMenuhidden in the same tick as data-state="closed"
DropdownMenu, ClipboardPopoverhidden on the wrapper; data-state never set
Selecthidden on the wrapper; exit keyed on the root's open value
Sheet, CommandDialogelement.remove() outright
Tooltipalready correct — the pattern this follows

The fix defers the hide to the end of the exit animation with one block that is byte-identical across the eight controllers, plus a one-line afterExit() per controller (hidden on the wrapper, or element.remove()):

  • data-state="closed" first, hide once the exit has finished — the classes get their frames.
  • The exit is whatever the closed state started — getAnimations() is snapshotted around the state flip and the block awaits the finished promise of exactly those animations (Promise.allSettled): a second, longer exit is waited for, an unrelated loop that keeps running is not, and a cancelled enter never enters the set. No events, so nothing bubbling up from an animated child can hide its container. Mirrors @radix-ui/react-presence.
  • Reopened mid-exit — finished rejects on cancel, the promise settles, and the element stays visible because its state is no longer closed.
  • No timeout fallback — if the closed state started nothing (no tw-animate-css, overridden classes, a consumer's reduced-motion rule, or no box to run in) it hides at once instead of waiting on something that never finishes.
  • settleExit on disconnect — every other listener in these controllers is torn down there.
  • data-[state=closed]:fill-mode-forwards on every affected content component — without it the element repaints at full opacity between the last keyframe and the hide, which flashes. TooltipContent already carries it.
  • Explicit panel target (and backdrop for Sheet/CommandDialog) for the animated element where it differs from the hidden wrapper.

Behaviour notes: DropdownMenu#toggle now reads openValue (hidden no longer distinguishes the states) and releases its z-index on settle; CommandDialog reopened while dismissing brings the same instance back rather than stacking a second one; Sheet reopened mid-exit inserts a new instance while the old one finishes, as it does today on a double click. Clipboard's empty-source branch called a method that did not exist (showErrorPopover) — fixed while restructuring it.

Kept as copies rather than a shared module: the gem ships no shared JS today, and one import would have to resolve under both importmap and bundlers — see the thread on the duplication comment.

Testing instructions

cd gem && bundle exec rake covers the rendered fill-mode-forwards class for all eight. The lifecycle is JS — cd docs && bin/dev, then for each of /docs/popover, /docs/hover_card, /docs/context_menu, /docs/dropdown_menu, /docs/clipboard, /docs/select, /docs/sheet, /docs/command:

  1. Open, then close (outside click, Esc, item, close button). It should fade/slide out, not vanish.
  2. Reopen mid-fade: it should come back without flicker or a stuck panel.
  3. Close mid-open (trigger twice in quick succession): the exit should still play in full.

I also drove all eight through those three scenarios in headless Chrome against the built docs bundle — every one settled (hidden applied / element removed / same instance reopened).

Note on reduced motion: unchanged — the library ships no prefers-reduced-motion rules today, so animate-in/animate-out run regardless. Happy to open a separate issue for library-wide support.