add examples for bugs reported with quick navigations and extra fetches #1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The problem --- Programmatically driving a `<turbo-frame>` element when its `[src]` attribute changes is a suitable end-user experience in consumer applications. It's a fitting black-box interface for the outside world: change the value of the attribute and let Turbo handle the rest. However, internally, it's a lossy abstraction. For example, when the `FrameRedirector` class listens for page-wide `click` and `submit` events, it determines if their targets are meant to drive a `<turbo-frame>` element by: 1. finding an element that matches a clicked `<a>` element's `[data-turbo-frame]` attribute 2. finding an element that matches a submitted `<form>` element's `[data-turbo-frame]` attribute 3. finding an element that matches a submitted `<form>` element's _submitter's_ `[data-turbo-frame]` attribute 4. finding the closest `<turbo-frame>` ancestor to the `<a>` or `<form>` Once it finds the matching frame element, it disposes of all that additional context and navigates the `<turbo-frame>` by updating its `[src]` attribute. This makes it impossible to control various aspects of the frame navigation (like its "rendering" explored in [hotwired#146][]) outside of its destination URL. Similarly, since a `<form>` and submitter pairing have an impact on which `<turbo-frame>` is navigated, the `FrameController` implementation passes around a `HTMLFormElement` and `HTMLSubmitter?` data clump and constantly re-fetches a matching `<turbo-frame>` instance. Outside of frames, page-wide navigation is driven by a `Visit` instance that manages the HTTP life cycle and delegates along the way to a `VisitDelegate`. It also pairs calls to visit with a `VisitOption` object to capture additional context. The proposal --- This commit introduces the `FrameVisit` class. It serves as an encapsulation of the `FetchRequest` and `FormSubmission` lifecycle events involved in navigating a frame. It's implementation draws inspiration from the `Visit`, `VisitDelegate`, and `VisitOptions` pairing. Since the `FrameVisit` knows how to unify both `FetchRequest` and `FormSubmission` hooks, the resulting callbacks fired from within the `FrameController` are flat and consistent. Extra benefits --- The biggest benefit is the introduction of a DRY abstraction to manage the behind the scenes HTTP calls necessary to drive a `<turbo-frame>`. With the introduction of the `FrameVisit` concept, we can also declare a `visit()` and `submit()` method for `FrameElementDelegate` implementations in the place of other implementation-specific methods like `loadResponse()` and `formSubmissionIntercepted()`. In addition, these changes have the potential to close [hotwired#326][], since we can consistently invoke `loadResponse()` across `<a>`-click-initiated and `<form>`-submission-initiated visits. To ensure that's the case, this commit adds test coverage for navigating a `<turbo-frame>` by making a `GET` request to an endpoint that responds with a `500` status. [hotwired#146]: hotwired#146 [hotwired#326]: hotwired#326
The issues outlined by [hotwired#793][] are resolved by the introduction of the `FrameVisit` object and its lifecycle. To ensure that behavior is fixed, this commit cherry-picks the test coverage introduced in that branch. [hotwired#793]: hotwired#793
seanpdoyle
force-pushed
the
frame-visit-delegate
branch
2 times, most recently
from
00daeee to
d10b72b
Compare
|
I think you reverted some commits after I created this PR 🤔 |
seanpdoyle
force-pushed
the
frame-visit-delegate
branch
5 times, most recently
from
a2e2965 to
b4d01ff
Compare
seanpdoyle
force-pushed
the
frame-visit-delegate
branch
2 times, most recently
from
c8dbbab to
f8c260d
Compare
seanpdoyle
force-pushed
the
frame-visit-delegate
branch
from
f8c260d to
23a858c
Compare
seanpdoyle
force-pushed
the
frame-visit-delegate
branch
from
23a858c to
44592f1
Compare
seanpdoyle
force-pushed
the
frame-visit-delegate
branch
from
44592f1 to
4a361a1
Compare
seanpdoyle
force-pushed
the
frame-visit-delegate
branch
2 times, most recently
from
3415576 to
e190d11
Compare
seanpdoyle
force-pushed
the
frame-visit-delegate
branch
from
e190d11 to
fa46afc
Compare
seanpdoyle
force-pushed
the
frame-visit-delegate
branch
2 times, most recently
from
845459d to
5700105
Compare
seanpdoyle
force-pushed
the
frame-visit-delegate
branch
from
5700105 to
4cdce4f
Compare
seanpdoyle
added a commit
that referenced
this pull request
Mar 27, 2024
The scenario
---
Imagine a List-Details style page layout, with navigation links on the
left and the contents of the page on the right:
```html
<turbo-frame id="list">
<a href="/articles/1" data-turbo-frame="details">
Article #1
<turbo-frame id="preview_article_1" src="/articles/1/preview">
Some preview text
</turbo-frame>
</a>
<!-- ... -->
<a href="/?page=2">Next Page</a>
</turbo-frame>
<turbo-frame id="details">
<h1>Details</h1>
</turbo-frame>
```
The `#list` element is a `<turbo-frame>` to handle pagination, and the
`#details` element is a `<turbo-frame>` as well. The `<a>` elements
within the `#list` frame drive the `#details` frame through their
`[data-turbo-frame="details"]`. The `<a>` element nests a third kind of
`<turbo-frame>` that is specific to the resource being linked to. It
asynchronously loads in preview content with a `[src]` attribute.
The problem
---
Clicking the `Article #1` text within the `<a>` element drives the
`#details` frame as expected.
However, clicking the `Some preview text` within the `<a>` element's
nested `<turbo-frame>` element navigates the entire page.
This is demonstrated in the following reproduction script:
```ruby
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 "action_controller/railtie"
require "action_view/railtie"
require "action_cable/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,
headless: false
}
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
binding.irb
click_link "Drive #details to ?key=1"
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"
</script>
<style>
body {
display: grid;
grid-template-areas:
"list details"
"list details";
grid-template-columns: 150px 1fr;
grid-template-rows: 50px 1fr;
height: 100vh;
}
#list {
display: flex;
flex-direction: column;
grid-area: list;
overflow-y: scroll;
}
#details {
grid-area: details;
}
</style>
<meta name="turbo-prefetch" content="false">
</head>
<body>
<turbo-frame id="list">
<% 1.upto(5).each do |key| %>
<%= link_to({key:}, data: {turbo_frame: "details"}) do %>
<turbo-frame id="frame_<%= key %>">
Drive #details to <%= {key:}.to_query %>
</turbo-frame>
<% end %>
<% end %>
</turbo-frame>
<turbo-frame id="details">
<%= params.fetch(:key, "0") %>
</turbo-frame>
</body>
</html>
```
The solution
---
When observing `click` events, utilize the `findLinkFromClickTarget` to
find the nearest `<a>` element to the `click` target so that **that**
element's ancestors are used to determine which `<turbo-frame>` to
target instead of risking the possibility of using one of its
**descendants**.
seanpdoyle
added a commit
that referenced
this pull request
Mar 27, 2024
The scenario
---
Imagine a List-Details style page layout, with navigation links on the
left and the contents of the page on the right:
```html
<turbo-frame id="list">
<a href="/articles/1" data-turbo-frame="details">
Article #1
<turbo-frame id="preview_article_1" src="/articles/1/preview">
Some preview text
</turbo-frame>
</a>
<!-- ... -->
<a href="/?page=2">Next Page</a>
</turbo-frame>
<turbo-frame id="details">
<h1>Details</h1>
</turbo-frame>
```
The `#list` element is a `<turbo-frame>` to handle pagination, and the
`#details` element is a `<turbo-frame>` as well. The `<a>` elements
within the `#list` frame drive the `#details` frame through their
`[data-turbo-frame="details"]`. The `<a>` element nests a third kind of
`<turbo-frame>` that is specific to the resource being linked to. It
asynchronously loads in preview content with a `[src]` attribute.
The problem
---
Clicking the `Article #1` text within the `<a>` element drives the
`#details` frame as expected.
However, clicking the `Some preview text` within the `<a>` element's
nested `<turbo-frame>` element navigates the entire page.
This is demonstrated in the following reproduction script:
```ruby
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 "action_controller/railtie"
require "action_view/railtie"
require "action_cable/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,
headless: false
}
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
binding.irb
click_link "Drive #details to ?key=1"
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"
</script>
<style>
body {
display: grid;
grid-template-areas:
"list details"
"list details";
grid-template-columns: 150px 1fr;
grid-template-rows: 50px 1fr;
height: 100vh;
}
#list {
display: flex;
flex-direction: column;
grid-area: list;
overflow-y: scroll;
}
#details {
grid-area: details;
}
</style>
<meta name="turbo-prefetch" content="false">
</head>
<body>
<turbo-frame id="list">
<% 1.upto(5).each do |key| %>
<%= link_to({key:}, data: {turbo_frame: "details"}) do %>
<turbo-frame id="frame_<%= key %>">
Drive #details to <%= {key:}.to_query %>
</turbo-frame>
<% end %>
<% end %>
</turbo-frame>
<turbo-frame id="details">
<%= params.fetch(:key, "0") %>
</turbo-frame>
</body>
</html>
```
The solution
---
When observing `click` events, utilize the `findLinkFromClickTarget` to
find the nearest `<a>` element to the `click` target so that **that**
element's ancestors are used to determine which `<turbo-frame>` to
target instead of risking the possibility of using one of its
**descendants**.
seanpdoyle
added a commit
that referenced
this pull request
Mar 27, 2024
The scenario
---
Imagine a List-Details style page layout, with navigation links on the
left and the contents of the page on the right:
```html
<turbo-frame id="list">
<a href="/articles/1" data-turbo-frame="details">
Article #1
<turbo-frame id="preview_article_1" src="/articles/1/preview">
Some preview text
</turbo-frame>
</a>
<!-- ... -->
<a href="/?page=2">Next Page</a>
</turbo-frame>
<turbo-frame id="details">
<h1>Details</h1>
</turbo-frame>
```
The `#list` element is a `<turbo-frame>` to handle pagination, and the
`#details` element is a `<turbo-frame>` as well. The `<a>` elements
within the `#list` frame drive the `#details` frame through their
`[data-turbo-frame="details"]`. The `<a>` element nests a third kind of
`<turbo-frame>` that is specific to the resource being linked to. It
asynchronously loads in preview content with a `[src]` attribute.
The problem
---
Clicking the `Article #1` text within the `<a>` element drives the
`#details` frame as expected.
However, clicking the `Some preview text` within the `<a>` element's
nested `<turbo-frame>` element navigates the entire page.
This is demonstrated in the following reproduction script:
```ruby
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 "action_controller/railtie"
require "action_view/railtie"
require "action_cable/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,
headless: false
}
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
binding.irb
click_link "Drive #details to ?key=1"
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"
</script>
<style>
body {
display: grid;
grid-template-areas:
"list details"
"list details";
grid-template-columns: 150px 1fr;
grid-template-rows: 50px 1fr;
height: 100vh;
}
#list {
display: flex;
flex-direction: column;
grid-area: list;
overflow-y: scroll;
}
#details {
grid-area: details;
}
</style>
<meta name="turbo-prefetch" content="false">
</head>
<body>
<turbo-frame id="list">
<% 1.upto(5).each do |key| %>
<%= link_to({key:}, data: {turbo_frame: "details"}) do %>
<turbo-frame id="frame_<%= key %>">
Drive #details to <%= {key:}.to_query %>
</turbo-frame>
<% end %>
<% end %>
</turbo-frame>
<turbo-frame id="details">
<%= params.fetch(:key, "0") %>
</turbo-frame>
</body>
</html>
```
The solution
---
When observing `click` events, utilize the `findLinkFromClickTarget` to
find the nearest `<a>` element to the `click` target so that **that**
element's ancestors are used to determine which `<turbo-frame>` to
target instead of risking the possibility of using one of its
**descendants**.
seanpdoyle
added a commit
that referenced
this pull request
Mar 27, 2024
The scenario
---
Imagine a List-Details style page layout, with navigation links on the
left and the contents of the page on the right:
```html
<turbo-frame id="list">
<a href="/articles/1" data-turbo-frame="details">
Article #1
<turbo-frame id="preview_article_1" src="/articles/1/preview">
Some preview text
</turbo-frame>
</a>
<!-- ... -->
<a href="/?page=2">Next Page</a>
</turbo-frame>
<turbo-frame id="details">
<h1>Details</h1>
</turbo-frame>
```
The `#list` element is a `<turbo-frame>` to handle pagination, and the
`#details` element is a `<turbo-frame>` as well. The `<a>` elements
within the `#list` frame drive the `#details` frame through their
`[data-turbo-frame="details"]`. The `<a>` element nests a third kind of
`<turbo-frame>` that is specific to the resource being linked to. It
asynchronously loads in preview content with a `[src]` attribute.
The problem
---
Clicking the `Article #1` text within the `<a>` element drives the
`#details` frame as expected.
However, clicking the `Some preview text` within the `<a>` element's
nested `<turbo-frame>` element navigates the entire page.
This is demonstrated in the following reproduction script:
```ruby
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 "action_controller/railtie"
require "action_view/railtie"
require "action_cable/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,
headless: false
}
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
binding.irb
click_link "Drive #details to ?key=1"
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"
</script>
<style>
body {
display: grid;
grid-template-areas:
"list details"
"list details";
grid-template-columns: 150px 1fr;
grid-template-rows: 50px 1fr;
height: 100vh;
}
#list {
display: flex;
flex-direction: column;
grid-area: list;
overflow-y: scroll;
}
#details {
grid-area: details;
}
</style>
<meta name="turbo-prefetch" content="false">
</head>
<body>
<turbo-frame id="list">
<% 1.upto(5).each do |key| %>
<%= link_to({key:}, data: {turbo_frame: "details"}) do %>
<turbo-frame id="frame_<%= key %>">
Drive #details to <%= {key:}.to_query %>
</turbo-frame>
<% end %>
<% end %>
</turbo-frame>
<turbo-frame id="details">
<%= params.fetch(:key, "0") %>
</turbo-frame>
</body>
</html>
```
The solution
---
When observing `click` events, utilize the `findLinkFromClickTarget` to
find the nearest `<a>` element to the `click` target so that **that**
element's ancestors are used to determine which `<turbo-frame>` to
target instead of risking the possibility of using one of its
**descendants**.
seanpdoyle
added a commit
that referenced
this pull request
Mar 27, 2024
The scenario
---
Imagine a List-Details style page layout, with navigation links on the
left and the contents of the page on the right:
```html
<turbo-frame id="list">
<a href="/articles/1" data-turbo-frame="details">
Article #1
<turbo-frame id="preview_article_1" src="/articles/1/preview">
Some preview text
</turbo-frame>
</a>
<!-- ... -->
<a href="/?page=2">Next Page</a>
</turbo-frame>
<turbo-frame id="details">
<h1>Details</h1>
</turbo-frame>
```
The `#list` element is a `<turbo-frame>` to handle pagination, and the
`#details` element is a `<turbo-frame>` as well. The `<a>` elements
within the `#list` frame drive the `#details` frame through their
`[data-turbo-frame="details"]`. The `<a>` element nests a third kind of
`<turbo-frame>` that is specific to the resource being linked to. It
asynchronously loads in preview content with a `[src]` attribute.
The problem
---
Clicking the `Article #1` text within the `<a>` element drives the
`#details` frame as expected.
However, clicking the `Some preview text` within the `<a>` element's
nested `<turbo-frame>` element navigates the entire page.
This is demonstrated in the following reproduction script:
```ruby
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 "action_controller/railtie"
require "action_view/railtie"
require "action_cable/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,
headless: false
}
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
binding.irb
click_link "Drive #details to ?key=1"
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"
</script>
<style>
body {
display: grid;
grid-template-areas:
"list details"
"list details";
grid-template-columns: 150px 1fr;
grid-template-rows: 50px 1fr;
height: 100vh;
}
#list {
display: flex;
flex-direction: column;
grid-area: list;
overflow-y: scroll;
}
#details {
grid-area: details;
}
</style>
<meta name="turbo-prefetch" content="false">
</head>
<body>
<turbo-frame id="list">
<% 1.upto(5).each do |key| %>
<%= link_to({key:}, data: {turbo_frame: "details"}) do %>
<turbo-frame id="frame_<%= key %>">
Drive #details to <%= {key:}.to_query %>
</turbo-frame>
<% end %>
<% end %>
</turbo-frame>
<turbo-frame id="details">
<%= params.fetch(:key, "0") %>
</turbo-frame>
</body>
</html>
```
The solution
---
When observing `click` events, utilize the `findLinkFromClickTarget` to
find the nearest `<a>` element to the `click` target so that **that**
element's ancestors are used to determine which `<turbo-frame>` to
target instead of risking the possibility of using one of its
**descendants**.
seanpdoyle
added a commit
that referenced
this pull request
Mar 27, 2024
The scenario
---
Imagine a List-Details style page layout, with navigation links on the
left and the contents of the page on the right:
```html
<turbo-frame id="list">
<a href="/articles/1" data-turbo-frame="details">
Article #1
<turbo-frame id="preview_article_1" src="/articles/1/preview">
Some preview text
</turbo-frame>
</a>
<!-- ... -->
<a href="/?page=2">Next Page</a>
</turbo-frame>
<turbo-frame id="details">
<h1>Details</h1>
</turbo-frame>
```
The `#list` element is a `<turbo-frame>` to handle pagination, and the
`#details` element is a `<turbo-frame>` as well. The `<a>` elements
within the `#list` frame drive the `#details` frame through their
`[data-turbo-frame="details"]`. The `<a>` element nests a third kind of
`<turbo-frame>` that is specific to the resource being linked to. It
asynchronously loads in preview content with a `[src]` attribute.
The problem
---
Clicking the `Article #1` text within the `<a>` element drives the
`#details` frame as expected.
However, clicking the `Some preview text` within the `<a>` element's
nested `<turbo-frame>` element navigates the entire page.
This is demonstrated in the following reproduction script:
```ruby
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 "action_controller/railtie"
require "action_view/railtie"
require "action_cable/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,
headless: false
}
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
binding.irb
click_link "Drive #details to ?key=1"
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"
</script>
<style>
body {
display: grid;
grid-template-areas:
"list details"
"list details";
grid-template-columns: 150px 1fr;
grid-template-rows: 50px 1fr;
height: 100vh;
}
#list {
display: flex;
flex-direction: column;
grid-area: list;
overflow-y: scroll;
}
#details {
grid-area: details;
}
</style>
<meta name="turbo-prefetch" content="false">
</head>
<body>
<turbo-frame id="list">
<% 1.upto(5).each do |key| %>
<%= link_to({key:}, data: {turbo_frame: "details"}) do %>
<turbo-frame id="frame_<%= key %>">
Drive #details to <%= {key:}.to_query %>
</turbo-frame>
<% end %>
<% end %>
</turbo-frame>
<turbo-frame id="details">
<%= params.fetch(:key, "0") %>
</turbo-frame>
</body>
</html>
```
The solution
---
When observing `click` events, utilize the `findLinkFromClickTarget` to
find the nearest `<a>` element to the `click` target so that **that**
element's ancestors are used to determine which `<turbo-frame>` to
target instead of risking the possibility of using one of its
**descendants**.
seanpdoyle
added a commit
that referenced
this pull request
Mar 27, 2024
The scenario
---
Imagine a List-Details style page layout, with navigation links on the
left and the contents of the page on the right:
```html
<turbo-frame id="list">
<a href="/articles/1" data-turbo-frame="details">
Article #1
<turbo-frame id="preview_article_1" src="/articles/1/preview">
Some preview text
</turbo-frame>
</a>
<!-- ... -->
<a href="/?page=2">Next Page</a>
</turbo-frame>
<turbo-frame id="details">
<h1>Details</h1>
</turbo-frame>
```
The `#list` element is a `<turbo-frame>` to handle pagination, and the
`#details` element is a `<turbo-frame>` as well. The `<a>` elements
within the `#list` frame drive the `#details` frame through their
`[data-turbo-frame="details"]`. The `<a>` element nests a third kind of
`<turbo-frame>` that is specific to the resource being linked to. It
asynchronously loads in preview content with a `[src]` attribute.
The problem
---
Clicking the `Article #1` text within the `<a>` element drives the
`#details` frame as expected.
However, clicking the `Some preview text` within the `<a>` element's
nested `<turbo-frame>` element navigates the entire page.
This is demonstrated in the following reproduction script:
```ruby
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 "action_controller/railtie"
require "action_view/railtie"
require "action_cable/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,
headless: false
}
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
binding.irb
click_link "Drive #details to ?key=1"
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"
</script>
<style>
body {
display: grid;
grid-template-areas:
"list details"
"list details";
grid-template-columns: 150px 1fr;
grid-template-rows: 50px 1fr;
height: 100vh;
}
#list {
display: flex;
flex-direction: column;
grid-area: list;
overflow-y: scroll;
}
#details {
grid-area: details;
}
</style>
<meta name="turbo-prefetch" content="false">
</head>
<body>
<turbo-frame id="list">
<% 1.upto(5).each do |key| %>
<%= link_to({key:}, data: {turbo_frame: "details"}) do %>
<turbo-frame id="frame_<%= key %>">
Drive #details to <%= {key:}.to_query %>
</turbo-frame>
<% end %>
<% end %>
</turbo-frame>
<turbo-frame id="details">
<%= params.fetch(:key, "0") %>
</turbo-frame>
</body>
</html>
```
The solution
---
When observing `click` events, utilize the `findLinkFromClickTarget` to
find the nearest `<a>` element to the `click` target so that **that**
element's ancestors are used to determine which `<turbo-frame>` to
target instead of risking the possibility of using one of its
**descendants**.
seanpdoyle
added a commit
that referenced
this pull request
Mar 27, 2024
The scenario
---
Imagine a List-Details style page layout, with navigation links on the
left and the contents of the page on the right:
```html
<turbo-frame id="list">
<a href="/articles/1" data-turbo-frame="details">
Article #1
<turbo-frame id="preview_article_1" src="/articles/1/preview">
Some preview text
</turbo-frame>
</a>
<!-- ... -->
<a href="/?page=2">Next Page</a>
</turbo-frame>
<turbo-frame id="details">
<h1>Details</h1>
</turbo-frame>
```
The `#list` element is a `<turbo-frame>` to handle pagination, and the
`#details` element is a `<turbo-frame>` as well. The `<a>` elements
within the `#list` frame drive the `#details` frame through their
`[data-turbo-frame="details"]`. The `<a>` element nests a third kind of
`<turbo-frame>` that is specific to the resource being linked to. It
asynchronously loads in preview content with a `[src]` attribute.
The problem
---
Clicking the `Article #1` text within the `<a>` element drives the
`#details` frame as expected.
However, clicking the `Some preview text` within the `<a>` element's
nested `<turbo-frame>` element navigates the entire page.
This is demonstrated in the following reproduction script:
```ruby
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 "action_controller/railtie"
require "action_view/railtie"
require "action_cable/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,
headless: false
}
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
binding.irb
click_link "Drive #details to ?key=1"
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"
</script>
<style>
body {
display: grid;
grid-template-areas:
"list details"
"list details";
grid-template-columns: 150px 1fr;
grid-template-rows: 50px 1fr;
height: 100vh;
}
#list {
display: flex;
flex-direction: column;
grid-area: list;
overflow-y: scroll;
}
#details {
grid-area: details;
}
</style>
<meta name="turbo-prefetch" content="false">
</head>
<body>
<turbo-frame id="list">
<% 1.upto(5).each do |key| %>
<%= link_to({key:}, data: {turbo_frame: "details"}) do %>
<turbo-frame id="frame_<%= key %>">
Drive #details to <%= {key:}.to_query %>
</turbo-frame>
<% end %>
<% end %>
</turbo-frame>
<turbo-frame id="details">
<%= params.fetch(:key, "0") %>
</turbo-frame>
</body>
</html>
```
The solution
---
When observing `click` events, utilize the `findLinkFromClickTarget` to
find the nearest `<a>` element to the `click` target so that **that**
element's ancestors are used to determine which `<turbo-frame>` to
target instead of risking the possibility of using one of its
**descendants**.
seanpdoyle
force-pushed
the
frame-visit-delegate
branch
from
4cdce4f to
51c406d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.