Skip to content

[Bug Fix] Sheet: render a native <dialog> and close on Escape #520

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 2 commits into
base: main
Choose a base branch
from
+352 −94
Open

[Bug Fix] Sheet: render a native <dialog> and close on Escape #520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
021005c
[Bug Fix] Sheet: render a native <dialog> and close on Escape
tvq Aug 29, 2026
9ed43db
chore(mcp): rebuild registry after the Sheet change
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
57 changes: 39 additions & 18 deletions docs/app/views/docs/sheet.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ def view_template
component = "Sheet"

div(class: "max-w-2xl mx-auto w-full py-10 space-y-10") do
render Docs::Header.new(title: "Sheet", description: "Extends the Sheet component to display content that complements the main content of the screen.")
render Docs::Header.new(title: "Sheet", description: "Extends the Dialog component to display content that complements the main content of the screen.")

Heading(level: 2) { "Usage" }

Expand All @@ -15,7 +15,7 @@ def view_template
SheetTrigger do
Button(variant: :outline) { "Open Sheet" }
end
SheetContent(class: 'sm:max-w-sm') do
SheetContent do
SheetHeader do
SheetTitle { "Edit profile" }
SheetDescription { "Make changes to your profile here. Click save when you're done." }
Expand All @@ -28,7 +28,9 @@ def view_template
Input(placeholder: "joel@drapper.me")
end
SheetFooter do
Button(variant: :outline, data: { action: 'click->ruby-ui--sheet-content#close' }) { "Cancel" }
SheetClose do
Button(variant: :outline) { "Cancel" }
end
Button(type: "submit") { "Save" }
end
end
Expand All @@ -38,27 +40,25 @@ def view_template

render Docs::VisualCodeExample.new(title: "Side", description: "Use the side property to indicate the edge of the screen where the component will appear.", context: self) do
<<~RUBY
div(class: 'grid grid-cols-2 gap-4') do
# -- TOP --
Sheet do
SheetTrigger do
Button(variant: :outline, class: 'w-full justify-center') { :top }
end
SheetContent(side: :top, class: ("sm:max-w-sm" if [:left, :right].include?(:top))) do
SheetHeader do
SheetTitle { "Edit profile" }
SheetDescription { "Make changes to your profile here. Click save when you're done." }
div(class: 'flex flex-wrap gap-2') do
[:top, :right, :bottom, :left].each do |side|
Sheet do
SheetTrigger do
Button(variant: :outline, class: 'capitalize') { side.to_s }
end
Form do
SheetContent(side: side) do

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

P3: The rewritten "Side" example drops the <Form wrapper (it used to wrap the fields and footer) but keeps Button(type: "submit") { "Save" }. Without a form, the Save submit button now renders inert — pressing it does nothing. Wrap the SheetMiddle/SheetFooter in a Form (as the previous example did) or change Save to a plain button so the copied example behaves as documented.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At docs/app/views/docs/sheet.rb, line 49:

<comment>The rewritten "Side" example drops the `<Form` wrapper (it used to wrap the fields and footer) but keeps `Button(type: "submit") { "Save" }`. Without a form, the Save submit button now renders inert — pressing it does nothing. Wrap the SheetMiddle/SheetFooter in a `Form` (as the previous example did) or change Save to a plain button so the copied example behaves as documented.</comment>

<file context>
@@ -38,27 +40,25 @@ def view_template
+                  Button(variant: :outline, class: 'capitalize') { side.to_s }
                 end
-                Form do
+                SheetContent(side: side) do
+                  SheetHeader do
+                    SheetTitle { "Edit profile" }
</file context>

SheetHeader do
SheetTitle { "Edit profile" }
SheetDescription { "Make changes to your profile here. Click save when you're done." }
end
SheetMiddle do
label { "Name" }
Input(placeholder: "Joel Drapper") { "Joel Drapper" }

label { "Email" }
Input(placeholder: "joel@drapper.me")
end
SheetFooter do
Button(variant: :outline, data: { action: 'click->ruby-ui--sheet-content#close' }) { "Cancel" }
SheetClose do
Button(variant: :outline) { "Cancel" }
end
Button(type: "submit") { "Save" }
end
end
Expand All @@ -68,6 +68,27 @@ def view_template
RUBY
end

render Docs::VisualCodeExample.new(title: "No close button", description: "Pass show_close_button: false to hide the close button in the corner. The sheet still closes on Escape or a click outside.", context: self) do
<<~RUBY
Sheet do
SheetTrigger do
Button(variant: :outline) { "Open Sheet" }
end
SheetContent(show_close_button: false) do
SheetHeader do
SheetTitle { "No close button" }
SheetDescription { "This sheet has no close button in the corner. Press Escape or click outside to close it." }
end
SheetFooter do
SheetClose do
Button(variant: :outline) { "Close" }
end
end
end
end
RUBY
end

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

render Docs::ComponentsTable.new(component_files(component))
Expand Down
17 changes: 17 additions & 0 deletions gem/lib/ruby_ui/sheet/sheet_close.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# frozen_string_literal: true

module RubyUI
class SheetClose < Base
def view_template(&)
div(**attrs, &)

@cubic-dev-ai cubic-dev-ai Bot Aug 29, 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: SheetClose renders a non-interactive

as the close control, so a bare SheetClose with no button child is not focusable, not keyboard-activatable, and not announced by screen readers. This PR's goal is alignment with the shadcn Sheet API, where SheetClose is a , and the component's own close_button is already a real . Render a so the wrapper is interactive regardless of its contents.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At gem/lib/ruby_ui/sheet/sheet_close.rb, line 6:

<comment>SheetClose renders a non-interactive <div> as the close control, so a bare SheetClose with no button child is not focusable, not keyboard-activatable, and not announced by screen readers. This PR's goal is alignment with the shadcn Sheet API, where SheetClose is a <button>, and the component's own close_button is already a real <button>. Render a <button type="button"> so the wrapper is interactive regardless of its contents.</comment>

<file context>
@@ -0,0 +1,17 @@
+module RubyUI
+  class SheetClose < Base
+    def view_template(&)
+      div(**attrs, &)
+    end
+
</file context>

end

private

def default_attrs
{
data: {action: "click->ruby-ui--sheet-content#close"}
}
end
end
end
52 changes: 24 additions & 28 deletions gem/lib/ruby_ui/sheet/sheet_content.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,40 +2,46 @@

module RubyUI
class SheetContent < Base
# Per side: release the opposite edge (a modal <dialog> is pinned to all four by inset: 0),
# then the default size and the direction to slide in from.
SIDE_CLASS = {
top: "inset-x-0 top-0 border-b data-[state=closed]:slide-out-to-top data-[state=open]:slide-in-from-top",
right: "inset-y-0 right-0 h-full border-l data-[state=closed]:slide-out-to-right data-[state=open]:slide-in-from-right",
bottom: "inset-x-0 bottom-0 border-t data-[state=closed]:slide-out-to-bottom data-[state=open]:slide-in-from-bottom",
left: "inset-y-0 left-0 h-full border-r data-[state=closed]:slide-out-to-left data-[state=open]:slide-in-from-left"
top: "inset-x-0 top-0 bottom-auto w-full h-auto border-b data-[state=closed]:slide-out-to-top data-[state=open]:slide-in-from-top",
right: "inset-y-0 right-0 left-auto h-full w-3/4 sm:max-w-sm border-l data-[state=closed]:slide-out-to-right data-[state=open]:slide-in-from-right",
bottom: "inset-x-0 bottom-0 top-auto w-full h-auto border-t data-[state=closed]:slide-out-to-bottom data-[state=open]:slide-in-from-bottom",
left: "inset-y-0 left-0 right-auto h-full w-3/4 sm:max-w-sm border-r data-[state=closed]:slide-out-to-left data-[state=open]:slide-in-from-left"
}

def initialize(side: :right, **attrs)
def initialize(side: :right, show_close_button: true, **attrs)
@side = side
@side_classes = SIDE_CLASS[side]
@show_close_button = show_close_button
super(**attrs)
end

def view_template(&block)
template(data: {ruby_ui__sheet_target: "content"}) do
div(data: {controller: "ruby-ui--sheet-content"}) do
backdrop
div(**attrs) do
block&.call
close_button
end
end
dialog(**attrs) do
block&.call
close_button if @show_close_button
end
end

private

def default_attrs
{
data_state: "open", # For animate in
data_ruby_ui__sheet_content_target: "panel",
data: {
controller: "ruby-ui--sheet-content",
ruby_ui__sheet_target: "dialog",
action: "click->ruby-ui--sheet-content#backdropClick",
side: @side
},
class: [
"fixed pointer-events-auto z-50 gap-4 bg-background p-6 shadow-lg transition ease-in-out data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fill-mode-forwards data-[state=closed]:duration-300 data-[state=open]:duration-500 overflow-scroll",
@side_classes
# UA <dialog> reset; not-open:hidden keeps a caller's bare `flex` from overriding the display: none of a closed dialog.
"m-0 max-w-full max-h-full not-open:hidden",
"fixed pointer-events-auto z-50 gap-4 bg-background text-foreground p-6 shadow-lg transition ease-in-out overflow-scroll",
"data-[state=open]:animate-in data-[state=open]:duration-500 data-[state=closed]:animate-out data-[state=closed]:duration-300 data-[state=closed]:fill-mode-forwards",
# The ::backdrop's animationend lands on the dialog itself, so its exit must last as long as the panel's.
"backdrop:bg-background/80 backdrop:backdrop-blur-sm 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:duration-300 data-[state=closed]:backdrop:fill-mode-forwards",
SIDE_CLASS[@side]
]
}
end
Expand Down Expand Up @@ -65,15 +71,5 @@ def close_button
span(class: "sr-only") { "Close" }
end
end

def backdrop
div(
data_state: "open",
data_action: "click->ruby-ui--sheet-content#close",
data_ruby_ui__sheet_content_target: "backdrop",
class:
"fixed pointer-events-auto inset-0 z-50 bg-background/80 backdrop-blur-sm 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]:fill-mode-forwards"
)
end
end
end
48 changes: 40 additions & 8 deletions gem/lib/ruby_ui/sheet/sheet_content_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,56 @@
import { Controller } from "@hotwired/stimulus";

// Connects to data-controller="ruby-ui--sheet-content" on the <dialog>; ruby-ui--sheet opens it.
export default class extends Controller {
static targets = ["backdrop", "panel"];
connect() {
this.element.addEventListener("cancel", this.handleCancel);
this.element.addEventListener("close", this.handleClose);
}

disconnect() {
// Nothing is left to wait for the exit animation, so apply the pending removal now.
if (this.hasPanelTarget) this.settleExit(this.panelTarget);
this.element.removeEventListener("cancel", this.handleCancel);
this.element.removeEventListener("close", this.handleClose);
// Nothing is left to wait for the exit animation, so apply the pending close now.
this.settleExit(this.element);

@cubic-dev-ai cubic-dev-ai Bot Aug 29, 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 SheetContent is detached while its parent sheet remains connected, the removed close listener cannot unlock the body and scrolling stays disabled. Remove overflow-hidden during teardown as well.

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

<comment>When `SheetContent` is detached while its parent sheet remains connected, the removed `close` listener cannot unlock the body and scrolling stays disabled. Remove `overflow-hidden` during teardown as well.</comment>

<file context>
@@ -1,24 +1,56 @@
+    this.element.removeEventListener("cancel", this.handleCancel);
+    this.element.removeEventListener("close", this.handleClose);
+    // Nothing is left to wait for the exit animation, so apply the pending close now.
+    this.settleExit(this.element);
   }
 
</file context>
Suggested change
this.settleExit(this.element);
this.settleExit(this.element);
document.body.classList.remove("overflow-hidden");

}

close() {
this.backdropTarget.dataset.state = "closed";
this.panelTarget.dataset.state = "closed";
// The panel carries the longer exit, so the backdrop has finished by the time it settles.
this.hideAfterExitAnimation(this.panelTarget);
if (this.element.dataset.state === "closed") return;

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

@cubic-dev-ai cubic-dev-ai Bot Aug 29, 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 sheet is reopened and closed again before the prior animationcancel is delivered, the stale cancellation matches the new run and closes the dialog immediately. Track the specific animation run or animation objects instead of matching names alone.

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

<comment>When a sheet is reopened and closed again before the prior `animationcancel` is delivered, the stale cancellation matches the new run and closes the dialog immediately. Track the specific animation run or animation objects instead of matching names alone.</comment>

<file context>
@@ -1,24 +1,56 @@
+
+    this.element.dataset.state = "closed";
+    // The ::backdrop's animationend lands on the dialog too; panel and backdrop share one exit duration so either settles it.
+    this.hideAfterExitAnimation(this.element);
   }
 
</file context>

}

afterExit() {
this.element.remove();
this.element.close();
}

// A click on the ::backdrop targets the dialog, but so does one on the panel's own padding: hit-test the box.
backdropClick(e) {
if (e.target === this.element && !this.coversPoint(e.clientX, e.clientY)) this.close();
}

coversPoint(x, y) {
const { top, right, bottom, left } = this.element.getBoundingClientRect();
return left <= x && x <= right && top <= y && y <= bottom;
}

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

e.preventDefault();
this.close();
};

handleClose = () => {
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.element);
};

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

Expand Down
22 changes: 15 additions & 7 deletions gem/lib/ruby_ui/sheet/sheet_controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,23 @@
import { Controller } from "@hotwired/stimulus"
import { Controller } from "@hotwired/stimulus";

// Connects to data-controller="ruby-ui--sheet"; opens the <dialog> that ruby-ui--sheet-content closes.
export default class extends Controller {
static targets = ["content"]

static values = { open: false }
static targets = ["dialog"];
static values = { open: false };

connect() {
if (this.openValue) this.open()
if (this.openValue) this.open();
}

disconnect() {
document.body.classList.remove("overflow-hidden");

@cubic-dev-ai cubic-dev-ai Bot Aug 29, 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 one Sheet disconnects while another Sheet is still open, this unconditional cleanup restores body scrolling behind the remaining modal. Track the lock per open instance and remove overflow-hidden only when no Sheet still owns it.

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

<comment>When one Sheet disconnects while another Sheet is still open, this unconditional cleanup restores body scrolling behind the remaining modal. Track the lock per open instance and remove `overflow-hidden` only when no Sheet still owns it.</comment>

<file context>
@@ -1,15 +1,23 @@
+  }
+
+  disconnect() {
+    document.body.classList.remove("overflow-hidden");
   }
 
</file context>

}

open() {
document.body.insertAdjacentHTML("beforeend", this.contentTarget.innerHTML)
open(e) {
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");
}
}
57 changes: 39 additions & 18 deletions gem/lib/ruby_ui/sheet/sheet_docs.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ def view_template
component = "Sheet"

div(class: "max-w-2xl mx-auto w-full py-10 space-y-10") do
render Docs::Header.new(title: "Sheet", description: "Extends the Sheet component to display content that complements the main content of the screen.")
render Docs::Header.new(title: "Sheet", description: "Extends the Dialog component to display content that complements the main content of the screen.")

Heading(level: 2) { "Usage" }

Expand All @@ -15,7 +15,7 @@ def view_template
SheetTrigger do
Button(variant: :outline) { "Open Sheet" }
end
SheetContent(class: 'sm:max-w-sm') do
SheetContent do
SheetHeader do
SheetTitle { "Edit profile" }
SheetDescription { "Make changes to your profile here. Click save when you're done." }
Expand All @@ -28,7 +28,9 @@ def view_template
Input(placeholder: "joel@drapper.me")
end
SheetFooter do
Button(variant: :outline, data: { action: 'click->ruby-ui--sheet-content#close' }) { "Cancel" }
SheetClose do
Button(variant: :outline) { "Cancel" }
end
Button(type: "submit") { "Save" }
end
end
Expand All @@ -38,27 +40,25 @@ def view_template

render Docs::VisualCodeExample.new(title: "Side", description: "Use the side property to indicate the edge of the screen where the component will appear.", context: self) do
<<~RUBY
div(class: 'grid grid-cols-2 gap-4') do
# -- TOP --
Sheet do
SheetTrigger do
Button(variant: :outline, class: 'w-full justify-center') { :top }
end
SheetContent(side: :top, class: ("sm:max-w-sm" if [:left, :right].include?(:top))) do
SheetHeader do
SheetTitle { "Edit profile" }
SheetDescription { "Make changes to your profile here. Click save when you're done." }
div(class: 'flex flex-wrap gap-2') do
[:top, :right, :bottom, :left].each do |side|
Sheet do
SheetTrigger do
Button(variant: :outline, class: 'capitalize') { side.to_s }
end
Form do
SheetContent(side: side) do
SheetHeader do
SheetTitle { "Edit profile" }
SheetDescription { "Make changes to your profile here. Click save when you're done." }
end
SheetMiddle do
label { "Name" }
Input(placeholder: "Joel Drapper") { "Joel Drapper" }

label { "Email" }
Input(placeholder: "joel@drapper.me")
end
SheetFooter do
Button(variant: :outline, data: { action: 'click->ruby-ui--sheet-content#close' }) { "Cancel" }
SheetClose do
Button(variant: :outline) { "Cancel" }
end
Button(type: "submit") { "Save" }
end
end
Expand All @@ -68,6 +68,27 @@ def view_template
RUBY
end

render Docs::VisualCodeExample.new(title: "No close button", description: "Pass show_close_button: false to hide the close button in the corner. The sheet still closes on Escape or a click outside.", context: self) do
<<~RUBY
Sheet do
SheetTrigger do
Button(variant: :outline) { "Open Sheet" }
end
SheetContent(show_close_button: false) do
SheetHeader do
SheetTitle { "No close button" }
SheetDescription { "This sheet has no close button in the corner. Press Escape or click outside to close it." }
end
SheetFooter do
SheetClose do
Button(variant: :outline) { "Close" }
end
end
end
end
RUBY
end

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

render Docs::ComponentsTable.new(component_files(component))
Expand Down
Loading