From 0e8145fe379a6f73292e85ca4b157a11c3b46d0d Mon Sep 17 00:00:00 2001 From: Tomek Date: Thu, 13 Aug 2026 00:27:50 +0200 Subject: [PATCH 1/3] [Bug Fix] Popover, HoverCard, ContextMenu: play the exit animation before hiding All three set data-state="closed" and add `hidden` (display: none) in the same frame, so data-[state=closed]:animate-out never gets one. Defer `hidden` to animationend/animationcancel, and add fill-mode-forwards so the last frame holds until it lands. Co-Authored-By: Claude Opus 5 (1M context) --- .../context_menu/context_menu_content.rb | 2 +- .../context_menu/context_menu_controller.js | 32 ++++++++++++++++++- .../ruby_ui/hover_card/hover_card_content.rb | 2 +- .../hover_card/hover_card_controller.js | 32 ++++++++++++++++++- gem/lib/ruby_ui/popover/popover_content.rb | 1 + gem/lib/ruby_ui/popover/popover_controller.js | 32 ++++++++++++++++++- gem/test/ruby_ui/context_menu_test.rb | 11 +++++++ gem/test/ruby_ui/hover_card_test.rb | 9 ++++++ gem/test/ruby_ui/popover_test.rb | 9 ++++++ 9 files changed, 125 insertions(+), 5 deletions(-) diff --git a/gem/lib/ruby_ui/context_menu/context_menu_content.rb b/gem/lib/ruby_ui/context_menu/context_menu_content.rb index 596d0f387..d83af4d65 100644 --- a/gem/lib/ruby_ui/context_menu/context_menu_content.rb +++ b/gem/lib/ruby_ui/context_menu/context_menu_content.rb @@ -15,7 +15,7 @@ def default_attrs data_state: "closed", data: {ruby_ui__context_menu_target: "content"}, class: - "hidden absolute z-50 min-w-[8rem] outline-none pointer-events-auto overflow-hidden rounded-md border bg-background p-1 text-foreground shadow-md data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2", + "hidden absolute z-50 min-w-[8rem] outline-none pointer-events-auto overflow-hidden rounded-md border bg-background p-1 text-foreground shadow-md data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:fill-mode-forwards data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2", tabindex: "-1", data_orientation: "vertical" } diff --git a/gem/lib/ruby_ui/context_menu/context_menu_controller.js b/gem/lib/ruby_ui/context_menu/context_menu_controller.js index ebc1f20a4..e605f2441 100644 --- a/gem/lib/ruby_ui/context_menu/context_menu_controller.js +++ b/gem/lib/ruby_ui/context_menu/context_menu_controller.js @@ -49,8 +49,8 @@ export default class extends Controller { hide() { if (!this.openValue) return; this.openValue = false; - this.contentTarget.classList.add("hidden"); this.contentTarget.dataset.state = "closed"; + this.hideAfterExitAnimation(); this.removeEventListeners(); this.deselectAll(); if (this.cleanup) { @@ -59,6 +59,36 @@ export default class extends Controller { } } + hideAfterExitAnimation() { + const content = this.contentTarget; + const styles = getComputedStyle(content); + + // An element with no exit animation never fires animationend. + if (styles.animationName === "none" || styles.display === "none") { + this.hideUnlessReopened(content); + return; + } + + content.addEventListener("animationend", this.handleExitAnimationEnd); + content.addEventListener("animationcancel", this.handleExitAnimationEnd); + } + + handleExitAnimationEnd = (event) => { + // animationend bubbles — an animated child must not hide its container. + if (event.target !== event.currentTarget) return; + + const content = event.currentTarget; + content.removeEventListener("animationend", this.handleExitAnimationEnd); + content.removeEventListener("animationcancel", this.handleExitAnimationEnd); + this.hideUnlessReopened(content); + }; + + hideUnlessReopened(content) { + if (content.dataset.state !== "closed") return; + + content.classList.add("hidden"); + } + updatePosition() { if (this.cleanup) this.cleanup(); diff --git a/gem/lib/ruby_ui/hover_card/hover_card_content.rb b/gem/lib/ruby_ui/hover_card/hover_card_content.rb index 1a5f2b3e0..11576072b 100644 --- a/gem/lib/ruby_ui/hover_card/hover_card_content.rb +++ b/gem/lib/ruby_ui/hover_card/hover_card_content.rb @@ -14,7 +14,7 @@ def default_attrs ruby_ui__hover_card_target: "content", state: :closed }, - class: "hidden absolute z-50 rounded-md border bg-background p-4 text-foreground shadow-md outline-none data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2" + class: "hidden absolute z-50 rounded-md border bg-background p-4 text-foreground shadow-md outline-none data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:fill-mode-forwards data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2" } end end diff --git a/gem/lib/ruby_ui/hover_card/hover_card_controller.js b/gem/lib/ruby_ui/hover_card/hover_card_controller.js index 955117f08..23593c1fc 100644 --- a/gem/lib/ruby_ui/hover_card/hover_card_controller.js +++ b/gem/lib/ruby_ui/hover_card/hover_card_controller.js @@ -95,8 +95,8 @@ export default class extends Controller { hide() { this.openValue = false; - this.contentTarget.classList.add("hidden"); this.contentTarget.dataset.state = "closed"; + this.hideAfterExitAnimation(); document.removeEventListener("keydown", this.boundHandleKeydown); this.deselectAll(); if (this.cleanup) { @@ -105,6 +105,36 @@ export default class extends Controller { } } + hideAfterExitAnimation() { + const content = this.contentTarget; + const styles = getComputedStyle(content); + + // An element with no exit animation never fires animationend. + if (styles.animationName === "none" || styles.display === "none") { + this.hideUnlessReopened(content); + return; + } + + content.addEventListener("animationend", this.handleExitAnimationEnd); + content.addEventListener("animationcancel", this.handleExitAnimationEnd); + } + + handleExitAnimationEnd = (event) => { + // animationend bubbles — an animated child must not hide its container. + if (event.target !== event.currentTarget) return; + + const content = event.currentTarget; + content.removeEventListener("animationend", this.handleExitAnimationEnd); + content.removeEventListener("animationcancel", this.handleExitAnimationEnd); + this.hideUnlessReopened(content); + }; + + hideUnlessReopened(content) { + if (content.dataset.state !== "closed") return; + + content.classList.add("hidden"); + } + updatePosition() { if (this.cleanup) this.cleanup(); diff --git a/gem/lib/ruby_ui/popover/popover_content.rb b/gem/lib/ruby_ui/popover/popover_content.rb index 1730dd9bb..440774049 100644 --- a/gem/lib/ruby_ui/popover/popover_content.rb +++ b/gem/lib/ruby_ui/popover/popover_content.rb @@ -18,6 +18,7 @@ def default_attrs "hidden z-50 rounded-md border bg-background p-1 text-foreground shadow-md outline-none", "data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0", "data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95", + "data-[state=closed]:fill-mode-forwards", "data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2", "data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2", "absolute" diff --git a/gem/lib/ruby_ui/popover/popover_controller.js b/gem/lib/ruby_ui/popover/popover_controller.js index 90e6f062f..ac57c00b1 100644 --- a/gem/lib/ruby_ui/popover/popover_controller.js +++ b/gem/lib/ruby_ui/popover/popover_controller.js @@ -112,8 +112,38 @@ export default class extends Controller { if (!this.hasContentTarget) return; - this.contentTarget.classList.add("hidden"); this.contentTarget.dataset.state = "closed"; + this.hideAfterExitAnimation(); + } + + hideAfterExitAnimation() { + const content = this.contentTarget; + const styles = getComputedStyle(content); + + // An element with no exit animation never fires animationend. + if (styles.animationName === "none" || styles.display === "none") { + this.hideUnlessReopened(content); + return; + } + + content.addEventListener("animationend", this.handleExitAnimationEnd); + content.addEventListener("animationcancel", this.handleExitAnimationEnd); + } + + handleExitAnimationEnd = (event) => { + // animationend bubbles — an animated child must not hide its container. + if (event.target !== event.currentTarget) return; + + const content = event.currentTarget; + content.removeEventListener("animationend", this.handleExitAnimationEnd); + content.removeEventListener("animationcancel", this.handleExitAnimationEnd); + this.hideUnlessReopened(content); + }; + + hideUnlessReopened(content) { + if (content.dataset.state !== "closed") return; + + content.classList.add("hidden"); } updatePosition() { diff --git a/gem/test/ruby_ui/context_menu_test.rb b/gem/test/ruby_ui/context_menu_test.rb index 6cdefa3aa..9dcb30705 100644 --- a/gem/test/ruby_ui/context_menu_test.rb +++ b/gem/test/ruby_ui/context_menu_test.rb @@ -41,4 +41,15 @@ def test_content_renders_hidden_positioned_div_not_template assert_match(/absolute/, output) assert_match(/Back/, output) end + + # `hidden` lands a frame after the animation ends; without a forwards fill mode that frame flashes. + def test_content_holds_the_last_frame_of_the_exit_animation + output = phlex do + RubyUI.ContextMenuContent do + RubyUI.ContextMenuItem(href: "#") { "Back" } + end + end + + assert_match(/data-\[state=closed\]:fill-mode-forwards/, output) + end end diff --git a/gem/test/ruby_ui/hover_card_test.rb b/gem/test/ruby_ui/hover_card_test.rb index 0d0ebd2c3..5f45cfe6f 100644 --- a/gem/test/ruby_ui/hover_card_test.rb +++ b/gem/test/ruby_ui/hover_card_test.rb @@ -35,4 +35,13 @@ def test_content_renders_hidden_positioned_div_not_template assert_match(/absolute/, output) assert_match(/card body/, output) end + + # `hidden` lands a frame after the animation ends; without a forwards fill mode that frame flashes. + def test_content_holds_the_last_frame_of_the_exit_animation + output = phlex do + RubyUI.HoverCardContent { "card body" } + end + + assert_match(/data-\[state=closed\]:fill-mode-forwards/, output) + end end diff --git a/gem/test/ruby_ui/popover_test.rb b/gem/test/ruby_ui/popover_test.rb index a4039c96c..6f935cef4 100644 --- a/gem/test/ruby_ui/popover_test.rb +++ b/gem/test/ruby_ui/popover_test.rb @@ -38,4 +38,13 @@ def test_content_renders_closed_state_by_default assert_match(/absolute/, output) assert_match(/popover body/, output) end + + # `hidden` lands a frame after the animation ends; without a forwards fill mode that frame flashes. + def test_content_holds_the_last_frame_of_the_exit_animation + output = phlex do + RubyUI.PopoverContent { "popover body" } + end + + assert_match(/data-\[state=closed\]:fill-mode-forwards/, output) + end end From 204845ecba4094ae4bcced88b337953beb572437 Mon Sep 17 00:00:00 2001 From: Tomek Date: Thu, 13 Aug 2026 00:55:38 +0200 Subject: [PATCH 2/3] Only settle on the exit animation, and settle on disconnect Closing while the opening animation runs cancels `enter`, and that animationcancel reached the handler as if the exit had finished. Capture the exit animation-name when arming and ignore events from any other run. disconnect() left the handlers attached, unlike every other listener in these controllers; ContextMenu's disconnect armed them on an element it was about to drop. It now applies the pending hide instead. Co-Authored-By: Claude Opus 5 (1M context) --- .../context_menu/context_menu_controller.js | 17 +++++++++++------ .../ruby_ui/hover_card/hover_card_controller.js | 17 +++++++++++------ gem/lib/ruby_ui/popover/popover_controller.js | 17 +++++++++++------ 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/gem/lib/ruby_ui/context_menu/context_menu_controller.js b/gem/lib/ruby_ui/context_menu/context_menu_controller.js index e605f2441..8c3a2d88b 100644 --- a/gem/lib/ruby_ui/context_menu/context_menu_controller.js +++ b/gem/lib/ruby_ui/context_menu/context_menu_controller.js @@ -24,6 +24,8 @@ export default class extends Controller { disconnect() { this.hide(); + // Nothing is left to wait for the exit animation, so apply the pending hide now. + if (this.hasContentTarget) this.settleExit(this.contentTarget); } handleContextMenu(event) { @@ -65,10 +67,11 @@ export default class extends Controller { // An element with no exit animation never fires animationend. if (styles.animationName === "none" || styles.display === "none") { - this.hideUnlessReopened(content); + this.settleExit(content); return; } + this.exitAnimationName = styles.animationName; content.addEventListener("animationend", this.handleExitAnimationEnd); content.addEventListener("animationcancel", this.handleExitAnimationEnd); } @@ -76,14 +79,16 @@ export default class extends Controller { handleExitAnimationEnd = (event) => { // animationend bubbles — an animated child must not hide its container. if (event.target !== event.currentTarget) return; + // Closing mid-open cancels the enter animation; only the exit run settles this. + if (event.animationName !== this.exitAnimationName) return; - const content = event.currentTarget; - content.removeEventListener("animationend", this.handleExitAnimationEnd); - content.removeEventListener("animationcancel", this.handleExitAnimationEnd); - this.hideUnlessReopened(content); + this.settleExit(event.currentTarget); }; - hideUnlessReopened(content) { + settleExit(content) { + content.removeEventListener("animationend", this.handleExitAnimationEnd); + content.removeEventListener("animationcancel", this.handleExitAnimationEnd); + // Reopened mid-exit: it is on its way back in, leave it visible. if (content.dataset.state !== "closed") return; content.classList.add("hidden"); diff --git a/gem/lib/ruby_ui/hover_card/hover_card_controller.js b/gem/lib/ruby_ui/hover_card/hover_card_controller.js index 23593c1fc..7408b2cf3 100644 --- a/gem/lib/ruby_ui/hover_card/hover_card_controller.js +++ b/gem/lib/ruby_ui/hover_card/hover_card_controller.js @@ -35,6 +35,8 @@ export default class extends Controller { this.cleanup(); this.cleanup = null; } + // Nothing is left to wait for the exit animation, so apply the pending hide now. + if (this.hasContentTarget) this.settleExit(this.contentTarget); } // Supports the tippy-style `delay` option: a number or a [open, close] tuple. @@ -111,10 +113,11 @@ export default class extends Controller { // An element with no exit animation never fires animationend. if (styles.animationName === "none" || styles.display === "none") { - this.hideUnlessReopened(content); + this.settleExit(content); return; } + this.exitAnimationName = styles.animationName; content.addEventListener("animationend", this.handleExitAnimationEnd); content.addEventListener("animationcancel", this.handleExitAnimationEnd); } @@ -122,14 +125,16 @@ export default class extends Controller { handleExitAnimationEnd = (event) => { // animationend bubbles — an animated child must not hide its container. if (event.target !== event.currentTarget) return; + // Closing mid-open cancels the enter animation; only the exit run settles this. + if (event.animationName !== this.exitAnimationName) return; - const content = event.currentTarget; - content.removeEventListener("animationend", this.handleExitAnimationEnd); - content.removeEventListener("animationcancel", this.handleExitAnimationEnd); - this.hideUnlessReopened(content); + this.settleExit(event.currentTarget); }; - hideUnlessReopened(content) { + settleExit(content) { + content.removeEventListener("animationend", this.handleExitAnimationEnd); + content.removeEventListener("animationcancel", this.handleExitAnimationEnd); + // Reopened mid-exit: it is on its way back in, leave it visible. if (content.dataset.state !== "closed") return; content.classList.add("hidden"); diff --git a/gem/lib/ruby_ui/popover/popover_controller.js b/gem/lib/ruby_ui/popover/popover_controller.js index ac57c00b1..eb9ce16a7 100644 --- a/gem/lib/ruby_ui/popover/popover_controller.js +++ b/gem/lib/ruby_ui/popover/popover_controller.js @@ -33,6 +33,8 @@ export default class extends Controller { document.removeEventListener("click", this.handleOutsideClick); this.stopAutoUpdate(); this.removeElementEventListeners(); + // Nothing is left to wait for the exit animation, so apply the pending hide now. + if (this.hasContentTarget) this.settleExit(this.contentTarget); } addEventListeners() { @@ -122,10 +124,11 @@ export default class extends Controller { // An element with no exit animation never fires animationend. if (styles.animationName === "none" || styles.display === "none") { - this.hideUnlessReopened(content); + this.settleExit(content); return; } + this.exitAnimationName = styles.animationName; content.addEventListener("animationend", this.handleExitAnimationEnd); content.addEventListener("animationcancel", this.handleExitAnimationEnd); } @@ -133,14 +136,16 @@ export default class extends Controller { handleExitAnimationEnd = (event) => { // animationend bubbles — an animated child must not hide its container. if (event.target !== event.currentTarget) return; + // Closing mid-open cancels the enter animation; only the exit run settles this. + if (event.animationName !== this.exitAnimationName) return; - const content = event.currentTarget; - content.removeEventListener("animationend", this.handleExitAnimationEnd); - content.removeEventListener("animationcancel", this.handleExitAnimationEnd); - this.hideUnlessReopened(content); + this.settleExit(event.currentTarget); }; - hideUnlessReopened(content) { + settleExit(content) { + content.removeEventListener("animationend", this.handleExitAnimationEnd); + content.removeEventListener("animationcancel", this.handleExitAnimationEnd); + // Reopened mid-exit: it is on its way back in, leave it visible. if (content.dataset.state !== "closed") return; content.classList.add("hidden"); From dd5694da375b62f2d835119c6913e6a7fd5c4816 Mon Sep 17 00:00:00 2001 From: Tomek Date: Thu, 13 Aug 2026 01:05:29 +0200 Subject: [PATCH 3/3] Match the exit run against every computed animation name animation-name is comma-separated when the content carries more than one animation, while each event names a single run, so the equality check rejected them all and the element never hid. Co-Authored-By: Claude Opus 5 (1M context) --- gem/lib/ruby_ui/context_menu/context_menu_controller.js | 4 ++-- gem/lib/ruby_ui/hover_card/hover_card_controller.js | 4 ++-- gem/lib/ruby_ui/popover/popover_controller.js | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/gem/lib/ruby_ui/context_menu/context_menu_controller.js b/gem/lib/ruby_ui/context_menu/context_menu_controller.js index 8c3a2d88b..aa1dd16c6 100644 --- a/gem/lib/ruby_ui/context_menu/context_menu_controller.js +++ b/gem/lib/ruby_ui/context_menu/context_menu_controller.js @@ -71,7 +71,7 @@ export default class extends Controller { return; } - this.exitAnimationName = styles.animationName; + this.exitAnimationNames = styles.animationName.split(",").map((name) => name.trim()); content.addEventListener("animationend", this.handleExitAnimationEnd); content.addEventListener("animationcancel", this.handleExitAnimationEnd); } @@ -80,7 +80,7 @@ export default class extends Controller { // animationend bubbles — an animated child must not hide its container. if (event.target !== event.currentTarget) return; // Closing mid-open cancels the enter animation; only the exit run settles this. - if (event.animationName !== this.exitAnimationName) return; + if (!this.exitAnimationNames.includes(event.animationName)) return; this.settleExit(event.currentTarget); }; diff --git a/gem/lib/ruby_ui/hover_card/hover_card_controller.js b/gem/lib/ruby_ui/hover_card/hover_card_controller.js index 7408b2cf3..ada5ed0cc 100644 --- a/gem/lib/ruby_ui/hover_card/hover_card_controller.js +++ b/gem/lib/ruby_ui/hover_card/hover_card_controller.js @@ -117,7 +117,7 @@ export default class extends Controller { return; } - this.exitAnimationName = styles.animationName; + this.exitAnimationNames = styles.animationName.split(",").map((name) => name.trim()); content.addEventListener("animationend", this.handleExitAnimationEnd); content.addEventListener("animationcancel", this.handleExitAnimationEnd); } @@ -126,7 +126,7 @@ export default class extends Controller { // animationend bubbles — an animated child must not hide its container. if (event.target !== event.currentTarget) return; // Closing mid-open cancels the enter animation; only the exit run settles this. - if (event.animationName !== this.exitAnimationName) return; + if (!this.exitAnimationNames.includes(event.animationName)) return; this.settleExit(event.currentTarget); }; diff --git a/gem/lib/ruby_ui/popover/popover_controller.js b/gem/lib/ruby_ui/popover/popover_controller.js index eb9ce16a7..292a9e160 100644 --- a/gem/lib/ruby_ui/popover/popover_controller.js +++ b/gem/lib/ruby_ui/popover/popover_controller.js @@ -128,7 +128,7 @@ export default class extends Controller { return; } - this.exitAnimationName = styles.animationName; + this.exitAnimationNames = styles.animationName.split(",").map((name) => name.trim()); content.addEventListener("animationend", this.handleExitAnimationEnd); content.addEventListener("animationcancel", this.handleExitAnimationEnd); } @@ -137,7 +137,7 @@ export default class extends Controller { // animationend bubbles — an animated child must not hide its container. if (event.target !== event.currentTarget) return; // Closing mid-open cancels the enter animation; only the exit run settles this. - if (event.animationName !== this.exitAnimationName) return; + if (!this.exitAnimationNames.includes(event.animationName)) return; this.settleExit(event.currentTarget); };