Skip to content

Commit 430a233

Browse files
djalmaaraujoclaude
andauthored
[Bug Fix] Dialog: keep closed native <dialog> hidden and sync docs controller (#343) (#458)
The native-<dialog> migration in #343 left two gaps that made dialogs render open by default in production: 1. DialogContent used a bare `flex` utility. Author CSS overrides the user-agent `dialog:not([open]) { display: none }` regardless of specificity, so a closed <dialog> was forced visible. Gate the layout on the open: variant (`open:flex`) so a closed dialog falls back to the UA display:none. 2. The docs app bundles its own copy of the Stimulus controller, which was never updated to the native-dialog version and still referenced the removed `content` target (console: "Missing target element content"), so triggers threw instead of opening the modal. Sync it with the gem's controller (dialogTarget / showModal / backdropClick). Also adds a regression test and rebuilds the MCP registry. Verified in a real browser against a local docs build: closed dialogs are hidden, the trigger opens a native modal (backdrop + scroll lock), and closing restores display:none. No console errors. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e7baee4 commit 430a233

4 files changed

Lines changed: 39 additions & 12 deletions

File tree

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { Controller } from "@hotwired/stimulus"
22

3-
// Connects to data-controller="dialog"
3+
// Connects to data-controller="ruby-ui--dialog"
44
export default class extends Controller {
5-
static targets = ["content"]
5+
static targets = ["dialog"]
66
static values = {
77
open: {
88
type: Boolean,
@@ -11,22 +11,34 @@ export default class extends Controller {
1111
}
1212

1313
connect() {
14+
this.dialogTarget.addEventListener("close", this.handleClose)
1415
if (this.openValue) {
1516
this.open()
1617
}
1718
}
1819

20+
disconnect() {
21+
this.dialogTarget.removeEventListener("close", this.handleClose)
22+
document.body.classList.remove("overflow-hidden")
23+
}
24+
1925
open(e) {
20-
e?.preventDefault();
21-
document.body.insertAdjacentHTML('beforeend', this.contentTarget.innerHTML)
22-
// prevent scroll on body
23-
document.body.classList.add('overflow-hidden')
26+
e?.preventDefault()
27+
this.dialogTarget.showModal()
28+
document.body.classList.add("overflow-hidden")
2429
}
2530

2631
dismiss() {
27-
// allow scroll on body
28-
document.body.classList.remove('overflow-hidden')
29-
// remove the element
30-
this.element.remove()
32+
this.dialogTarget.close()
33+
}
34+
35+
backdropClick(e) {
36+
if (e.target === this.dialogTarget) {
37+
this.dismiss()
38+
}
39+
}
40+
41+
handleClose = () => {
42+
document.body.classList.remove("overflow-hidden")
3143
}
3244
}

gem/lib/ruby_ui/dialog/dialog_content.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ def default_attrs
3030
data_ruby_ui__dialog_target: "dialog",
3131
data_action: "click->ruby-ui--dialog#backdropClick",
3232
class: [
33-
"fixed 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",
33+
"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",
3434
SIZES[@size]
3535
]
3636
}

gem/test/ruby_ui/dialog_test.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,21 @@ def test_dialog_content_has_backdrop_click_action
7777
assert_match(/data-action="click->ruby-ui--dialog#backdropClick"/, output)
7878
end
7979

80+
# Regression test: a closed native <dialog> must stay hidden. The bare `flex`
81+
# utility (author CSS) overrides the UA `dialog:not([open]) { display: none }`,
82+
# making the dialog always visible. Display must be gated on the open: variant.
83+
def test_dialog_content_does_not_force_display_when_closed
84+
output = phlex do
85+
RubyUI.Dialog do
86+
RubyUI.DialogContent { "Content" }
87+
end
88+
end
89+
90+
classes = output[/<dialog\b.*?\sclass="([^"]*)"/m, 1].to_s.split
91+
refute_includes classes, "flex", "Bare `flex` forces a closed <dialog> to display; use `open:flex`"
92+
assert_includes classes, "open:flex", "Dialog must apply flex only when open (open:flex)"
93+
end
94+
8095
def test_dialog_content_sizes
8196
{xs: "max-w-sm", sm: "max-w-md", md: "max-w-lg", lg: "max-w-2xl", xl: "max-w-4xl", full: "max-w-full"}.each do |size, expected_class|
8297
output = phlex do

mcp/data/registry.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1329,7 +1329,7 @@
13291329
},
13301330
{
13311331
"path": "dialog_content.rb",
1332-
"content": "# frozen_string_literal: true\n\nmodule RubyUI\n class DialogContent < Base\n SIZES = {\n xs: \"max-w-sm\",\n sm: \"max-w-md\",\n md: \"max-w-lg\",\n lg: \"max-w-2xl\",\n xl: \"max-w-4xl\",\n full: \"max-w-full\"\n }\n\n def initialize(size: :md, **attrs)\n @size = size\n super(**attrs)\n end\n\n def view_template\n dialog(**attrs) do\n yield\n close_button\n end\n end\n\n private\n\n def default_attrs\n {\n data_ruby_ui__dialog_target: \"dialog\",\n data_action: \"click->ruby-ui--dialog#backdropClick\",\n class: [\n \"fixed 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\",\n SIZES[@size]\n ]\n }\n end\n\n def close_button\n button(\n type: \"button\",\n class: \"absolute end-4 top-4 rounded-sm opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:pointer-events-none\",\n data_action: \"click->ruby-ui--dialog#dismiss\"\n ) do\n svg(\n width: \"15\",\n height: \"15\",\n viewbox: \"0 0 15 15\",\n fill: \"none\",\n xmlns: \"http://www.w3.org/2000/svg\",\n class: \"h-4 w-4\"\n ) do |s|\n s.path(\n d:\n \"M11.7816 4.03157C12.0062 3.80702 12.0062 3.44295 11.7816 3.2184C11.5571 2.99385 11.193 2.99385 10.9685 3.2184L7.50005 6.68682L4.03164 3.2184C3.80708 2.99385 3.44301 2.99385 3.21846 3.2184C2.99391 3.44295 2.99391 3.80702 3.21846 4.03157L6.68688 7.49999L3.21846 10.9684C2.99391 11.193 2.99391 11.557 3.21846 11.7816C3.44301 12.0061 3.80708 12.0061 4.03164 11.7816L7.50005 8.31316L10.9685 11.7816C11.193 12.0061 11.5571 12.0061 11.7816 11.7816C12.0062 11.557 12.0062 11.193 11.7816 10.9684L8.31322 7.49999L11.7816 4.03157Z\",\n fill: \"currentColor\",\n fill_rule: \"evenodd\",\n clip_rule: \"evenodd\"\n )\n end\n span(class: \"sr-only\") { \"Close\" }\n end\n end\n end\nend\n"
1332+
"content": "# frozen_string_literal: true\n\nmodule RubyUI\n class DialogContent < Base\n SIZES = {\n xs: \"max-w-sm\",\n sm: \"max-w-md\",\n md: \"max-w-lg\",\n lg: \"max-w-2xl\",\n xl: \"max-w-4xl\",\n full: \"max-w-full\"\n }\n\n def initialize(size: :md, **attrs)\n @size = size\n super(**attrs)\n end\n\n def view_template\n dialog(**attrs) do\n yield\n close_button\n end\n end\n\n private\n\n def default_attrs\n {\n data_ruby_ui__dialog_target: \"dialog\",\n data_action: \"click->ruby-ui--dialog#backdropClick\",\n class: [\n \"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\",\n SIZES[@size]\n ]\n }\n end\n\n def close_button\n button(\n type: \"button\",\n class: \"absolute end-4 top-4 rounded-sm opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:pointer-events-none\",\n data_action: \"click->ruby-ui--dialog#dismiss\"\n ) do\n svg(\n width: \"15\",\n height: \"15\",\n viewbox: \"0 0 15 15\",\n fill: \"none\",\n xmlns: \"http://www.w3.org/2000/svg\",\n class: \"h-4 w-4\"\n ) do |s|\n s.path(\n d:\n \"M11.7816 4.03157C12.0062 3.80702 12.0062 3.44295 11.7816 3.2184C11.5571 2.99385 11.193 2.99385 10.9685 3.2184L7.50005 6.68682L4.03164 3.2184C3.80708 2.99385 3.44301 2.99385 3.21846 3.2184C2.99391 3.44295 2.99391 3.80702 3.21846 4.03157L6.68688 7.49999L3.21846 10.9684C2.99391 11.193 2.99391 11.557 3.21846 11.7816C3.44301 12.0061 3.80708 12.0061 4.03164 11.7816L7.50005 8.31316L10.9685 11.7816C11.193 12.0061 11.5571 12.0061 11.7816 11.7816C12.0062 11.557 12.0062 11.193 11.7816 10.9684L8.31322 7.49999L11.7816 4.03157Z\",\n fill: \"currentColor\",\n fill_rule: \"evenodd\",\n clip_rule: \"evenodd\"\n )\n end\n span(class: \"sr-only\") { \"Close\" }\n end\n end\n end\nend\n"
13331333
},
13341334
{
13351335
"path": "dialog_controller.js",

0 commit comments

Comments
 (0)