search

hotwired / turbo-rails

Use Turbo in your Ruby on Rails app
https://turbo.hotwired.dev
MIT License
2.06k stars 319 forks source link

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

Closed nikolaokonesh closed 1 month ago

nikolaokonesh commented 6 months ago

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!

aantix commented 5 months ago

@nikolaokonesh Did you figure this issue out?

seanpdoyle commented 5 months ago

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 commented 5 months ago

  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 commented 5 months ago

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

aantix commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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 https://github.com/hotwired/turbo-rails/issues/575#issuecomment-1969229695 so please let me know if I'm missing something.

seanpdoyle commented 5 months ago

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 commented 5 months ago

@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 commented 5 months ago

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 commented 5 months ago

@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 commented 5 months ago

@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 commented 5 months ago

@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: https://github.com/hotwired/turbo/issues/1226