Persist Elements in Advance Visits

[tleish]

In consideration of data-turbo-permanent attribute which works for restoration visits, what about advance visits?

For example, imagine using a turbo-frame to dynamically update a portion of the page. When navigating to that page, the DOM creates a FOUC each time. Couldn't data-turbo-permanent also apply to advance visits in this case? I apply the attribute to the turbo-frame tag (or it's parent) and I'm able to persist the updates across pages, avoid FOUC. Even if the turbo-frame updates from the URL, at least I'm able to avoid a FOUC by initially loading some expected content.

Note: this can also be any javascript which dynamically updates a part of the page after load.

We're currently supporting this using the attribute data-turbo-persist with the following code:

document.addEventListener('turbo:before-render', function (event) {
  persistElements(event.detail.newBody);
});

// data-turbo-persist
// copy elements from current document to new before rendering prevents update after load no not flash (e.g. left menu or header)
function persistElements(newDocument) {
  if (document.documentElement.hasAttribute('data-turbo-persist')) return;

  const persistElements = document.querySelectorAll('[id][data-turbo-persist]');
  for (const persistElement of persistElements) {
    const id = persistElement.id;
    const newPersistElement = newDocument.querySelector(`#${id}[data-turbo-persist]`);
    if (newPersistElement) {
      newPersistElement.outerHTML = persistElement.outerHTML;
    }
  }
}

I think this would be a good addition to Turbo. Perhaps using data-turbo-permanent is for both could be desired? They seem like one-in-the-same, though one deals with restoration visits while the other deals with advance visits.

[seanpdoyle]

Thank you for opening this issue!

As I understand [data-turbo-permanent], "permanent" means "permanent", regardless of navigation direction or other factors.

I've opened #623 to highlight the ways in which Streams break that consistency. It sounds like "advance" visits should be changed as well.

Unless I'm misunderstand the nuances in which they differ, [data-turbo-persist] seems redundant, and could be incorporated into the framework's treatment of permanent elements.

Would it be possible to distill the behavior (for example, the flash of unstyled content) into a JSFiddle or diff?

[tleish]

As I understand [data-turbo-permanent], "permanent" means "permanent", regardless of navigation direction or other factors.

Correct

I've opened #623 to highlight the ways in which Streams break that consistency. It sounds like "advance" visits should be changed as well.

Yes, that's another scenario

Unless I'm misunderstand the nuances in which they differ, [data-turbo-persist] seems redundant, and could be incorporated into the framework's treatment of permanent elements.

Correct, that's what I'm thinking. We just chose [data-turbo-persist] to avoid potential future conflict with the existing Turbo [data-turbo-permanent] attributes.

Would it be possible to distill the behavior (for example, the flash of unstyled content) into a JSFiddle or diff?

Hm. Not sure how to create turbo navigation inside a JSFiddle. Are you just looking for a gif or some visual which demonstrates this behavior?

[seanpdoyle]

Are you just looking for a gif or some visual which demonstrates this behavior?

I'd like to reproduce it locally in the test suite. If you have an idea on how to do that, I'd be happy to take over a pull request that introduces a failing test to the suite.

Otherwise, a JSFiddle or other scratch pad that demonstrates the issue would be helpful to better understand the places that Turbo is failing.

Not sure how to create turbo navigation inside a JSFiddle

Turbo is happy to function without a server-side component. Could you drive a page with an <a data-turbo-action="advance"> link pointed at static HTML?

[tleish]

I created a test and the functionality appears to now be working. I know this did not work on previous version at one point, I just had not tested it without our original patch.