Skip to content

[Feature] Dialog: use native <dialog> element (#343)#435

Merged
cirdes merged 2 commits into
mainfrom
fix/dialog-native-element
Jun 22, 2026
Merged

[Feature] Dialog: use native <dialog> element (#343)#435
cirdes merged 2 commits into
mainfrom
fix/dialog-native-element

Conversation

@djalmaaraujo

@djalmaaraujo djalmaaraujo commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Closes #343

How to reproduce the original problem

Before this change, opening the browser devtools and inspecting an open Dialog revealed the modal rendered as nested <div> elements cloned from a <template> tag — no native <dialog> semantics, no top-layer rendering, no built-in focus trap, and Esc required a manual keydown listener wired via a Stimulus action.

Steps:

  1. Visit any page on https://rubyui.com/docs/dialog
  2. Click "Open Dialog"
  3. Open browser devtools → Elements panel
  4. Observe the modal is rendered as <div data-controller="ruby-ui--dialog"> — no <dialog> element in the DOM

What changed

File Change
gem/lib/ruby_ui/dialog/dialog_content.rb Removed <template> wrapper and custom backdrop <div>; DialogContent now renders directly as a native <dialog> element with data-ruby-ui--dialog-target="dialog" and a click->ruby-ui--dialog#backdropClick action. The CSS backdrop: pseudo-element replaces the manual overlay div.
gem/lib/ruby_ui/dialog/dialog_controller.js Replaced the insertAdjacentHTML / element.remove() pattern with dialogTarget.showModal() / dialogTarget.close(). Backdrop click is handled by comparing event.target === dialogTarget. A close event listener removes overflow-hidden from the body so Esc (which fires close natively) also unlocks scroll.
gem/test/ruby_ui/dialog_test.rb Added 8 focused tests: regression test asserting <dialog> is rendered (not <div> / <template>), Stimulus target/action attributes, size variants, open value, trigger action, and close button action.

Public API is fully preserved — all component names, size variants, open: prop, and Stimulus action names (open, dismiss) remain unchanged.

Testing instructions

Automated tests

cd gem
bundle exec rake test TEST=test/ruby_ui/dialog_test.rb   # 233 tests, 1022 assertions
bundle exec rake                                          # full suite + linter

Manual browser verification (docs site)

  1. Start the docs site (cd docs && bin/dev)
  2. Navigate to /docs/dialog
  3. Click "Open Dialog"
  4. Open DevTools → Elements panel — confirm the modal is a <dialog> element (not a <div>)
  5. Press Esc — dialog should close and body scroll should be restored
  6. Click "Open Dialog" again, then click the backdrop (outside the dialog panel) — dialog should close
  7. Click "Open Dialog" again, then click the Cancel button — dialog should close
  8. While the dialog is open, confirm the page body does not scroll (overflow-hidden is applied to <body>)

Summary by cubic

Move Dialog to the native element using showModal()/close() for top-layer rendering, native backdrop, focus trap, and Esc‑to‑close while keeping the public API and Stimulus actions unchanged. Also clear body scroll lock on controller disconnect to avoid stuck scrolling during Turbo navigation.

  • Refactors

    • Native <dialog>: DialogContent renders <dialog> with data-ruby-ui--dialog-target="dialog" and click->ruby-ui--dialog#backdropClick; controller uses showModal()/close() and listens for close to restore body scroll.
    • Removed template cloning and overlay; styling moved to backdrop:/open: utilities; tests cover native <dialog>, attributes, size variants, and open/dismiss actions.
  • Bug Fixes

    • Clear overflow-hidden in disconnect() so navigating with Turbo while a dialog is open doesn’t leave the page locked; rebuilt MCP registry to include native-dialog changes.

Written for commit 9290140. Summary will update on new commits.

Review in cubic

Replace the template-clone pattern with a native <dialog> element.
DialogContent now renders as <dialog> opened via showModal() and closed
via close(), gaining free top-layer rendering, native backdrop, built-in
focus trapping, and Esc-to-close. Body scroll-lock is preserved. All
existing public component API and Stimulus controller actions are
backward-compatible.
@djalmaaraujo djalmaaraujo requested a review from cirdes as a code owner June 22, 2026 20:08

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found across 3 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread gem/lib/ruby_ui/dialog/dialog_controller.js
…P registry (#435)

Clear overflow-hidden from body in disconnect() so Turbo navigation while
a dialog is open does not leave a permanent scroll lock. Also rebuilt the
MCP registry to include the native-dialog changes from this PR.
@djalmaaraujo

Copy link
Copy Markdown
Contributor Author

Addressed the code review finding from cubic (confidence 8): added document.body.classList.remove("overflow-hidden") to disconnect() in dialog_controller.js. This ensures that if Turbo navigation tears down the controller while a dialog is open, the body scroll lock is always cleared — not just when the close event fires. Also rebuilt the MCP registry to incorporate the native-dialog component changes.

@cirdes cirdes left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Really clean PR. Native <dialog> is the right move!

@cirdes cirdes merged commit 30fd12b into main Jun 22, 2026
8 checks passed
@cirdes cirdes deleted the fix/dialog-native-element branch June 22, 2026 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make Dialog use <dialog>?

2 participants