search

turbolinks / turbolinks-classic

Classic version of Turbolinks. Now deprecated in favor of Turbolinks 5.
MIT License
3.54k stars 428 forks source link

Partial replacement for <tr> requires wrapping with extra <table> html #629

Open glennfu opened 8 years ago

glennfu commented 8 years ago

Here's a demo: http://codepen.io/anon/pen/avadXz

Basically, partial change of a <tr id="story:1" /> doesn't work. It has to look like <table><tr id="story:1" /></table> because when parsed, the "targetBody" that you inspect inside the swapNodes call has plucked out the outer tr and just left its contents.

I get that parsing a lone <tr> getting garbled like this is probably correct since it's not semantic by itself, but being able to predict and avoid this problem requires understanding the internals of how Turbolinks works. Perhaps something in the README would help, or is it possible for Turbolinks to warn about things like this?

alecdotninja commented 8 years ago

Hello @glennfu,

Having looked at your code example, it seems that this is not a typical usage of Turbolinks.replace. When Turbolinks::Redirection#render generates a JavaScript response, it will include the entire page's content as the first argument, not just the elements that are changed.

There are several reasons it makes sense to do this, not the least of which is the problem you just highlighted. Since Turbolinks depends on the browser's native DOM implementation, passing anything other than a well-formed document source is technically undefined. In this case, it seems that both of our browsers have decided to ignore <tr/>'s outside of <table/>'s.

Note that even with the <table/>, the missing document structure still materializes:

var replacement = document.documentElement.cloneNode();
replacement.innerHTML = '<table><tr></tr></table>';
console.log(replacement.innerHTML); // "<head></head><body><table><tbody><tr></tr></tbody></table></body>"

That being said, if you're willing to risk this problem, you could render a partial with only the changed elements, but this has the drawback of not working transparently when JavaScript is disabled.

glennfu commented 8 years ago

I've been inspired by DHH's RailsConf speech about Turbolinks vs modern js frameworks! My priority has been "how can I build a fast front-end experience using Turbolinks instead of Backbone/Ember/Angular" and not "Make my site work well without JavaScript". With that in mind, and since Turbolinks 3 is still so new, I've been having a lot of fun experimenting!

Partial and text rendering to Turbolinks with the 'change' attribute has given me a lot of really cheap and easy ajaxy features with very little front-end javascript. I would expect this kind of benefit to be highlighted especially in the case of pages that have a lot of html.