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

Layout not omitted for turbo frames when using custom layout #268

Closed
morgoth opened this issue Oct 27, 2021 · 9 comments · Fixed by #470
Closed

Layout not omitted for turbo frames when using custom layout #268

morgoth opened this issue Oct 27, 2021 · 9 comments · Fixed by #470

Comments

@morgoth
Copy link
Contributor

morgoth commented Oct 27, 2021

Let's say having:

AdminController < ApplicationController
  layout :admin
end

When I use turbo frame request in sample controller:

UsersController < AdminController
  def show
  end
end

The optimization to omit layout in

layout -> { false if turbo_frame_request? }
does not work.

I can see that this bug is known, ie #232 (comment) but opening this issue to keep track on this specific problem.

@WriterZephos
Copy link

I don't consider this a bug. You want dynamic behavior but you are setting the layout in a static way. Try setting your layout with a proc instead of a static value.

The change you are asking for would break other people's code (including my own) where we rely on being able to set the layout and override the turbo default.

@HalfBloody
Copy link

HalfBloody commented May 11, 2022

This behaviour breaks the turbo frame silently when it's placed in the layout because the header already contains an empty turbo frame with the same name.

image

It's counter-intuitive that you can place a turbo_frame in Application Layout and it will work but if you place it in custom layout it will unexpectedly not work. This will hinder Turbo adoption.

@samlachance
Copy link

I ran into something similar to what @HalfBloody did. In my case, though, I was using a turbo-frame with a src which errored when the returned HTML included the layout, which included the turbo-frame with a src

I'm not sure I understand why there is or should be a connection between whether a layout is set and whether a layout will be included in the turbo-frames response. Different layouts are used for all sorts of reasons.

I feel like the current behavior is only intuitive and desirable if you are employing a very specific design pattern. It sounds like the current behavior could be utilized in interesting ways but I'm not sure it's worth not being able to use turboframes in a layout if a layout is set in a controller.

@nickjj
Copy link

nickjj commented Nov 25, 2022

I've encountered this too. It's kind of a big deal because it means our targeted frames are really executing all of the queries associated with other areas of the layout.

I tried setting the layout in 2 different ways:

class Courses::ApplicationController < ApplicationController
  # layout 'courses'
  layout Proc.new { 'courses' }
end

In both cases the full page's content (from <html> to </html>) gets sent in the response when navigating links within a frame.

Does anyone have a suggested workaround?

For more context my custom courses layout has:

<% content_for :content do %>
  <%= render 'courses/video' %>

  <div>
    <%= turbo_frame_tag 'course_details', data: { turbo_action: 'advance' } do %>
      <%= render 'courses/nav' %>
      <%= yield %>
    <% end %>
  </div>

  <%= render 'courses/toc' %>
<% end %>

<%= render template: 'layouts/application' %>

The concern is rendering the video and toc is pretty expensive. There's a bunch of queries that get run and none of that content needs to change when navigating the links in the course detail's frame.

@morgoth
Copy link
Contributor Author

morgoth commented Nov 25, 2022

@nickjj You can write in the given controller action render layout: !turbo_frame_request?

@nickjj
Copy link

nickjj commented Nov 25, 2022

@morgoth that ends up bypassing the frame and makes links transition in the same way as Turbo Drive. That was pretty unexpected behavior. I am using nested frames too. Basically that "courses_details" frame has other frames that are used within it.

@tmaier
Copy link

tmaier commented May 17, 2023

I was also hit by this, but the other way around. I did not know that turbo-rails would set the layout to false automatically. I started to write layout: false in many related renders (e.g. of modals)

I would vote for properly documenting this behaviour.

The solution is to recommend to not use layout :admin in the controller, where admin is the name of the layout file, but to set it to a method.

For example

AdminController < ApplicationController
  layout :determine_layout

  private

  def determine_layout
    return false if turbo_frame_request?
    
    :admin
  end
end

@cice
Copy link
Contributor

cice commented Jun 5, 2023

We have just been bitten by this hard, too. Imho this should be documented

@nickjj
Copy link

nickjj commented Jun 20, 2023

What pattern would you use to get this to work when the following conditions are true:

  1. You have a custom layout
  2. Your custom layout includes defining a turbo_frame_tag
  3. You have a bunch of view partials that set a turbo_frame target to the frame defined in step 2

With the documentation's suggested changes, I do only get back the turbo_rails/frame.html.erb response but I also get a "Content missing" error and it's due to the response not containing the expected frame defined in step 2.

For example, here's my custom layout in layouts/course/player.html.erb:

<% content_for :content do %>
  <main>
    <div class="min-h-screen md:flex">
      <div class="flex-1">
        <%= turbo_frame_tag "course_video_player" do %>
          ...
        <% end %>

        <%= turbo_frame_tag "course_details", data: { turbo_action: "advance" } do %>
          <%= render "courses/nav" %>
          ...
        <% end %>
      </div>

      <%= render "courses/toc" %>
    </div>
  </main>
<% end %>

<%= render template: "layouts/application" %>

Then inside one of my views for this layout I have a partial with:

<%= link_to "Hello world", "...", data: { turbo_frame: "course_details" } %>

When I click that link, that's when I get the content missing error.

I think this makes sense because the course_details frame doesn't exist since I made a frame request which means the turbo_rails/frame.html.erb layout is being used which doesn't have that frame defined. But I'm not quite sure what an ideal pattern looks like here to make this work. Especially since the layout has a lot of other common things, having to duplicate essentially the entire course player's layout in each view which would be painful. The "real" version has like 100 lines of HTML and I have a lot of views.

Ideally that frame and its associated layout related HTML would exist once somewhere, which is what I did back when the layout loaded in each response but now the layout isn't loaded for frame requests (which technically is the result I want, except I still want the benefits of the layout). What am I doing wrong?

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

Successfully merging a pull request may close this issue.

7 participants