Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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 docs/app/views/docs/alert_dialog.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def view_template
<<~RUBY
AlertDialog do
AlertDialogTrigger do
Button { "Show dialog" }
Button(variant: :outline) { "Show dialog" }
end
AlertDialogContent do
AlertDialogHeader do
Expand Down
5 changes: 4 additions & 1 deletion gem/lib/ruby_ui/alert_dialog/alert_dialog_action.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ def view_template(&)

def default_attrs
{
variant: :primary
variant: :primary,
data: {
action: "click->ruby-ui--alert-dialog#dismiss"
}
}
end
end
Expand Down
1 change: 1 addition & 0 deletions gem/lib/ruby_ui/alert_dialog/alert_dialog_cancel.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ def view_template(&)
def default_attrs
{
variant: :outline,
autofocus: true,
data: {
action: "click->ruby-ui--alert-dialog#dismiss"
},
Expand Down
39 changes: 10 additions & 29 deletions gem/lib/ruby_ui/alert_dialog/alert_dialog_content.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,42 +2,23 @@

module RubyUI
class AlertDialogContent < Base
def view_template(&block)
template(**attrs) do
div(data: {controller: "ruby-ui--alert-dialog"}) do
background
container(&block)
end
end
end

def background
div(
data_state: "open",
class: "fixed inset-0 z-50 bg-black/80 backdrop-blur-sm data-[state=open]:animate-in",
style: "pointer-events:auto",
data_aria_hidden: "true",
aria_hidden: "true"
)
end

def container(&)
div(
role: "alertdialog",
data_state: "open",
class: "flex flex-col fixed left-[50%] top-[50%] z-50 w-full max-w-lg max-h-screen overflow-y-auto translate-x-[-50%] translate-y-[-50%] gap-4 border bg-background p-6 shadow-lg duration-200 data-[state=open]:animate-in data-[state=open]:fade-in-0 data-[state=open]:zoom-in-95 sm:rounded-lg md:w-full",
style: "pointer-events:auto",
&
)
def view_template(&)
dialog(**attrs, &)
end

private

def default_attrs
{
role: "alertdialog",
data: {
ruby_ui__alert_dialog_target: "content"
}
ruby_ui__alert_dialog_target: "dialog"
},
class: [
"fixed open:flex flex-col left-[50%] top-[50%] z-50 w-full max-w-lg 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"
]
}
end
end
Expand Down
78 changes: 71 additions & 7 deletions gem/lib/ruby_ui/alert_dialog/alert_dialog_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Controller } from "@hotwired/stimulus";

// Connects to data-controller="ruby-ui--alert-dialog"
export default class extends Controller {
static targets = ["content"];
static targets = ["dialog"];
static values = {
open: {
type: Boolean,
Expand All @@ -11,21 +11,85 @@ export default class extends Controller {
};

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

disconnect() {
this.dialogTarget.removeEventListener("cancel", this.handleCancel);
this.dialogTarget.removeEventListener("close", this.handleClose);
// Nothing is left to wait for the exit animation, so apply the pending close now.
this.settleExit(this.dialogTarget);
Comment on lines +22 to +25

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 the dialog target is removed before its controller element, disconnect() throws before removing overflow-hidden, leaving page scrolling disabled. Guard the target-specific teardown with hasDialogTarget while always removing the body class.

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

<comment>When the dialog target is removed before its controller element, `disconnect()` throws before removing `overflow-hidden`, leaving page scrolling disabled. Guard the target-specific teardown with `hasDialogTarget` while always removing the body class.</comment>

<file context>
@@ -11,21 +11,85 @@ export default class extends Controller {
   }
 
+  disconnect() {
+    this.dialogTarget.removeEventListener("cancel", this.handleCancel);
+    this.dialogTarget.removeEventListener("close", this.handleClose);
+    // Nothing is left to wait for the exit animation, so apply the pending close now.
</file context>
Suggested change
this.dialogTarget.removeEventListener("cancel", this.handleCancel);
this.dialogTarget.removeEventListener("close", this.handleClose);
// Nothing is left to wait for the exit animation, so apply the pending close now.
this.settleExit(this.dialogTarget);
if (this.hasDialogTarget) {
this.dialogTarget.removeEventListener("cancel", this.handleCancel);
this.dialogTarget.removeEventListener("close", this.handleClose);
// 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() {
document.body.insertAdjacentHTML("beforeend", this.contentTarget.innerHTML);
// prevent scroll on body
this.dialogTarget.dataset.state = "open";
if (!this.dialogTarget.open) this.dialogTarget.showModal();
document.body.classList.add("overflow-hidden");
}

dismiss(e) {
// allow scroll on body
dismiss() {
if (this.dialogTarget.dataset.state === "closed") return;

this.dialogTarget.dataset.state = "closed";
// The backdrop's animationend lands on this element under the same name, so both exits run for the same 200 ms.
this.hideAfterExitAnimation(this.dialogTarget);
}

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

// Escape (and requestClose) must play the exit animation instead of closing at once.
handleCancel = (event) => {
event.preventDefault();
this.dismiss();
};

// A close this controller did not start must not leave the exit listeners behind.
handleClose = () => {
document.body.classList.remove("overflow-hidden");
// remove the element
this.element.remove();
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()
.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));
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);
}
}
2 changes: 1 addition & 1 deletion gem/lib/ruby_ui/alert_dialog/alert_dialog_docs.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def view_template
<<~RUBY
AlertDialog do
AlertDialogTrigger do
Button { "Show dialog" }
Button(variant: :outline) { "Show dialog" }
end
AlertDialogContent do
AlertDialogHeader do
Expand Down
121 changes: 121 additions & 0 deletions gem/test/ruby_ui/alert_dialog_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,125 @@ def test_render_with_all_items

assert_match(/Show dialog/, output)
end

# Regression test: content must be a native <dialog>, not a <template> cloned onto <body>
def test_content_renders_native_dialog_with_alertdialog_role
output = render_alert_dialog

assert_match(/<dialog[^>]*\srole="alertdialog"/, output, "AlertDialogContent must render a native <dialog role=\"alertdialog\">")
refute_match(/<template[\s>]/, output, "AlertDialogContent must not use a <template> element")
end

def test_content_does_not_nest_a_second_controller
output = render_alert_dialog

assert_equal 1, output.scan('data-controller="ruby-ui--alert-dialog"').size, "Only the wrapper carries the controller"
assert_match(/<div[^>]*data-controller="ruby-ui--alert-dialog"/, output, "AlertDialog wrapper must be a <div>")
end

def test_content_has_stimulus_target
assert_match(/<dialog[^>]*data-ruby-ui--alert-dialog-target="dialog"/, render_alert_dialog)
end

# Radix parity: Escape closes, clicking the backdrop does not
def test_content_has_no_backdrop_click_action
refute_match(/backdropClick/, render_alert_dialog)
end

# Regression test: a closed native <dialog> must stay hidden. Bare `flex`
# overrides the UA `dialog:not([open]) { display: none }`.
def test_content_does_not_force_display_when_closed
classes = dialog_classes(render_alert_dialog)

refute_includes classes, "flex", "Bare `flex` forces a closed <dialog> to display; use `open:flex`"
assert_includes classes, "open:flex", "AlertDialog must apply flex only when open (open:flex)"
end

# The controller owns data-state; the closed markup must not start in one
def test_content_renders_without_data_state
refute_match(/<dialog[^>]*\sdata-state=/, render_alert_dialog)
end

def test_content_has_enter_and_exit_animations
assert_dialog_classes(
"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"
)
end

# The backdrop's animationend is dispatched on the <dialog> under the same keyframe name,
# so its exit must be as long as the panel's or the panel is cut short.
def test_content_animates_and_styles_backdrop
assert_dialog_classes(
"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"
)
end

def test_cancel_autofocuses_and_dismisses
output = phlex { RubyUI.AlertDialogCancel { "Cancel" } }

assert_match(/<button[^>]*\sautofocus[\s>]/, output, "Cancel must receive focus when the dialog opens")
assert_match(/data-action="click->ruby-ui--alert-dialog#dismiss"/, output)
end

def test_action_dismisses
output = phlex { RubyUI.AlertDialogAction { "Continue" } }

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

def test_action_keeps_user_supplied_actions
output = phlex { RubyUI.AlertDialogAction(data: {action: "click->account#destroy"}) { "Continue" } }

assert_match(/data-action="click->ruby-ui--alert-dialog#dismiss click->account#destroy"/, output)
end

def test_trigger_has_open_action
assert_match(/data-action="click->ruby-ui--alert-dialog#open"/, render_alert_dialog)
end

def test_open_value_is_set_on_wrapper
output = phlex do
RubyUI.AlertDialog(open: true) do
RubyUI.AlertDialogContent { "Content" }
end
end

assert_match(/data-ruby-ui--alert-dialog-open-value="true"/, output)
end

private

def render_alert_dialog
phlex do
RubyUI.AlertDialog do
RubyUI.AlertDialogTrigger do
RubyUI.Button { "Open" }
end
RubyUI.AlertDialogContent { "Content" }
end
end
end

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

def assert_dialog_classes(*expected)
classes = dialog_classes(render_alert_dialog)
expected.each { |class_name| assert_includes classes, class_name }
end
end
Loading