Turbo Morph with Dialog causes issues

[htcarr3]

When a <dialog> element is opened modally with .showModal(), the browser moves it into the top layer. Morphing seems to reload the content on the page, but does not reset the top layer. From the user's perspective, nothing on the page is interactive because everything is stuck underneath the top layer (even though the top layer is empty). I'm not sure what the solution to this is given there is no api to interact directly with the top layer. Maybe this can be solved with event listeners to either skip or .close() these sorts of elements.

[seanpdoyle]

@htcarr3 thank you for opening this issue. I test this locally by making the following changes to Turbo's test suite:

diff --git a/src/tests/fixtures/page_refresh.html b/src/tests/fixtures/page_refresh.html
index c058667..1ba8e92 100644
--- a/src/tests/fixtures/page_refresh.html
+++ b/src/tests/fixtures/page_refresh.html
@@ -82,6 +82,11 @@
 
     <a href="/src/tests/fixtures/page_refresh.html" id="reload-link">Reload</a>
 
+    <dialog id="modal">
+      Opened
+      <a href="/src/tests/fixtures/page_refresh.html">Reload</a>
+    </dialog>
+
     <turbo-frame id="refresh-morph" src="/src/tests/fixtures/frame_refresh_morph.html" refresh="morph">
       <h2>Frame to be morphed</h2>
     </turbo-frame>
diff --git a/src/tests/functional/page_refresh_tests.js b/src/tests/functional/page_refresh_tests.js
index c5c116c..a2bce0b 100644
--- a/src/tests/functional/page_refresh_tests.js
+++ b/src/tests/functional/page_refresh_tests.js
@@ -337,6 +337,18 @@ test("doesn't render previews when morphing", async ({ page }) => {
   assert.equal(await title.textContent(), "Page to be refreshed")
 })
 
+test("closes modal dialogs when morphing", async ({ page }) => {
+  await page.goto("/src/tests/fixtures/page_refresh.html")
+  const dialog = page.locator("dialog#modal")
+  const input = page.locator("input[name=query]")
+
+  await dialog.evaluate((dialog) => dialog.showModal())
+  await page.click("dialog#modal a")
+  await input.fill("element is interactive")
+
+  await expect(input).toBeFocused()
+})
+
 async function assertPageScroll(page, top, left) {
   const [scrollTop, scrollLeft] = await page.evaluate(() => {
     return [

Unfortunately, the test passed without any implementation changes, so I wasn't able to reproduce the unexpected behavior.

Could you share a reproducible test case? I've shared a Rails bug script template below. Could you modify it so that it mimics the behavior your application is exhibiting?

Copy-paste this into a `bug.rb` file, then execute `ruby bug.rb`

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails"
  gem "propshaft"
  gem "puma"
  gem "sqlite3"
  gem "turbo-rails"

  gem "capybara"
  gem "cuprite", "~> 0.9", require: "capybara/cuprite"
end

ENV["DATABASE_URL"] = "sqlite3::memory:"
ENV["RAILS_ENV"] = "test"

require "active_record/railtie"
# require "active_storage/engine"
require "action_controller/railtie"
require "action_view/railtie"
# require "action_mailer/railtie"
# require "active_job/railtie"
require "action_cable/engine"
# require "action_mailbox/engine"
# require "action_text/engine"
require "rails/test_unit/railtie"

class App < Rails::Application
  config.load_defaults Rails::VERSION::STRING.to_f

  config.root = __dir__
  config.hosts << "example.org"
  config.eager_load = false
  config.session_store :cookie_store, key: "cookie_store_key"
  config.secret_key_base = "secret_key_base"
  config.consider_all_requests_local = true
  config.action_cable.cable = {"adapter" => "async"}
  config.turbo.draw_routes = false

  Rails.logger = config.logger = Logger.new($stdout)

  routes.append do
    root to: "application#index"
  end
end

Rails.application.initialize!

ActiveRecord::Schema.define do
  create_table :messages, force: true do |t|
    t.text :body, null: false
  end
end

class Message < ActiveRecord::Base
end

class ApplicationController < ActionController::Base
  include Rails.application.routes.url_helpers

  class_attribute :template, default: DATA.read

  def index
    render inline: template, formats: :html
  end
end

class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
  driven_by :cuprite, using: :chrome, screen_size: [1400, 1400], options: { js_errors: true }
end

Capybara.configure do |config|
  config.server = :puma, {Silent: true}
  config.default_normalize_ws = true
end

require "rails/test_help"

class TurboSystemTest < ApplicationSystemTestCase
  test "reproduces bug" do
    visit root_path

    assert_text "Loaded with Turbo"
  end
end

__END__

<!DOCTYPE html>
<html>
  <head>
    <%= csrf_meta_tags %>

    <script type="importmap">
      {
        "imports": {
          "@hotwired/turbo-rails": "<%= asset_path("turbo.js") %>"
        }
      }
    </script>

    <script type="module">
      import "@hotwired/turbo-rails"

      addEventListener("turbo:load", () => document.body.innerHTML = "Loaded with Turbo")
    </script>
  </head>

  <body>Loaded without Turbo</body>
</html>

[JudeLawrence]

I've just come across this same issue. It seems that when you submit a form inside of a dialog element, the element is closed but the top layer is still there so your cursor cannot interact with the page.

I'm not sure how capybara works, but would it be able to pick up that an element cannot be clicked because a top_layer is open?

[htcarr3]

@JudeLawrence thank you for chiming in! My example is very similar. I spent about an hour trying to set up a reproducible test case for this, but wasn't successful and haven't had time to check back on this. We added a somewhat hacky-feeliing solution to listen for turbo-submit events and closing any open dialog elements, but eventually we ended up moving away from dialog elements and implementing some custom modal elements using stimulus (avoiding the #top-layer entirely)

[pbstriker38]

@JudeLawrence
I verified that Capybara can not click an element behind the dialog top-layer. I can not reproduce this either even though it's definitely happening in our app.

require 'bundler/inline'

gemfile(true) do
  source 'https://rubygems.org'

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem 'rails'
  gem 'propshaft'
  gem 'puma'
  gem 'sqlite3'
  gem 'turbo-rails'

  gem 'capybara'
  gem 'cuprite', '~> 0.9', require: 'capybara/cuprite'
end

ENV['DATABASE_URL'] = 'sqlite3::memory:'
ENV['RAILS_ENV'] = 'test'

require 'active_record/railtie'
# require "active_storage/engine"
require 'action_controller/railtie'
require 'action_view/railtie'
# require "action_mailer/railtie"
# require "active_job/railtie"
require 'action_cable/engine'
# require "action_mailbox/engine"
# require "action_text/engine"
require 'rails/test_unit/railtie'

class App < Rails::Application
  config.load_defaults Rails::VERSION::STRING.to_f

  config.root = __dir__
  config.hosts << 'example.org'
  config.eager_load = false
  config.session_store :cookie_store, key: 'cookie_store_key'
  config.secret_key_base = 'secret_key_base'
  config.consider_all_requests_local = true
  config.action_cable.cable = { 'adapter' => 'async' }
  config.turbo.draw_routes = false

  Rails.logger = config.logger = Logger.new($stdout)

  routes.append do
    root to: 'application#index'

    post '/create', to: 'application#create'
  end
end

Rails.application.initialize!

ActiveRecord::Schema.define do
  create_table :messages, force: true do |t|
    t.text :body, null: false
  end
end

class Message < ActiveRecord::Base
end

class ApplicationController < ActionController::Base
  include Rails.application.routes.url_helpers

  class_attribute :template, default: DATA.read

  def index
    render inline: template, formats: :html
  end

  def create
    redirect_to root_path
  end
end

class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
  driven_by :cuprite, using: :chrome, screen_size: [1400, 1400], options: { js_errors: true, inspector: true }
end

Capybara.configure do |config|
  config.server = :puma, { Silent: true }
  config.default_normalize_ws = true
end

require 'rails/test_help'

class TurboSystemTest < ApplicationSystemTestCase
  test 'reproduces bug' do
    visit root_path(open_dialog: true)

    assert_raise(Capybara::Cuprite::MouseEventFailed) { click_button 'hello' }

    # page.driver.debug(binding)

    click_button 'create'

    assert_nothing_raised { click_button 'hello' }
  end
end

__END__

<!DOCTYPE html>
<html>
  <head>
    <%= csrf_meta_tags %>

    <script type="importmap">
      {
        "imports": {
          "@hotwired/turbo-rails": "<%= asset_path("turbo.js") %>"
        }
      }
    </script>

    <script type="module">
      import "@hotwired/turbo-rails"

      addEventListener("turbo:load", () => {
        const searchParams = new URLSearchParams(window.location.search);
        if (searchParams.get('open_dialog') === 'true') {
          const dialog = document.querySelector("dialog");
          dialog.showModal();
        }
      })
    </script>

    <%= turbo_refreshes_with method: :morph, scroll: :preserve %>
    <%= yield :head %>
  </head>

  <body>
    <button>hello</button>
    <dialog>
      <%= form_with url: '/create' do |form| %>
        <%= form.submit 'create' %>
      <% end %>
    </dialog>
  </body>
</html>

[JudeLawrence]

I've been playing around with the test file as well, and cannot get it to replicate the issue. The #top-layer gets removed after the form submission in testing but not in real browser use, and I cannot figure out why.

I'm going to turn the response in to a turbo_stream to get around this issue, but if anyone figures out how to replicate this in tests / fix this issue, it would be much appreciated.

[4lllex]

I think I got a reproducible test:

require "bundler/inline"

gemfile true do
  source "https://rubygems.org"

  gem "rails"
  gem "propshaft"
  gem "puma"
  gem "sqlite3"
  gem "turbo-rails"

  gem "capybara"
  gem "cuprite", "~> 0.9", require: "capybara/cuprite"
end

ENV["DATABASE_URL"] = "sqlite3::memory:"
ENV["RAILS_ENV"] = "test"

require "active_record/railtie"
# require "active_storage/engine"
require "action_controller/railtie"
require "action_view/railtie"
# require "action_mailer/railtie"
# require "active_job/railtie"
require "action_cable/engine"
# require "action_mailbox/engine"
# require "action_text/engine"
require "rails/test_unit/railtie"

class App < Rails::Application
  config.load_defaults Rails::VERSION::STRING.to_f

  config.root = __dir__
  config.hosts << "example.org"
  config.eager_load = false
  config.session_store :cookie_store, key: "cookie_store_key"
  config.secret_key_base = "secret_key_base"
  config.consider_all_requests_local = true
  config.action_cable.cable = { "adapter" => "async" }
  config.turbo.draw_routes = false

  Rails.logger = config.logger = Logger.new($stdout)

  routes.append do
    root to: "application#index"
    post "/create", to: "application#create"
  end
end

Rails.application.initialize!

class ApplicationController < ActionController::Base
  include Rails.application.routes.url_helpers

  class_attribute :template, default: DATA.read

  def index
    render inline: template, formats: :html
  end

  def create
    render turbo_stream: turbo_stream.action(:refresh, nil)
  end
end

class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
  driven_by :cuprite,
    using: :chrome,
    screen_size: [1400, 1400],
    options: {
      js_errors: true,
      headless: true
    }
end

Capybara.configure do |config|
  config.server = :puma, { Silent: true }
  config.default_normalize_ws = true
end

require "rails/test_help"

class TurboSystemTest < ApplicationSystemTestCase
  test "reproduces bug" do
    visit "/"

    click_button "open modal"
    assert_raise(Capybara::Cuprite::MouseEventFailed) { click_button "hello" }

    click_button "create"
    assert_nothing_raised { click_button "hello" }
  end
end

__END__

<!DOCTYPE html>
<html>
  <head>
    <%= csrf_meta_tags %>
    <script type="importmap" data-turbo-track="reload">
      { "imports": { "@hotwired/turbo-rails": "<%= asset_path("turbo.js") %>" } }
    </script>
    <script type="module">
      import "@hotwired/turbo-rails"
    </script>
    <meta name="turbo-refresh-method" content="morph">
  </head>
  <body>
    <button>hello</button>

    <dialog>
      <%= form_with url: "/create" do |form| %>
        <%= form.submit "create" %>
      <% end %>
    </dialog>

    <button onclick="document.querySelector('dialog').showModal();">open modal</button>
  </body>
</html>

[seanpdoyle]

@4lllex thank you for sharing that reproduction script!

I was able to get it passing with the following change:

diff --git a/bug.rb b/bug.rb
index 0fe125c..9beff4e 100644
--- a/bug.rb
+++ b/bug.rb
@@ -103,6 +103,15 @@ __END__
     </script>
     <script type="module">
       import "@hotwired/turbo-rails"
+
+      addEventListener("turbo:before-morph-attribute", (event) => {
+        if (event.target instanceof HTMLDialogElement &&
+          event.detail.attributeName === "open" &&
+          event.detail.mutationType === "remove") {
+          event.preventDefault()
+          event.target.close()
+        }
+      })
     </script>
     <meta name="turbo-refresh-method" content="morph">
   </head>

Distilled into a code snippet, it might be worth considering a global event listener:

addEventListener("turbo:before-morph-attribute", (event) => {
  const { target, detail: { attributeName, mutationType } } = event

  if (target instanceof HTMLDialogElement && attributeName === "open" && mutationType === "remove") {
    event.preventDefault()
    target.close()
  }
})

I'm surprised that the removal of the [open] attribute does not automatically trigger an implicit call to HTMLDialogElement.close().

[4lllex]

@seanpdoyle oh, morphing just removes the open attribute, makes a lot of sense in retrospect. The spec is a lot clearer than MDN about this behavior, especially for modals:

Removing the open attribute will usually hide the dialog. However, doing so has a number of strange additional consequences:

• The close event will not be fired.
• The close() method, and any close requests, will no longer be able to close the dialog.
• If the dialog was shown using its showModal() method, the Document will still be blocked.

For these reasons, it is generally better to never remove the open attribute manually. Instead, use the close() method to close the dialog, or the hidden attribute to hide it.

https://html.spec.whatwg.org/multipage/interactive-elements.html#note-dialog-remove-open-attribute

https://html.spec.whatwg.org/multipage/interactive-elements.html#note-dialog-method-names

[seanpdoyle]

@jorgemanrubia @brunoprietog given the quirks highlighted by the specification does building-in a global turbo:before-attribute-morph listener targeting <dialog> elements feel appropriate?

[brunoprietog]

I think so. It's good to encourage the use of the dialog element, and Turbo is already aware of some of these peculiarities in other places.