Skip to content

[Bug Fix] Dialog: play the exit animation before closing the native <dialog> #517

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 4 commits into
base: main
Choose a base branch
from
+200 −34
Open

[Bug Fix] Dialog: play the exit animation before closing the native <dialog> #517

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6cba4b0
[Bug Fix] Dialog: play the exit animation before closing the native <…
tvq Aug 28, 2026
f52f8b8
chore(mcp): rebuild registry after the Dialog change
tvq Aug 28, 2026
88fc170
[Documentation] Dialog: outline trigger button, matching shadcn
tvq Aug 28, 2026
9abe94c
[Feature] Dialog: show_close_button option on DialogContent
tvq Aug 29, 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
25 changes: 22 additions & 3 deletions docs/app/views/docs/dialog.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def view_template
<<~RUBY
Dialog do
DialogTrigger do
Button { "Open Dialog" }
Button(variant: :outline) { "Open Dialog" }
end
DialogContent do
DialogHeader do
Expand Down Expand Up @@ -43,7 +43,7 @@ def view_template
div(class: 'flex flex-wrap justify-center gap-2') do
Dialog do
DialogTrigger do
Button { "Small Dialog" }
Button(variant: :outline) { "Small Dialog" }
end
DialogContent(size: :sm) do
DialogHeader do
Expand All @@ -68,7 +68,7 @@ def view_template

Dialog do
DialogTrigger do
Button { "Large Dialog" }
Button(variant: :outline) { "Large Dialog" }
end
DialogContent(size: :lg) do
DialogHeader do
Expand All @@ -94,6 +94,25 @@ def view_template
RUBY
end

render Docs::VisualCodeExample.new(title: "No close button", description: "Hide the corner close button and provide your own close action.", context: self) do
<<~RUBY
Dialog do
DialogTrigger do
Button(variant: :outline) { "No Close Button" }
end
DialogContent(show_close_button: false) do
DialogHeader do
DialogTitle { "No close button" }
DialogDescription { "This dialog has no close button in the corner. Escape and a click outside still close it." }
end
DialogFooter do
Button(variant: :outline, data: { action: 'click->ruby-ui--dialog#dismiss' }) { "Close" }
end
end
end
RUBY
end

render Components::ComponentSetup::Tabs.new(component_name: component)

render Docs::ComponentsTable.new(component_files(component))
Expand Down
9 changes: 6 additions & 3 deletions gem/lib/ruby_ui/dialog/dialog_content.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,16 @@ class DialogContent < Base
full: "max-w-full"
}

def initialize(size: :md, **attrs)
def initialize(size: :md, show_close_button: true, **attrs)
@size = size
@show_close_button = show_close_button
super(**attrs)
end

def view_template
dialog(**attrs) do
yield
close_button
close_button if @show_close_button
end
end

Expand All @@ -30,7 +31,9 @@ def default_attrs
data_ruby_ui__dialog_target: "dialog",
data_action: "click->ruby-ui--dialog#backdropClick",
class: [
"fixed open:flex flex-col pointer-events-auto left-[50%] top-[50%] z-50 w-full max-h-screen overflow-y-auto translate-x-[-50%] translate-y-[-50%] gap-4 border bg-background p-6 shadow-lg duration-200 backdrop:bg-background/80 backdrop:backdrop-blur-sm open:animate-in open:fade-in-0 open:zoom-in-95 sm:rounded-lg md:w-full",
"fixed open:flex flex-col pointer-events-auto left-[50%] top-[50%] z-50 w-full max-h-screen overflow-y-auto translate-x-[-50%] translate-y-[-50%] gap-4 border bg-background p-6 shadow-lg sm:rounded-lg md:w-full",
"duration-200 data-[state=open]:animate-in data-[state=open]:fade-in-0 data-[state=open]:zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[state=closed]:fill-mode-forwards",
"backdrop:bg-background/80 backdrop:backdrop-blur-sm backdrop:duration-200 data-[state=open]:backdrop:animate-in data-[state=open]:backdrop:fade-in-0 data-[state=closed]:backdrop:animate-out data-[state=closed]:backdrop:fade-out-0 data-[state=closed]:backdrop:fill-mode-forwards",
SIZES[@size]
]
}
Expand Down
90 changes: 76 additions & 14 deletions gem/lib/ruby_ui/dialog/dialog_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,44 +1,106 @@
import { Controller } from "@hotwired/stimulus"
import { Controller } from "@hotwired/stimulus";

// Connects to data-controller="ruby-ui--dialog"
export default class extends Controller {
static targets = ["dialog"]
static targets = ["dialog"];
static values = {
open: {
type: Boolean,
default: false
default: false,
},
}
};

connect() {
this.dialogTarget.addEventListener("close", this.handleClose)
this.dialogTarget.addEventListener("close", this.handleClose);
this.dialogTarget.addEventListener("cancel", this.handleCancel);
if (this.openValue) {
this.open()
this.open();
}
}

disconnect() {
this.dialogTarget.removeEventListener("close", this.handleClose)
document.body.classList.remove("overflow-hidden")
this.dialogTarget.removeEventListener("close", this.handleClose);
this.dialogTarget.removeEventListener("cancel", this.handleCancel);
// Nothing is left to wait for the exit animation, so apply the pending close now.
this.settleExit(this.dialogTarget);
document.body.classList.remove("overflow-hidden");
}

open(e) {
e?.preventDefault()
this.dialogTarget.showModal()
document.body.classList.add("overflow-hidden")
e?.preventDefault();
this.dialogTarget.dataset.state = "open";
// Reopened mid-exit the dialog is still open; showModal() on an open dialog throws in older browsers.
if (!this.dialogTarget.open) this.dialogTarget.showModal();
document.body.classList.add("overflow-hidden");
}

dismiss() {
this.dialogTarget.close()
if (this.dialogTarget.dataset.state === "closed") return;

this.dialogTarget.dataset.state = "closed";
// The ::backdrop's animationend lands on the dialog too; panel and backdrop share one duration so either settles it.
this.hideAfterExitAnimation(this.dialogTarget);
}

afterExit() {
this.dialogTarget.close();
}

backdropClick(e) {
if (e.target === this.dialogTarget) {
this.dismiss()
this.dismiss();
}
}

// Escape (and requestClose()) fire cancel; route it through the exit animation.
handleCancel = (e) => {
// A cancelled file picker inside the dialog bubbles its own cancel event.
if (e.target !== this.dialogTarget) return;

e.preventDefault();

@cubic-dev-ai cubic-dev-ai Bot Aug 28, 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 a second Escape arrives during an existing exit, this unconditional preventDefault() blocks browsers that expose that cancel event as cancelable. Return before preventing the event when data-state is already closed.

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

<comment>When a second Escape arrives during an existing exit, this unconditional `preventDefault()` blocks browsers that expose that `cancel` event as cancelable. Return before preventing the event when `data-state` is already `closed`.</comment>

<file context>
@@ -1,44 +1,106 @@
+    // A cancelled file picker inside the dialog bubbles its own cancel event.
+    if (e.target !== this.dialogTarget) return;
+
+    e.preventDefault();
+    this.dismiss();
+  };
</file context>
Suggested change
e.preventDefault();
if (this.dialogTarget.dataset.state === "closed") return;

this.dismiss();
};

handleClose = () => {
document.body.classList.remove("overflow-hidden")
document.body.classList.remove("overflow-hidden");
// A close this controller did not start (a second Escape mid-exit) must not leave the exit listeners behind.
this.settleExit(this.dialogTarget);
};

// Overlay exit — the same block in every overlay controller, so keep them in sync.
exitAnimationNames = new WeakMap();

hideAfterExitAnimation(animated) {
const exitAnimations = animated
.getAnimations()

@cubic-dev-ai cubic-dev-ai Bot Aug 28, 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: getAnimations() does not include ::backdrop animations by default, so this controller never tracks the backdrop exit. The native dialog closes on the panel's event, cutting off the backdrop whenever their timing differs. Track the ::backdrop animation explicitly and settle only after the required exits complete.

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

<comment>`getAnimations()` does not include `::backdrop` animations by default, so this controller never tracks the backdrop exit. The native dialog closes on the panel's event, cutting off the backdrop whenever their timing differs. Track the `::backdrop` animation explicitly and settle only after the required exits complete.</comment>

<file context>
@@ -1,44 +1,106 @@
+
+  hideAfterExitAnimation(animated) {
+    const exitAnimations = animated
+      .getAnimations()
+      .filter((animation) => animation instanceof CSSAnimation);
+
</file context>

.filter((animation) => animation instanceof CSSAnimation);

// No exit animation, or no box to run it in: animationend would never fire.
if (exitAnimations.length === 0) {
this.settleExit(animated);
return;
}

this.exitAnimationNames.set(animated, exitAnimations.map((animation) => animation.animationName));

@cubic-dev-ai cubic-dev-ai Bot Aug 28, 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.

P1: When a dialog is reopened and dismissed again before the prior animation event is delivered, the stale exit event matches this name-only map and closes the new run immediately. Track a dismissal generation or the specific animation run so events from older exits cannot settle the current one.

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

<comment>When a dialog is reopened and dismissed again before the prior animation event is delivered, the stale `exit` event matches this name-only map and closes the new run immediately. Track a dismissal generation or the specific animation run so events from older exits cannot settle the current one.</comment>

<file context>
@@ -1,44 +1,106 @@
+      return;
+    }
+
+    this.exitAnimationNames.set(animated, exitAnimations.map((animation) => animation.animationName));
+    animated.addEventListener("animationend", this.handleExitAnimationEnd);
+    animated.addEventListener("animationcancel", this.handleExitAnimationEnd);
</file context>

animated.addEventListener("animationend", this.handleExitAnimationEnd);
animated.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.get(event.currentTarget)?.includes(event.animationName)) return;

this.settleExit(event.currentTarget);
};

settleExit(animated) {
animated.removeEventListener("animationend", this.handleExitAnimationEnd);
animated.removeEventListener("animationcancel", this.handleExitAnimationEnd);
// Reopened mid-exit: it is on its way back in, leave it visible.
if (animated.dataset.state !== "closed") return;

this.afterExit(animated);
}
}
25 changes: 22 additions & 3 deletions gem/lib/ruby_ui/dialog/dialog_docs.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def view_template
<<~RUBY
Dialog do
DialogTrigger do
Button { "Open Dialog" }
Button(variant: :outline) { "Open Dialog" }
end
DialogContent do
DialogHeader do
Expand Down Expand Up @@ -43,7 +43,7 @@ def view_template
div(class: 'flex flex-wrap justify-center gap-2') do
Dialog do
DialogTrigger do
Button { "Small Dialog" }
Button(variant: :outline) { "Small Dialog" }
end
DialogContent(size: :sm) do
DialogHeader do
Expand All @@ -68,7 +68,7 @@ def view_template

Dialog do
DialogTrigger do
Button { "Large Dialog" }
Button(variant: :outline) { "Large Dialog" }
end
DialogContent(size: :lg) do
DialogHeader do
Expand All @@ -94,6 +94,25 @@ def view_template
RUBY
end

render Docs::VisualCodeExample.new(title: "No close button", description: "Hide the corner close button and provide your own close action.", context: self) do
<<~RUBY
Dialog do
DialogTrigger do
Button(variant: :outline) { "No Close Button" }
end
DialogContent(show_close_button: false) do
DialogHeader do
DialogTitle { "No close button" }
DialogDescription { "This dialog has no close button in the corner. Escape and a click outside still close it." }
end
DialogFooter do
Button(variant: :outline, data: { action: 'click->ruby-ui--dialog#dismiss' }) { "Close" }
end
end
end
RUBY
end

render Components::ComponentSetup::Tabs.new(component_name: component)

render Docs::ComponentsTable.new(component_files(component))
Expand Down
70 changes: 64 additions & 6 deletions gem/test/ruby_ui/dialog_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
require "test_helper"

class RubyUI::DialogTest < ComponentTest
CLOSE_BUTTON = /<span class="sr-only">Close<\/span>/

def test_render_with_all_items
output = phlex do
RubyUI.Dialog do
Expand Down Expand Up @@ -81,13 +83,8 @@ def test_dialog_content_has_backdrop_click_action
# utility (author CSS) overrides the UA `dialog:not([open]) { display: none }`,
# making the dialog always visible. Display must be gated on the open: variant.
def test_dialog_content_does_not_force_display_when_closed
output = phlex do
RubyUI.Dialog do
RubyUI.DialogContent { "Content" }
end
end
classes = dialog_classes

classes = output[/<dialog\b.*?\sclass="([^"]*)"/m, 1].to_s.split
refute_includes classes, "flex", "Bare `flex` forces a closed <dialog> to display; use `open:flex`"
assert_includes classes, "open:flex", "Dialog must apply flex only when open (open:flex)"
end
Expand Down Expand Up @@ -136,4 +133,65 @@ def test_trigger_has_open_action

assert_match(/data-action="click->ruby-ui--dialog#open"/, output)
end

# Animations key on data-state (set by the controller) so the closed state can still render the exit.
def test_dialog_content_animates_enter_and_exit_on_data_state
classes = dialog_classes

%w[
duration-200
data-[state=open]:animate-in data-[state=open]:fade-in-0 data-[state=open]:zoom-in-95
data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[state=closed]:fill-mode-forwards
].each { |klass| assert_includes classes, klass }

refute classes.any? { |klass| klass.start_with?("open:animate", "open:fade", "open:zoom") }, "Animations must key on data-state, not on the open: variant"
assert_includes classes, "open:flex", "Display must still be gated on the open attribute"
end

# The ::backdrop's animationend lands on the <dialog> itself, so both exits must share one duration.
def test_dialog_content_animates_backdrop_on_data_state
classes = dialog_classes

%w[
backdrop:bg-background/80 backdrop:backdrop-blur-sm backdrop:duration-200
data-[state=open]:backdrop:animate-in data-[state=open]:backdrop:fade-in-0
data-[state=closed]:backdrop:animate-out data-[state=closed]:backdrop:fade-out-0 data-[state=closed]:backdrop:fill-mode-forwards
].each { |klass| assert_includes classes, klass }
end

def test_dialog_content_renders_the_close_button_by_default
assert_match(CLOSE_BUTTON, dialog_output)
end

def test_dialog_content_omits_the_close_button_when_disabled
output = dialog_output(show_close_button: false)

assert_match(/Content/, output)
refute_match(CLOSE_BUTTON, output, "show_close_button: false must not render the close button")
refute_match(/data-action="click->ruby-ui--dialog#dismiss"/, output, "The only dismiss action came from the close button")
end

def test_dialog_content_does_not_render_data_state_when_closed
output = phlex do
RubyUI.Dialog do
RubyUI.DialogContent { "Content" }
end
end

refute_match(/<dialog\b[^>]*\sdata-state=/, output, "data-state is owned by the controller; a closed dialog must not render one")
end

private

def dialog_output(**attrs)
phlex do
RubyUI.Dialog do
RubyUI.DialogContent(**attrs) { "Content" }
end
end
end

def dialog_classes
dialog_output[/<dialog\b.*?\sclass="([^"]*)"/m, 1].to_s.split
end
end
Loading