Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

turbo-rails v2.0.2 explain to me why when I click on links I am taken to the top, “scroll: :preserve” does not work #575

Open
nikolaokonesh opened this issue Feb 10, 2024 · 14 comments

Comments

@nikolaokonesh
Copy link

nikolaokonesh commented Feb 10, 2024

Guys, explain to me why when I click on links I am taken to the top, “scroll: :preserve” does not work
turbo-rails v2.0.2
rails 7.1.3

worked well in --pre versions
after update not work

appication.html.erb

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

importmap.rb

pin "@hotwired/turbo-rails", to: "turbo.min.js", preload: true

please help!

@nikolaokonesh nikolaokonesh changed the title turbo-rails v2.0.2 not work scroll: :preserve turbo-rails v2.0.2 not work "scroll: :preserve" Feb 11, 2024
@nikolaokonesh nikolaokonesh changed the title turbo-rails v2.0.2 not work "scroll: :preserve" turbo-rails v2.0.2 explain to me why when I click on links I am taken to the top, “scroll: :preserve” does not work Feb 14, 2024
@aantix
Copy link

aantix commented Feb 28, 2024

@nikolaokonesh Did you figure this issue out?

@seanpdoyle
Copy link
Contributor

For scroll preservation to behave like you're describing, the navigation needs to:

  1. have a Turbo Action of "replace"
  2. navigate to the current URL

This dovetails with Morphing and form redirects, as well as <turbo-stream action="refresh">, since that Stream ends up calling Turbo.visit directly with the current page's URL and { action: "replace" }.

Do your navigations match that criteria?

@aantix
Copy link

aantix commented Feb 28, 2024

  1. I do not know what the Turbo action of "replace" means within the context of the link_to method.

  2. I am linking to the exact same path, with the exception of an additional parameter appended.
    e.g. <%= link_to edit_form_path(form, selected_question: form_question.id) do %>

@seanpdoyle
Copy link
Contributor

Does link_to edit_form_path(form, selected_question: form_question.id), data: {turbo_action: "replace"} behave as you'd expect?

@aantix
Copy link

aantix commented Feb 28, 2024

The link_to worked as expected with the appropriate data attribute. Thank-you.

A form_with does not, even with that same data attribute added.data: {turbo_action: "replace"}

@seanpdoyle
Copy link
Contributor

A form_with does not

That depends on where the submitted form redirects to. Is it back to the same page, or to a different URL?

@aantix
Copy link

aantix commented Feb 28, 2024

The form_with posts to a different URL, but that action redirects back to the same URL path, with the exception of removing a parameter (e.g. ?selected=1 is removed). This appears to be a full refresh as my scroll position is lost.

If I do a redirect_to request.referer then the morphing works as expected.

@asavageiv
Copy link

I think I'm seeing the same issue with button_to. The meta tags are there with "morph" and "preserve".
button_to [:ocr, :api, image], method: :post, class: "btn btn-light btn-sm"

Controller:
redirect_back fallback_location: ...matching url..., status: :see_other

Regardless of whether the referrer is there, the fallback is the same URL I'm already on.

turbo-rails 2.0.4 and @hotwired/turbo 8.0.3.

I'm not quite sure I understand the criteria in #575 (comment) so please let me know if I'm missing something.

@seanpdoyle
Copy link
Contributor

seanpdoyle commented Feb 29, 2024

I've done my best to reproduce the behavior you've all described above, but I'm having trouble.

I've created the following reproduction script:

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
  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.fetch(:count, 0)} %>
  </body>
</html>

Could you each:

  1. save the script to a bug.rb file
  2. change the script to reproduce the issues you're experiencing
    2a. execute ruby bug.rb to make sure it passes as-is, then fails with your changes
  3. share your reproduction script in this thread

@nikolaokonesh
Copy link
Author

@aantix
Hi! yes, I figured it out, added to every link_to and also added it to the "form.submit" of each form

<%= link_to "Home", root_path, data: { turbo_action: "replace" } %>

This is just an example, There are doubts whether this is correct:

<%= form_with @person do |form| %>
  <%= form.text_field :first_name %>
  <%= form.submit, data: { turbo_action: "replace" } %>
<% end %>

I should have read the Turbo.Drive documentation carefully

My English is not that good =)

@asavageiv
Copy link

My problem was that I had both @hotwired/turbo-rails and @hotwired/turbo in my package.json and @hotwired/turbo-rails hadn't been upgraded 🤦‍♂️

Thanks for the help and the handy reproduction script. That will be useful in the future!

@james-em
Copy link

james-em commented Mar 15, 2024

@seanpdoyle

It appears I have the same issue. Turbo 7.3.0 is fine, Turbo 8 Beta 1 up to latest ain't.

I'm having this issue with turbo_streams. Basically I make an AJAX calls and I render that turbo stream response with Turbo.renderStreamMessage(response...)

Axios.put(target.dataset.url,  {...params},  { headers: { 'Accept': 'text/vnd.turbo-stream.html' } })
      .then((response) => Turbo.renderStreamMessage(response.data))

Response is a simple turbo_stream that replaces an element:

<turbo-stream action="replace" target="project_time_61316">
  <template>
   ...
  </template>
</turbo-stream>

It always scroll back to top. It seems to ignore whatever config I use with

    %meta{content: "replace", name: "turbo-refresh-method"}
    %meta{content: "preserve", name: "turbo-refresh-scroll"}

@seanpdoyle
Copy link
Contributor

@james-em thank you for sharing more details. Could you try and reproduce the behavior with the bug report template proposed in https://github.com/hotwired/turbo-rails/pull/593/files#diff-08735e4e085e5b9ef4c3d78d3bf64c3ed9cf68365fc9d19f9c627036bbb773b4?

@james-em
Copy link

james-em commented Mar 15, 2024

@james-em thank you for sharing more details. Could you try and reproduce the behavior with the bug report template proposed in https://github.com/hotwired/turbo-rails/pull/593/files#diff-08735e4e085e5b9ef4c3d78d3bf64c3ed9cf68365fc9d19f9c627036bbb773b4?

@seanpdoyle

After further debugging it appears it happens when there are duplicated IDs in the page, more specifically when the element triggering the event is the one being duplicated.

Here is a reproductible exemple : https://pastebin.com/ui0zzDZr (Simply run it with ruby file.rb and then visit http://localhost:4567) (Scroll down and check any box)

PS: An empty Turbo.renderStreamMessage("") can trigger the error just fine.

While it ain't really a bug, it definitely is a breaking change that could potentially bring head aches to some people who aren't aware they have duplicated IDs. Here was our line of code in Rails:

- @project_times.each do |project_time|
    = check_box_tag :process, project_time.id, project_time.processed_at.present?, {data: { action: 'change->process-time#onChange'}

All the checkboxes had "process" for an ID

Edit: hotwired/turbo#1226

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants