Include layout for `Turbo-Frame:` requests #232

seanpdoyle · 2021-09-10T17:45:16Z

Include layout for `Turbo-Frame:` requests

The bandwidth benefits of optimizing Turbo-Frame: header responses to
omit the layout do not offset the HTTP-layer cache busting trade-offs.

Similarly, there's an opportunity to implement behavior changes related
to:

if both <turbo-frame> elements' FrameController instances and
Session instances were able to share Visit instances built from
fully formed <html> pages. By trimming the layout and outer portions
of the document, turbo-rails forces frames to deal with incomplete
fragments.

Mark variant as `turbo_frame` when header present

Closes #229

Mark the request's variant as :turbo_frame whenever the
Turbo-Frame: header is present in the request. Also adds documentation
to describe how to achieve the previously built-in layout optimizations.

The bandwidth benefits of optimizing `Turbo-Frame:` header responses to omit the layout do not offset the HTTP-layer cache busting trade-offs. Similarly, there's an opportunity to implement behavior changes related to: * [@hotwired/turbohotwired#361][] * [@hotwired/turbohotwired#257][] if both `<turbo-frame>` elements' `FrameController` instances and `Session` instances were able to share `Visit` instances built from fully formed `<html>` pages. By trimming the layout and outer portions of the document, `turbo-rails` forces frames to deal with incomplete fragments. [@hotwired/turbohotwired#257]: hotwired/turbo#257 [@hotwired/turbohotwired#361]: hotwired/turbo#361

dhh · 2021-09-17T12:40:15Z

I like the idea of simplifying things, but there are more considerations than just bandwidth. There's also the price of producing the request server side. Like the cost to produce the layout itself. But... that cost is paid everywhere else, so is that enough?

I'd like to verify this on our apps though.

Also, there's the slight regression in readability on the return when you're debugging. It's been very nice just to see the frame that changed.

Closes hotwired#229 --- Mark the request's [variant][] as `:turbo_frame` whenever the `Turbo-Frame:` header is present in the request. Also adds documentation to describe how to achieve the previously built-in layout optimizations. [variant]: https://guides.rubyonrails.org/4_1_release_notes.html#action-pack-variants

seanpdoyle · 2021-09-17T18:14:03Z

If hotwired/turbo#398 is viable, it'd be simpler to traffic in full HTML documents.

If there's interest to support <turbo-frame>-only responses without the outer document, hotwired/turbo#398 would need to synthesize the VisitOptions.responseHTML by injecting the <turbo-frame> into the browser's "live" document, then transforming the DOM into an HTML string.

It would be possible with only fragments, but committing to transmitting full HTML documents as responses has other upsides that make it seem worthwhile.

This is, of course, only a requirement for frames that drive the browser's history. If applications or frames aren't interested in that, they can opt-into the bare <%= yield %> layout via the html+turbo_frame variant.

It would be easier to opt-in for scenarios that need it than it is to reverse the existing default behavior of never sending a full document response.

dhh · 2021-09-23T08:51:10Z

So been thinking about this some more. I don't think layout-by-default is going to give us any caching advantages, because most turbo frames are used/loaded exclusively for that purpose. They're rarely used directly. At least that's the usage across all of HEY for us.

Furthermore, many layouts are actually pretty big. For HEY, now that we're going with import maps, there's a lot of stuff in the head. Don't want to keep shipping that across all the time.

ghiculescu · 2021-09-28T20:05:35Z

app/controllers/turbo/frames/frame_request.rb

-# This is merely a rendering optimization. Everything would still work just fine if we rendered everything including the layout.
-# Turbo Frames knows how to fish out the relevant frame regardless.
+# When the <tt>Turbo-Frame</tt> header is present, the request's <a
+# href="https://guides.rubyonrails.org/4_1_release_notes.html#action-pack-variants">variant</a>


Suggested change

# href="https://guides.rubyonrails.org/4_1_release_notes.html#action-pack-variants">variant</a>

# href="https://guides.rubyonrails.org/layouts_and_rendering.html#the-variants-option">variant</a>

ghiculescu · 2021-09-28T20:10:44Z

Something that maybe hasn't been noted yet: At the moment if you set layout "whatever" in a controller that's also receiving frame requests, the current optimisations are lost (since you've overridden the layout). It looks like this PR presents a way to fix that, which is awesome.

ghiculescu · 2021-09-28T20:11:03Z

app/controllers/turbo/frames/frame_request.rb

+# <tt>html+turbo_frame.erb</tt> extension that renders
+# without the navigation and layout elements:
+#
+#     <%= app/views/layouts/application.html+turbo_frame.erb %>


Suggested change

# <%= app/views/layouts/application.html+turbo_frame.erb %>

# <%# app/views/layouts/application.html+turbo_frame.erb %>

Would it be possible to generate this view as part of the install generator?

What exactly would be in this layout? Seems like we would need a way to customize the turbo_frame_id here if it wraps the rendered content with a turbo_frame, but I am not sure that is the intent.

ghiculescu · 2021-09-29T15:09:08Z

Ahhh, sorry, I meant if you do something like this

class ApplicationController
  layout "my_layout_view"
end

Then that overrides the layout that turbo-rails adds, so your frame requests get a layout.

This PR fixes it though so that's cool. Also you can just take the stuff in this PR and copy it into your own app (which I did yesterday and it worked great).

seanpdoyle · 2021-09-29T15:13:29Z

For what it's worth, the turbo_frame variant support is also covered by #250.

The behavior mentioned in #232 (comment) relies on the variant portion of these changes more than reverting the before_action and etag blocks.

WriterZephos · 2021-10-09T03:04:10Z

I don't think this Closes #229 exactly. The solution proposed in #229 is much more flexible in my opinion, and I think not sending the full rendered page on every request is a major benefit and one of turbo's strengths. It would be a shame to lose that.

I favor the solution in provided by #250

seanpdoyle · 2021-11-14T20:36:24Z

I've opened hotwired/turbo#445 to introduce the turbo:frame-missing event to provide applications with a seam to respond to a request when the response does not contain a matching <turbo-frame> element.

In some cases, (like in the case of a 401...403 status code in response to an expired session), applications might want to replace the contents of the current page with the frame-less Response HTML.

That capability is contingent on the response being a fully formed HTML document.

There's also an ongoing discussion about providing applications with the ability to "break out" of a frame. The proposed version of that feature would also depend on the response to a request with theTurbo-Frame: header containing a fully-formed HTML document.

WriterZephos · 2022-02-17T19:08:41Z

@dhh @seanpdoyle I have implemented a module that, when included in a controller, dynamically determines the layout. This way you can use standard templates and either wrap them in a turbo_frame or wrap them in the application layout, depending on what kind of request it is.

The module is here: https://github.com/WriterZephos/TurboRouter/blob/main/lib/turbo_router/controller.rb

I am curious if this is born out of a misunderstanding and if there is an official way to do this, or if the PR is meant to provide an official way.

Also, it seams like the changes proposed by @ghiculescu would break the pattern I implemented, and as I stated before I think there is real benefit from not rendering the layout by default, since most turbo_frame requests will be intended to render partials.

My 2 cents from someone trying to figure out the right patterns for this stuff.

WriterZephos · 2022-02-17T19:15:09Z

Something that maybe hasn't been noted yet: At the moment if you set layout "whatever" in a controller that's also receiving frame requests, the current optimisations are lost (since you've overridden the layout). It looks like this PR presents a way to fix that, which is awesome.

So I think setting a layout globally SHOULD override the current behavior. I think if you want to set a layout in a more nuanced way, you need a before action such as what was proposed above or something like the pattern I implemented here where layout is a proc:

https://github.com/WriterZephos/TurboRouter/blob/main/lib/turbo_router/controller.rb

bborn · 2022-05-18T20:24:52Z

Chiming in here in favor of rendering with the layout (or at least making this configurable). If you have a layout that uses a content_for block, this causes lots of confusion. For example in my case, my layout has something like

-if content_for(:sidebar)
  = yield :sidebar

If I try to render a turbo response with a template that captures the :sidebar content, it just doesn't work, because the layout isn't rendered at all.

dhh · 2022-05-22T13:55:43Z

I don't think doing the layout for all turbo requests is the right trade-off as a default. But I'd be happy to take a way, or perhaps just documentation, to let people tweak this for an individual app. Let's explore that in a new PR, though.

seanpdoyle · 2022-07-05T14:25:49Z

A new edge case has been uncovered that hasn't been mentioned in this discussion: when responding to a form submitted from within a <turbo-frame> with a Turbo Stream response, the app/views/layouts/application.turbo_stream.erb layout is omitted. Circumventing the default layout -> { false if turbo_frame_request? } block becomes tricky, since the response's MIME type isn't known until very late in the controller's action.

For example, a controller action "ending" with redirect_to(...) would respond with an HTML content type redirect response without a layout, but one that "ends" with

respond_to do |format|
  format.html { redirect_to(...) }
  format.turbo_stream
end

would respond with a Turbo Stream content type, and that response would surprisingly omit a layout.

If the layout { false if turbo_stream_request? } block were removed, then responses to Turbo Frame HTML requests would include a layout and responses to Turbo Stream requests would also include a layout.

dhh · 2022-07-14T23:42:37Z

Wouldn't this fly:

respond_to do |format|
  format.html { redirect_to(...) }
  format.turbo_stream { render layout: "something" }
end

?

Kulgar · 2022-07-22T16:15:23Z

Same problem as @seanpdoyle regarding form submission

@dhh yes it would (and it works), but then, we would have to put the "render layout" every time? Or is there a better way?

[Edit] I found a way, if you add this line in your application_controller.rb :

layout -> { "application" if turbo_frame_request? }

Then, the application.turbo_stream layout is loaded for every turbo stream request.
As my turbo stream layout is really light (it is just including the turbo stream for flash messages), my app won't suffer much for this addition.

Re-submission of [hotwired#232][] Related to [hotwired/turbo#1047][] Render full documents, including default layout rendering behavior. Rendering a minimal layout forces `turbo:reload` events because of the severe difference in the contents of the minimal layout's `<head>` and the requesting document's fully populated `<head>`. [hotwired#232]: hotwired#232 [hotwired/turbo#1047]: hotwired/turbo#1047

seanpdoyle force-pushed the turbo-frame-include-layout branch from 72b9134 to 6777a72 Compare September 15, 2021 16:35

seanpdoyle force-pushed the turbo-frame-include-layout branch from 6777a72 to f56024c Compare September 17, 2021 12:34

seanpdoyle force-pushed the turbo-frame-include-layout branch from dd6fb31 to d0a7661 Compare September 17, 2021 14:13

ghiculescu reviewed Sep 28, 2021

View reviewed changes

morgoth mentioned this pull request Oct 27, 2021

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

Closed

seanpdoyle mentioned this pull request Nov 13, 2021

Ability to override frame-target from server response hotwired/turbo#257

Open

dhh closed this May 22, 2022

seanpdoyle mentioned this pull request Jun 26, 2022

Document how to add a layout for turbo frame requests #351

Closed

seanpdoyle mentioned this pull request Aug 1, 2022

Introduce turbo:frame-missing event hotwired/turbo#445

Merged

willcosgrove mentioned this pull request Nov 11, 2022

turbo:frame-missing: Re-use FetchResponse HTML hotwired/turbo#672

Merged

This was referenced Jan 30, 2023

Turbo-frame caching doesn't seem to work when preloading frame links. hotwired/turbo#857

Closed

Include minimal layout in frame responses #428

Merged

seanpdoyle mentioned this pull request Dec 1, 2023

Render full documents for requests with Turbo-Frame: header #534

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Include layout for `Turbo-Frame:` requests #232

Include layout for `Turbo-Frame:` requests #232

seanpdoyle commented Sep 10, 2021 •

edited

Loading

dhh commented Sep 17, 2021

seanpdoyle commented Sep 17, 2021

dhh commented Sep 23, 2021

ghiculescu Sep 28, 2021

ghiculescu commented Sep 28, 2021

ghiculescu Sep 28, 2021

ghiculescu Sep 29, 2021

WriterZephos Feb 17, 2022

ghiculescu commented Sep 29, 2021

seanpdoyle commented Sep 29, 2021

WriterZephos commented Oct 9, 2021 •

edited

Loading

seanpdoyle commented Nov 14, 2021

WriterZephos commented Feb 17, 2022

WriterZephos commented Feb 17, 2022 •

edited

Loading

bborn commented May 18, 2022

dhh commented May 22, 2022

seanpdoyle commented Jul 5, 2022

dhh commented Jul 14, 2022

Kulgar commented Jul 22, 2022 •

edited

Loading

	# href="https://guides.rubyonrails.org/4_1_release_notes.html#action-pack-variants">variant</a>
	# href="https://guides.rubyonrails.org/layouts_and_rendering.html#the-variants-option">variant</a>

	# <%= app/views/layouts/application.html+turbo_frame.erb %>
	# <%# app/views/layouts/application.html+turbo_frame.erb %>

Include layout for Turbo-Frame: requests #232

Include layout for Turbo-Frame: requests #232

Conversation

seanpdoyle commented Sep 10, 2021 • edited Loading

Include layout for Turbo-Frame: requests

Mark variant as turbo_frame when header present

dhh commented Sep 17, 2021

seanpdoyle commented Sep 17, 2021

dhh commented Sep 23, 2021

ghiculescu Sep 28, 2021

Choose a reason for hiding this comment

ghiculescu commented Sep 28, 2021

ghiculescu Sep 28, 2021

Choose a reason for hiding this comment

ghiculescu Sep 29, 2021

Choose a reason for hiding this comment

WriterZephos Feb 17, 2022

Choose a reason for hiding this comment

ghiculescu commented Sep 29, 2021

seanpdoyle commented Sep 29, 2021

WriterZephos commented Oct 9, 2021 • edited Loading

seanpdoyle commented Nov 14, 2021

WriterZephos commented Feb 17, 2022

WriterZephos commented Feb 17, 2022 • edited Loading

bborn commented May 18, 2022

dhh commented May 22, 2022

seanpdoyle commented Jul 5, 2022

dhh commented Jul 14, 2022

Kulgar commented Jul 22, 2022 • edited Loading

Include layout for `Turbo-Frame:` requests #232

Include layout for `Turbo-Frame:` requests #232

seanpdoyle commented Sep 10, 2021 •

edited

Loading

Include layout for `Turbo-Frame:` requests

Mark variant as `turbo_frame` when header present

WriterZephos commented Oct 9, 2021 •

edited

Loading

WriterZephos commented Feb 17, 2022 •

edited

Loading

Kulgar commented Jul 22, 2022 •

edited

Loading