Expand rendering capabilities of <turbo-frame>

[seanpdoyle]

Note:

This PR depends on the FrameVisit extraction introduced in #430.

When reviewing the code and comparing the diffs, the first commit should be ignored: https://github.com/seanpdoyle/turbo/compare/frame-visit-delegate...seanpdoyle:frame-action-with-visit?expand=1

Problem

Navigating a <turbo-frame> element always renders the new content by
updating the contents with the children from the response. This limits
the utility of <turbo-frame> in scenarios involving pagination driven
by "next" and "previous" page links.

Solution

Add support for <turbo-frame rendering="...">, <a data-turbo-rendering="...">, <form data-turbo-rendering="...">, and
<button data-turbo-rendering="..."> where the value of [rendering]
and [data-turbo-rendering] is one of the values that <turbo-stream action="..."> supports.

By default, <turbo-frame> elements will continue to render render with
the behavior equivalent to a <turbo-stream action="update"> element.

This commit extends that support to also include the other actions:

• after will insert the contents of the response frame after the
  request frame
• append will extract the contents out of the response frame and
  append them into the request frame
• before will insert the contents of the response frame before the
  request frame
• prepend will extract the contents out of the response frame and
  append them into the request frame
• replace will extract the contents out of the response frame, remove
  the request frame, and inject the extracted contents in its place
  (conceptually similar to setting outerHTML)
• remove will remove the request frame, and ignore the contents of the
  response frame

This enables behaviors that might have been achievable with
GET-request powered Turbo Stream responses.

For example, in-place pagination could be achieved with
rendering="prepend" or rendering="append":

<!-- current HTML -->
<turbo-frame id="posts">
  <article id="article_1"><!-- contents --></article>
  <!-- articles 2-9 -->
  <article id="article_10"><!-- contents --></article>

  <a href="/posts?page=2" data-turbo-rendering="append">Next page</a>
</turbo-frame>

<!-- response HTML -->
<turbo-frame id="posts">
  <article id="article_11"><!-- contents --></article>

  <a href="/posts?page=3" data-turbo-rendering="append">Next page</a>
</turbo-frame>

<!-- HTML after the request -->
<turbo-frame id="posts">
  <article id="article_1"><!-- contents --></article>
  <!-- articles 2-9 -->
  <article id="article_10"><!-- contents --></article>
  <a href="/posts?page=2" data-turbo-rendering="append">Next page</a>
  <article id="article_11"><!-- contents --></article>

  <a href="/posts?page=3" data-turbo-rendering="append">Next page</a>
</turbo-frame>

Through the power of a CSS rules utilizing :last-of-type, we can hide
the pagination links:

  #posts a              { display: none; }
  #posts a:last-of-type { display: block; }

[jonathanhefner]

This is very cool! 😍 Would it be reasonable to also support configuring this behavior via a data-turbo-frame-action attribute on the link itself? That way different links could trigger different behavior.

[seanpdoyle]

This is very cool! 😍 Would it be reasonable to also support configuring this behavior via a data-turbo-frame-action attribute on the link itself? That way different links could trigger different behavior.

@jonathanhefner that makes a lot of sense, and is in line with the rest of the form- prefixed attributes we intercept from Form submitters.

What I'm still not sure about is whether or not the action attribute on <turbo-frame> communicates what's going on. I conceptualize it as different ways of rendering. The action name is borrowed from <turbo-stream> elements, where action feels appropriate: the element's existence is purely centered around actively changing the DOM, and action communicates that.

I'm unsure about whether or not sharing the values across both implementations while changing the attribute's name will lead to confusion. I think I'm considering <turbo-form render="..."> instead of <turbo-form action="..."> or <turbo-stream render="..."> instead of <turbo-stream action="...">.

Either way, I think enabling an external frame driver to decide on how its rendered makes sense, though I can't immediately think of a use case for that. Maybe a <turbo-frame> modal you'd want to load (action="update") or dismiss (action="remove")?

[rainerborene]

This frame API looks very confusing at first sight. IMO this will enforce bad practices and kill the purpose of turbo-stream. I think it makes more sense to create better abstractions on the server side than following this direction.

[kaspth]

@rainerborene this is an open PR where the API and documentation haven't settled yet, saying that it's confusing is valuable, but also not that helpful for where this is at and taking it further or figuring out if another direction should be taken. Can you give concrete suggestions on making the API less confusing as a code review with potential documentation?

I'd also like to see some concrete suggestions on the server side abstractions, in here as a comment or a separate PR — otherwise there's no other direction to follow, and you're implicitly asking us to do work for you.

[seanpdoyle]

IMO this will enforce bad practices and kill the purpose of turbo-stream.

I think we're in agreement that it should be hard to fall into bad practices, and that the tooling and framework should make it easy to do the right thing.

In my mind, turbo-stream elements are very valuable and would still be after this proposed change. They're useful for globally changing parts of the requesting page's DOM, and are not scoped by where on the page the request was initiated from. Turbo Stream elements are intended to be sent in response to a form submission, and can be as small as a single <turbo-stream> element fragment. They do not need to be served in a valid HTML document response, and can not be accessed as their own resource.

On the other hand, turbo-frames are explicitly scoped to a part of the page during both the request and response part of their lifecycles. A turbo-frame response must contain a <turbo-frame> element with an [id] attribute that matches the requesting frame, and should be a fully valid HTML page of its own, and could be accessed via its own resource via an HTTP GET request. When a form submission redirects after submission, the URL it redirects to must be accessible via an HTTP GET request.

This PR proposes that we extend the ways in which a frame can retrieve the fragment in the resulting GET request. Its scope is purely GET related, and doesn't overlap with other ways to respond to a form submission (like a <turbo-stream> page change).

Does that make sense? I admit that there's an opportunity to better tell these stories as part of Turbo's Frame and Stream documentation, and I plan on opening PRs there once the attribute interface discussion settles.

[stgm]

Hmm, it is interesting, but it's definitely hard to come up with good use cases. Let me add one that seems to be a good fit.

Currently I have a modal dialog in my site to perform admin actions, get quick overviews (e.g. what a user has posted) but still allows me to return to the main interface immediately. It works great and it was extremely easy to port it to Turbo Frames. But, currently, if there is a link to another admin-like page, the next screen always completely replaces the previous one. I can't get back unless that screen has a link back.

For my site, it makes total sense to implement a basic "back stack" for the modal:

• clicking through to another admin page uses the proposed frame append action
• the new content is shown using a :last-of-type CSS selector like in @seanpdoyle's pagination example
• and a back button that simply removes the last element in the modal

This makes navigation much nicer and the admin modal quite a bit more useful.

I might agree that this kind of stuff is easy to abuse by developers, though :-) As the stack builds, so will the DOM. But I also think that these kinds of use cases aren't meant to be implemented using Streams because they're entirely driven by users' own actions.

[seanpdoyle]

Thinking through a rough modal editing experience, here's some pseudocode demonstrating what I envision as the ideal:

<!-- app/views/posts/show.html.erb -->
<main>
  <a href="/admin/posts/1/edit" data-turbo-frame="dialog">Edit post</a>
</main>
<dialog data-controller="dialog" data-action="turbo:frame-load->dialog#open">
  <turbo-frame id="dialog"></turbo-frame>
</dialog>

<!-- app/views/admin/posts/edit.html.erb -->
<main><!-- empty --></main>
<dialog open>
  <turbo-frame id="dialog">
    <form method="post" action="/admin/posts/1">
      <input type="hidden" name="_method" value="put">
      <button type="submit">Update</button>
    </form>
    <a href="/admin/posts/1">Preview</a>
    <a href="/posts/1" data-turbo-frame="_top">Cancel</a>
  </turbo-frame>
</dialog>

<!-- app/views/admin/posts/show.html.erb -->
<main><!-- empty --></main>
<dialog open>
  <turbo-frame id="dialog">
    <h1>PREVIEW: <%= @post.name %></h1>
    <!-- ... -->
    <a href="/admin/posts/1/edit">Edit</a>
    <a href="/posts/1" data-turbo-frame="_top">Cancel</a>
  </turbo-frame>
</dialog>

This would:

0. start within the show.html.erb template containing a link that targets the <turbo-frame id="dialog"> element on the page within a <dialog> element. Assuming that some form of Dispatch turbo:frame-load on turbo-frame #59 ships in the future, we'll be able to listen for turbo:frame-load events and set <dialog open> when the descendant frame loads.
1. clicking the "Edit post" link would drive the turbo-frame#dialog element, which would load via GET /admin/posts/1/edit and open the dialog element
2. The frame would load and present a form. Assuming that the form would result in a redirect_to post_url(@post) when submitted, clicking the Update button would "close" the modal by driving the page back to GET /posts/1.
3. Within the turbo-frame#dialog, there is an <a href="/admin/posts/1"> element that would navigate within the turbo-frame#dialog element to a "preview" template by loading GET /admin/posts/1
4. Within the /admin/posts/1 page, there is an Edit link that would navigate the turbo-frame#dialog frame back to the edit form. Since both the GET admin/posts/1 and GET /admin/posts/1/edit templates declare turbo-frame#dialog elements, their contents are swapped during navigation.
5. Each template contains an <a href="/posts/1"> element that could "close" the modal by navigating the _top frame (that is, the page itself) back to the original GET /posts/1 page.

In addition to turbo:frame-load events, what's missing from this story is a way to signal for steps 1-4 to signal to _top that we want to push onto the history stack (either as an advance or replace navigation) while preserving the turbo-frame's fragment swapping behavior (preserving the rest of the original page's state). A way to opt into that would make each intermediate step from 0-5 able to survive a page refresh, since each step's modal contents would have its own URL.

For what it's worth, none of this hypothetical currently requires the turbo-frame[action] attribute described in this PR.

[rainerborene]

@kaspth Advice taken, sir. 👍

@seanpdoyle Thank you for the full explanation. By confusing I meant that one can be in doubt when to use one over the other if that makes sense. Before the last release all GET requests were being sent with the Accept header as text/vnd.turbo-stream.html. Now that you pointed it out that Turbo Streams are supposed to be sent in response to form submissions, that totally makes sense. I had a few files like index.turbo_stream.erb so that means I was doing it wrong then? I guess so.

[seanpdoyle]

Now that you pointed it out that Turbo Streams are supposed to be sent in response to form submissions, that totally makes sense. I had a few likes index.turbo_stream.erb so that means I was doing it wrong then? I guess so.

@rainerborene hotwired/turbo-site#40 intends to make that more clear in a more public way.

[panda-madness]

On a tangentially related note, should there be a way to register custom Render actions? Potential candidates could be toggling classes and other attributes, performing complex dom manipulations (i.e. morphdom), etc.. CableReady has a pretty comprehensive list of operations, some of which might be useful for Turbo users. I especially like the idea of Streams being able to trigger not just DOM mutations, but browser events, scroll control, notifications, etc.

[seanpdoyle]

@javan @sstephenson I've renamed StreamActions to RenderActions.

What I'm not sure about is how to promote the concept of what's going on here. While the values are instructions on how to render a change, they were previously <turbo-stream> elements' [action] attribute.

Extended to a <turbo-frame> element, the [action] name is a little more vague: frames can be driven by <a> navigations or <form> submissions, and <form> already declares an [action] attribute.

Should we promote the collection of values as a concept (e.g. "Rendering styles") and then let Streams refer to them as "actions" and Frames refer to them as "render" styles, or do we lean into the "actions" concept and force Streams and Frames to use a consistent term?

Similarly, I realize that it might be valuable for an <a> element to override the <turbo-frame action> attribute for the frame it targets. For example, imagine an infinitely paginating feed, with "load older messages" at the top of the feed, and "load newer messages" at the bottom of the feed. I'd want clicking on the top link to "prepend", and clicking on the bottom link to "append". That seems like a reasonable feature.

However, I don't think I would want the same for <form> and form submitters that target a frame element through data-turbo-frame. For those cases, a submission that responds with <turbo-stream> elements feels good enough.

Does that type of element-based overriding seem valuable enough? If so, we'd need to change how the FrameRedirector.linkClickIntercepted and FrameController.linkClickIntercepted implementations work, since they would need to set the src attribute and temporarily override the action attribute in a way that does not modify the element.

[santib]

Hi @seanpdoyle can this one be re-considered? I think it'd be a great addition, otherwise, turbo streams will still be needed on GET requests (which AFAIK is not the idea)