Skip to content

[Bug Fix] Popover, HoverCard, ContextMenu: play the exit animation before hiding #506

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tvq wants to merge 3 commits into
base: main
Choose a base branch
from
Open

[Bug Fix] Popover, HoverCard, ContextMenu: play the exit animation before hiding #506

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0e8145f
[Bug Fix] Popover, HoverCard, ContextMenu: play the exit animation be…
tvq Aug 12, 2026
204845e
Only settle on the exit animation, and settle on disconnect
tvq Aug 12, 2026
dd5694d
Match the exit run against every computed animation name
tvq Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gem/lib/ruby_ui/context_menu/context_menu_content.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down
37 changes: 36 additions & 1 deletion gem/lib/ruby_ui/context_menu/context_menu_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -49,8 +51,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) {
Expand All @@ -59,6 +61,39 @@ 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") {

@cubic-dev-ai cubic-dev-ai Bot Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When prefers-reduced-motion: reduce is active, this controller still waits for the animate-out event because the repository does not disable that animation automatically. Check the reduced-motion media query here, or provide a CSS rule that makes the animation name none, so the menu hides immediately.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At gem/lib/ruby_ui/context_menu/context_menu_controller.js, line 67:

<comment>When `prefers-reduced-motion: reduce` is active, this controller still waits for the `animate-out` event because the repository does not disable that animation automatically. Check the reduced-motion media query here, or provide a CSS rule that makes the animation name `none`, so the menu hides immediately.</comment>

<file context>
@@ -59,6 +59,36 @@ export default class extends Controller {
+    const styles = getComputedStyle(content);
+
+    // An element with no exit animation never fires animationend.
+    if (styles.animationName === "none" || styles.display === "none") {
+      this.hideUnlessReopened(content);
+      return;
</file context>
Suggested change
if (styles.animationName === "none" || styles.display === "none") {
if (
styles.animationName === "none" ||
styles.display === "none" ||
window.matchMedia("(prefers-reduced-motion: reduce)").matches
) {

@tvq tvq Aug 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked this one before acting on it, and I'd rather not take the suggestion — but the premise is right.

grep -c prefers-reduced-motion on the docs app's built application.css returns 0: the library ships no reduced-motion rules at all, so tw-animate-css animations run regardless of the setting. Where I think the conclusion differs is the consequence — the exit animation still runs and animationend still fires, so nothing hangs. Reduced-motion users see the same exit they already see on enter.

Adding matchMedia only here would make the close jump while the open still animates, which reads worse than the current consistency. And reduced-motion support looks library-wide to me — every animate-in in the overlay family, not just these three exits — so I'd rather not smuggle a partial version in under a bug fix. I've noted it in the PR description and am happy to open a separate issue for it.

The computed animation-name check earns its place for the cases it does cover: no tw-animate-css installed, overridden classes, or a consumer's own reduced-motion CSS.

this.settleExit(content);
return;
}

this.exitAnimationNames = styles.animationName.split(",").map((name) => name.trim());
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;
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
// Closing mid-open cancels the enter animation; only the exit run settles this.
if (!this.exitAnimationNames.includes(event.animationName)) return;

this.settleExit(event.currentTarget);
};

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");
}

updatePosition() {
if (this.cleanup) this.cleanup();

Expand Down
2 changes: 1 addition & 1 deletion gem/lib/ruby_ui/hover_card/hover_card_content.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 36 additions & 1 deletion gem/lib/ruby_ui/hover_card/hover_card_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -95,8 +97,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) {
Expand All @@ -105,6 +107,39 @@ 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.settleExit(content);
return;
}

this.exitAnimationNames = styles.animationName.split(",").map((name) => name.trim());
content.addEventListener("animationend", this.handleExitAnimationEnd);
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
content.addEventListener("animationcancel", this.handleExitAnimationEnd);
}

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 (!this.exitAnimationNames.includes(event.animationName)) return;

this.settleExit(event.currentTarget);
};

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");
}

updatePosition() {
if (this.cleanup) this.cleanup();

Expand Down
1 change: 1 addition & 0 deletions gem/lib/ruby_ui/popover/popover_content.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
37 changes: 36 additions & 1 deletion gem/lib/ruby_ui/popover/popover_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -112,8 +114,41 @@ export default class extends Controller {

if (!this.hasContentTarget) return;

this.contentTarget.classList.add("hidden");
this.contentTarget.dataset.state = "closed";
this.hideAfterExitAnimation();
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
}

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.settleExit(content);
return;
}

this.exitAnimationNames = styles.animationName.split(",").map((name) => name.trim());
content.addEventListener("animationend", this.handleExitAnimationEnd);
content.addEventListener("animationcancel", this.handleExitAnimationEnd);
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
}

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 (!this.exitAnimationNames.includes(event.animationName)) return;

this.settleExit(event.currentTarget);
};

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");
}

updatePosition() {
Expand Down
11 changes: 11 additions & 0 deletions gem/test/ruby_ui/context_menu_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 9 additions & 0 deletions gem/test/ruby_ui/hover_card_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 9 additions & 0 deletions gem/test/ruby_ui/popover_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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