Describe criteria for link and form morphing #178
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
Expand upon the Page Refresh sections explaining how to morph and preserve scrolling. Page Refreshes --- A "page refresh" is a [application visit](/handbook/drive#application-visits) with a `"replace"` action to a URL with a whose [pathname](https://developer.mozilla.org/en-US/docs/Web/API/URL/pathname) matches the current URL [path](https://developer.mozilla.org/en-US/docs/Learn/Common_questions/Web_mechanics/What_is_a_URL#path_to_resource). Page refreshes can be initiated by driving the page with a link, or by [redirecting after a form submission](/handbook/drive#redirecting-after-a-form-submission). In either case, the elements must have a `[data-turbo-action="replace"]` attribute: ```html <a href="/" data-turbo-action="replace">Page refresh link</a> <form action="/redirect_back" method="post" data-turbo-action="replace"> <button>Page refresh form</button> </form> ```
|
This level of detail feels necessary, since there has been some surprise around links and forms not morphing. Below is a self-contained Turbo Rails application that morphs with form submissions and link clicks. The system test covering scroll preservation passes. If the require "bundler/inline"
gemfile(true) do
source "https://rubygems.org"
git_source(:github) { |repo| "https://github.com/#{repo}.git" }
gem "rails"
gem "propshaft"
gem "sqlite3"
gem "turbo-rails"
gem "capybara"
gem "cuprite", "~> 0.9", require: "capybara/cuprite"
end
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.turbo.draw_routes = false
Rails.logger = config.logger = Logger.new($stdout)
routes.append do
post "/" => "application#create"
root to: "application#index"
end
end
$template = DATA.read
class ApplicationController < ActionController::Base
include Rails.application.routes.url_helpers
def index
render inline: $template, formats: :html
end
def create
redirect_to root_url(count: params[:count].to_i + 1)
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 = :webrick
config.default_normalize_ws = true
end
ENV["DATABASE_URL"] = "sqlite3::memory:"
ENV["RAILS_ENV"] ||= "test"
Rails.application.initialize!
require "rails/test_help"
class TurboSystemTest < ApplicationSystemTestCase
test "reproduces bug" do
visit root_path
scroll_to find_button("Morph")
assert_scroll_preserved do
click_button "Morph"
assert_text "Count 1"
end
assert_scroll_preserved do
click_button "Morph"
assert_text "Count 2"
end
assert_scroll_preserved do
click_button "Morph"
assert_text "Count 3"
end
assert_scroll_preserved do
click_link "Morph"
assert_text "Count 4"
end
assert_scroll_preserved do
click_link "Morph"
assert_text "Count 5"
end
end
def assert_scroll_preserved(&block)
assert_no_changes -> { evaluate_script("window.scrollY") }, &block
end
end
__END__
<html>
<head>
<script type="importmap">
{
"imports": {
"@hotwired/turbo-rails": "<%= asset_path("turbo.js") %>"
}
}
</script>
<script type="module">
import "@hotwired/turbo-rails"
</script>
<%= turbo_refreshes_with method: :morph, scroll: :preserve %>
<%= yield :head %>
</head>
<body>
<p style="margin-bottom: 100vh;">Count <%= params.fetch(:count, 0) %><p>
<%= button_to "Morph", root_path, method: "post",
data: {turbo_action: "replace"},
params: {count: params[:count].to_i} %>
<%= link_to "Morph", root_path(count: params[:count].to_i + 1),
data: {turbo_action: "replace"} %>
</body>
</html> |
seanpdoyle
commented
Feb 29, 2024
Comment on lines
+19
to
+23
| <a href="/" data-turbo-action="replace">Page refresh link</a> | ||
|
|
||
| <form action="/redirect_back" method="post" data-turbo-action="replace"> | ||
| <button>Page refresh form</button> | ||
| </form> |
There was a problem hiding this comment.
@jorgemanrubia is this guidance correct? Are the [data-turbo-action] attributes necessary, or should the resulting Visit get classified as a "page refresh" through a different mechanism? This guidance is based on boolean criteria in PageView.isPageRefresh.
There was a problem hiding this comment.
@seanpdoyle same-location redirects are already replace by default as per this:
So the attribute is not necessary.
There was a problem hiding this comment.
@jorgemanrubia this documentation was mostly in response to confusion stemming from <a>- and <form>-driven Page Refreshes.
I've verified that redirects in the way you've described *do* work as expected:
require "bundler/inline"
gemfile(true) do
source "https://rubygems.org"
git_source(:github) { |repo| "https://github.com/#{repo}.git" }
gem "rails"
gem "propshaft"
gem "sqlite3"
gem "turbo-rails"
gem "capybara"
gem "cuprite", "~> 0.9", require: "capybara/cuprite"
end
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.turbo.draw_routes = false
Rails.logger = config.logger = Logger.new($stdout)
routes.append do
post "/" => "application#create"
root to: "application#index"
end
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_url
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 = :webrick
config.default_normalize_ws = true
end
ENV["DATABASE_URL"] = "sqlite3::memory:"
ENV["RAILS_ENV"] ||= "test"
Rails.application.initialize!
require "rails/test_help"
class TurboSystemTest < ApplicationSystemTestCase
test "reproduces bug" do
visit root_path
assert_css "h1", text: "Loaded without morphing"
click_button "Morph"
assert_css "h1", text: "Loaded with morph"
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:morph", () => document.querySelector("h1").textContent = "Loaded with morph")
</script>
<meta name="turbo-refresh-method" content="morph">
</head>
<body>
<h1>Loaded without morphing</h1>
<%= button_to "Morph", root_path %>
</body>
</html>There was a problem hiding this comment.
On further inspection, I realized the difference between #178 (comment) and #178 (comment).
The Navigator.getDefaultAction you've shared:
#getDefaultAction(fetchResponse) {
const sameLocationRedirect = fetchResponse.redirected && fetchResponse.location.href === this.location?.href
return sameLocationRedirect ? "replace" : "advance"
}differs from the Visit.isPageRefresh comparison:
isPageRefresh(visit) {
return !visit || (this.lastRenderedLocation.pathname === visit.location.pathname && visit.action === "replace")
}The Navigator.getDefaultAction compares URL.href values, whereas the Visit.isPageRefresh compares URL.pathname.
Put another way, redirects back require exact matches of the full URLs (including query parameters), while refreshing when the action is forced only compares the URL paths (excluding query parameters).
This script passes with the `data: {turbo_action: "replace"}` option, then fails without the option:
require "bundler/inline"
gemfile(true) do
source "https://rubygems.org"
git_source(:github) { |repo| "https://github.com/#{repo}.git" }
gem "rails"
gem "propshaft"
gem "sqlite3"
gem "turbo-rails"
gem "capybara"
gem "cuprite", "~> 0.9", require: "capybara/cuprite"
end
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.turbo.draw_routes = false
Rails.logger = config.logger = Logger.new($stdout)
routes.append do
post "/" => "application#create"
root to: "application#index"
end
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_url(count: params[:count].to_i + 1)
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 = :webrick
config.default_normalize_ws = true
end
ENV["DATABASE_URL"] = "sqlite3::memory:"
ENV["RAILS_ENV"] ||= "test"
Rails.application.initialize!
require "rails/test_help"
class TurboSystemTest < ApplicationSystemTestCase
test "reproduces bug" do
visit root_path
scroll_to find_button("Morph")
assert_scroll_preserved do
click_button "Morph"
assert_text "Count 1"
end
end
def assert_scroll_preserved(&block)
assert_no_changes -> { evaluate_script("window.scrollY") }, &block
end
end
__END__
<html>
<head>
<script type="importmap">
{
"imports": {
"@hotwired/turbo-rails": "<%= asset_path("turbo.js") %>"
}
}
</script>
<script type="module">
import "@hotwired/turbo-rails"
</script>
<meta name="turbo-refresh-method" content="morph">
<meta name="turbo-refresh-scroll" content="preserve">
</head>
<body>
<p style="margin-bottom: 100vh;">Count <%= params.fetch(:count, 0) %><p>
<%= button_to "Morph", root_path,
data: {turbo_action: "replace"},
params: {count: params[:count].to_i} %>
</body>
</html>Does that inconsistency need to be resolved? Should replace actions become more flexible by comparing URL.pathname instead of URL.href? Should page refreshes become more strict by comparing URL.href instead of URL.pathname?
There was a problem hiding this comment.
@seanpdoyle The motivation behind using pathname is here, although this also generates slight inconsistencies in mobile adapters that still only check href. What is your opinion on this?
There was a problem hiding this comment.
What is your opinion on this?
I think behavior should be consistent. I'm not sure I have enough practical experience to judge which side should change to match the other.
If it's determined that inconsistency is the best option, then I think a documentation change (like the one proposed in this PR) will be necessary to clarify what is and is not going to result in a Page Refresh.
There was a problem hiding this comment.
@jorgemanrubia should determining Visit Action based on the URL (as described in #178 (comment)) be consistent in both Navigator.getDefaultAction and Visit.isPageRefresh?
If it should, do you have an opinion on which implementation should change to be consistent with the other?
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.
Expand upon the Page Refresh sections explaining how to morph and preserve scrolling.
Page Refreshes
A "page refresh" is a application visit with a
"replace"action to a URL with a whose pathname matches the current URL path. Page refreshes can be initiated by driving the page with a link, or by redirecting after a form submission. In either case, the elements must have a[data-turbo-action="replace"]attribute: