Introduce `turbo:before-permanent-element-render` #622

seanpdoyle · 2022-07-06T17:45:22Z

While transposing a permanent element from page to page, dispatch a
turbo:before-permanent-element-render event on the document with a
(currentPermanentElement: Element, newPermanentElement: Element) => void available as the event's detail.render property.

The turbo:before-permanent-element-render is named to follow the
pattern established by its predecessors (turbo:before-render,
turbo:before-frame-render, turbo:before-stream-render), and its
CustomEvent.detail.render property name mirrors the
turbo:before-render event's CustomEvent.detail.render property.

This opportunity to modify a permanent element might be useful if it's
possible for the preserved element to become partially stale. For
example, consider an invalid <form> submission with an <input type="file">.

Since the server's response cannot fully encode the submitted file into
the response's element, it might be a useful to mark it with
[data-turbo-permanent] to preserve whatever client-side state precedes
the submission:

<label for="profile_image">Profile image</label>
<input id="profile_image" type="file" data-turbo-permanent>

Suppose the uploaded file is too large, or doesn't match the server's
expectations for file type. The server's response might contain
a fragment like:

<label for="profile_image">Profile image</label>
<input id="profile_image" type="file" data-turbo-permanent
       class="invalid" aria-describedby="profile_image_error">
<p id="profile_image_error">Profile image is too large (5GB). Try a smaller file (<5KB)</p>

Prior to this change, the fact that the <input> is marked with
[data-turbo-permanent] would ignore the [class] and
[aria-describedby] attributes from the response, and the <input>
would remain unchanged (with the attached file still in-memory).

With a new turbo:before-permanent-element-render available, there's an
opportunity to do application-specific merging:

addEventListener("turbo:before-permanent-element-render", ({ target, { detail } }) => {
  detail.render = (currentElement, newElement) => {
    currentElement.setAttribute("class", newElement.getAttribute("class"))
    currentElement.setAttribute("aria-describedby", newElement.getAttribute("aria-describedby"))
  }
})

dhh · 2022-07-06T20:46:03Z

Wouldn't you be able to get the same done using turbo:before-render and digging through the existing page body and the new page body? What's the additional utility of having this narrowed down?

seanpdoyle · 2022-07-06T20:59:37Z

I considered a permanent-specific event to separate the idea from the details of the rendering (was it a full Drive visit? a Frame navigation? a Stream?) so that the code could be more general purpose.

Currently, the newBody is only available to a turbo:before-render event. Frame navigations only dispatch turbo:frame-render events, and those don't provide access to the new fragment of HTML.

I'm also not sure if Turbo Stream rendering currently accounts for permanent elements correctly. If they do, that would be another point in time and event (turbo:before-stream-render) where callers would need access to the new element ahead of time.

If a new event specific to permanent elements isn't warranted, then a turbo:before-frame-render with access to the newFrame would be an appropriate replacement, along with an extension to the turbo:before-stream-render event to provide a handle to the <turbo-stream> element's <template> as well as the elements referenced by the [target] and [targets] attributes.

seanpdoyle · 2022-07-06T21:19:24Z

I'm also not sure if Turbo Stream rendering currently accounts for permanent elements correctly. If they do, that would be another point in time and event (turbo:before-stream-render) where callers would need access to the new element ahead of time.

I've just tried this out, and it appears that Turbo Stream operations ignore [data-turbo-permanent].

You can experiment with the code on JSFiddle:

<html>                                        
  <head>                                  
    <script type="importmap">                                                                                      
      {
        "imports": {
          "@hotwired/turbo": "https://cdn.skypack.dev/@hotwired/turbo"
        }
      }
    </script>
    <script type="module">
      import "@hotwired/turbo"
      
      addEventListener("click", ({ target }) => {
        if (target.matches("button")) {
          const { content } = target.querySelector("template")
          target.append(content.cloneNode(true))
        }
      })
    </script>
    <script type="module" src="https://unpkg.com/[email protected]/dist/es-module-shims.js"></script>
  </head>
  <body>
    <p id="permanent" data-turbo-permanent>This should not change when you click the button</p>
      
    <button type="button">
      Click me, and hopefully nothing changes

      <template>
        <turbo-stream action="replace" target="permanent">
          <template><p id="permanent" data-turbo-permanent>Uh oh, this changed when you clicked the button!</p></template>
        </turbo-stream>
      </template>
    </button>
  </body>
</html>

My gut tells me that's surprising behavior, but maybe it isn't. I've opened #623 to discuss this further.

Turbo Streams aside, the gap between turbo:before-render and turbo:frame-render still poses a problem.

kstratis · 2022-08-07T15:19:56Z

Does this mean that we get the chance to rid the menu "flashing" when navigating with turbo drive?

seanpdoyle · 2022-08-07T15:35:13Z

Does this mean that we get the chance to rid the menu "flashing" when navigating with turbo drive?

Maybe! Could you share some more details of what you're experiencing?

kstratis · 2022-08-09T12:05:12Z

Sure.

The "flashing" effect I'm talking about usually happens when clicking around a menu or top nav.
This thing often comes in the form of a partial fixed somewhere in the layout.

When a user clicks any of the menu options, Turbo Drive replaces the entire body. The side-effect is that since the "nav" partial is part of the layout which is part of the body, it gets replaced too. This is totally expected cause the "active" menu link has to somehow be updated. However is it suboptimal cause the user sees a "flash" as a result of the "rerender".

At the moment one could set the entire menu as permanent but then the "active" link would never update.
Looking at this PR I was wondering if we could somehow use the permanent attribute along with the new hook turbo:before-permanent-element-render and manually update the "active" menu link in javascript.

I'm also attaching a couple of videos below further demonstrating the issue:

gone_fishing.mp4

Video source: Gone fishing tutorial app

railsdevs.mp4

Video source: RailsDevs

kstratis · 2022-08-12T10:01:58Z

cc @seanpdoyle

While transposing a [permanent element][] from page to page, dispatch a `turbo:before-permanent-element-render` event on the document with a `(currentPermanentElement: Element, newPermanentElement: Element) => void` available as the event's `detail.render` property. The `turbo:before-permanent-element-render` is named to follow the pattern established by its predecessors (`turbo:before-render`, `turbo:before-frame-render`, `turbo:before-stream-render`), and its `CustomEvent.detail.render` property name mirrors the `turbo:before-render` event's `CustomEvent.detail.render` property. This opportunity to modify a permanent element might be useful if it's possible for the preserved element to become partially stale. For example, consider an invalid `<form>` submission with an `<input type="file">`. Since the server's response cannot fully encode the submitted file into the response's element, it might be a useful to mark it with `[data-turbo-permanent]` to preserve whatever client-side state precedes the submission: ```html <label for="profile_image">Profile image</label> <input id="profile_image" type="file" data-turbo-permanent> ``` Suppose the uploaded file is too large, or doesn't match the server's expectations for file type. The server's response might contain a fragment like: ```html <label for="profile_image">Profile image</label> <input id="profile_image" type="file" data-turbo-permanent class="invalid" aria-describedby="profile_image_error"> <p id="profile_image_error">Profile image is too large (5GB). Try a smaller file (<5KB)</p> ``` Prior to this change, the fact that the `<input>` is marked with `[data-turbo-permanent]` would ignore the `[class]` and `[aria-describedby]` attributes from the response, and the `<input>` would remain unchanged (with the attached file still in-memory). With a new `turbo:before-permanent-element-render` available, there's an opportunity to do application-specific merging: ```js addEventListener("turbo:before-permanent-element-render", ({ target, { detail } }) => { detail.render = (currentElement, newElement) => { currentElement.setAttribute("class", newElement.getAttribute("class")) currentElement.setAttribute("aria-describedby", newElement.getAttribute("aria-describedby")) } }) ``` [permanent element]: https://turbo.hotwired.dev/handbook/building#persisting-elements-across-page-loads

seanpdoyle mentioned this pull request Jul 6, 2022

Restore focus when transposing Permanent Elements #436

Merged

seanpdoyle force-pushed the before-permanent-element-render-event branch 2 times, most recently from 129eaa3 to 829fbe0 Compare July 6, 2022 18:46

seanpdoyle mentioned this pull request Jul 6, 2022

Turbo Stream operations ignore [data-turbo-permanent] #623

Closed

seanpdoyle force-pushed the before-permanent-element-render-event branch 3 times, most recently from 8e412f1 to ebabd5f Compare July 18, 2022 04:33

seanpdoyle force-pushed the before-permanent-element-render-event branch from ebabd5f to c037d3e Compare July 29, 2022 21:20

seanpdoyle force-pushed the before-permanent-element-render-event branch 5 times, most recently from 8444071 to 316929f Compare September 14, 2022 19:35

seanpdoyle force-pushed the before-permanent-element-render-event branch from 316929f to dbef5c4 Compare September 27, 2022 13:51

seanpdoyle force-pushed the before-permanent-element-render-event branch from dbef5c4 to a8924f4 Compare November 20, 2022 15:54

seanpdoyle force-pushed the before-permanent-element-render-event branch 2 times, most recently from 5bab75b to 2d30dc2 Compare December 31, 2022 23:06

seanpdoyle force-pushed the before-permanent-element-render-event branch from 2d30dc2 to 7ef02be Compare February 4, 2023 18:03

seanpdoyle force-pushed the before-permanent-element-render-event branch from 7ef02be to b51399a Compare July 21, 2023 15:37

seanpdoyle force-pushed the before-permanent-element-render-event branch 2 times, most recently from c82aa26 to a7b66e5 Compare September 14, 2023 14:07

seanpdoyle force-pushed the before-permanent-element-render-event branch from a7b66e5 to a188c94 Compare September 14, 2023 14:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce `turbo:before-permanent-element-render` #622

Introduce `turbo:before-permanent-element-render` #622

seanpdoyle commented Jul 6, 2022 •

edited

Loading

dhh commented Jul 6, 2022

seanpdoyle commented Jul 6, 2022

seanpdoyle commented Jul 6, 2022 •

edited

Loading

kstratis commented Aug 7, 2022

seanpdoyle commented Aug 7, 2022

kstratis commented Aug 9, 2022 •

edited

Loading

kstratis commented Aug 12, 2022

Introduce turbo:before-permanent-element-render #622

Are you sure you want to change the base?

Introduce turbo:before-permanent-element-render #622

Conversation

seanpdoyle commented Jul 6, 2022 • edited Loading

dhh commented Jul 6, 2022

seanpdoyle commented Jul 6, 2022

seanpdoyle commented Jul 6, 2022 • edited Loading

kstratis commented Aug 7, 2022

seanpdoyle commented Aug 7, 2022

kstratis commented Aug 9, 2022 • edited Loading

kstratis commented Aug 12, 2022

Introduce `turbo:before-permanent-element-render` #622

Introduce `turbo:before-permanent-element-render` #622

seanpdoyle commented Jul 6, 2022 •

edited

Loading

seanpdoyle commented Jul 6, 2022 •

edited

Loading

kstratis commented Aug 9, 2022 •

edited

Loading