Preventing Reload of Permanent Element?

swamidass commented 2 years ago

Turbo is really great. Thanks for building it.

So, I coded up a video tray to persist across navigation. A user can click a button, which will load an iframe into a fixed positioned element that persists across navigation. This sort of works, but not quite. On navigation, the permanent element flashes in and out, and the iframe has to reload.

Here are some iterations I tried;

<html>
<head>
</head>
<body>

{{ rest of page }}

<div id="video-tray" turbo-data-permanent>
  {{ iframe dynamically added here }}
</div>
</body>
</html>

That has the problem with the iframe load on navigation. So I tried this:

<html>
<head>
</head>
<body>

<turbo-frame id="main_outlet" data-turbo-action="advance" >
  {{ rest of page }}
</turbo-frame>

<div id="video-tray"  turbo-data-permanent>
  {{ iframe dynamically added here }}
</div>
</body>
</html>

This solves the problem with the video tray! It does not reload any more on navigation.

HOWEVER, a whole lot of other far worse problems arise. The head section is not merged on navigation. This doesn't end up as a true solution.

So we can try this:

<html>
<head>
</head>
<body>

<turbo-frame id="main_outlet" data-turbo-action="advance" target="_top">
  {{ rest of page }}
</turbo-frame>

<div id="video-tray"  turbo-data-permanent>
  {{ iframe dynamically added here }}
</div>
</body>
</html>

This ends up being identical to the first attempt. There is a reload flash on navigation.

So, how can this problem be solved without creating new problems?

swamidass commented 2 years ago

Looking at some past issues (e.g. #226) it seems that the problem is that permanent elements are removed from the dom, and then added back in for all cases.

Can there be a way, in some simple cases, of not removing the permanent elements from the DOM? Doing this in the general case might be difficult, perhaps requiring DOM diffing. But, if the permanent element is a direct child of the body element it would not require DOM diffing at all. Just leave it where it is. That is particularly true if it's fixed positioned, because then even its order in the DOM doesn't matter.

What are your thoughts?

swamidass commented 2 years ago

Another option would be to merge in the head into the _top page with a turboframe navigation. Perhaps that could be done in a custom event handler or by a new data attribute? Is there a straighforward way to trigger merging the head?

rainerborene commented 2 years ago

You can solve this problem by wrapping your app into another root element instead of the body element. I'm currently using this monkey patch to change the body reference internally. Here's the code:

import { PageRenderer } from "@hotwired/turbo"

Object.assign(PageRenderer.prototype, {
  assignNewBody() {
    const container = document.querySelector("#app")
    const newContainer = this.newElement.querySelector("#app")

    if (container && newContainer) {
      container.replaceWith(newContainer)
    } else {
      document.body.replaceWith(this.newElement)
    }
  }
})

And markup example:


<html>
  <head>
    <!-- ... -->
  </head>
  <body>
    <div id="app">
    </div>

    <div id="persisted-video">
    </div>
  </body>
</html>

swamidass commented 2 years ago

Thank you! I'll try it out and see if it works. That's deep enough in the internals I wouldn't have been able to figure it out earlier.

For anyone listening in, I also tried morphdom, diffhtml, and nanodom. None of them worked due to incompatiblities with turbo I couldn't find the way to work around.

swamidass commented 2 years ago

So it did work, with a catch. They key thing, it turns out, is removing the turbo-data-permanent attribute.

So this will work:

  <body>
    <div id="app">
    </div>

    <div id="persisted-video">
    </div>
  </body>

But this won't:

  <body>
    <div id="app">
    </div>

    <div id="persisted-video" data-turbo-permanent>
    </div>
  </body>

Ironic isn't it!

Thanks for your help though. It works now!

swamidass commented 2 years ago

@dhh @sstephenson @ctrochalakis would you be interested in a PR that makes this part of Turbo as an option?

hotwired / turbo

Preventing Reload of Permanent Element? #499