-
Notifications
You must be signed in to change notification settings - Fork 428
Partial replacement of new pages #448
Comments
@dhh interesting to see this discussion taking place. A few comments: Rouging out a few examples: <h1>Dashboard</h1>
<nav data-turbolinks-partial="sidebar">
<a data-turbolinks-partial="order-count">Orders <%= @open_order_count %></a>
</nav>
<h1 data-turbolinks-partial="order-count">You have <%= @open_order_count %> orders that need your attention.</h1>
<%= link_to "Update your account to have more features", onclick="Turbolinks.visit(url, change: ['sidebar'])" %>
<!-- option 1: care less about what changes and more about what doesn't -->
<%= link_to "Capture and fulfill all orders", onclick="Turbolinks.visit(url, keep: ['sidebar'])" %>
<!-- option 2: you have context on what's changing and are likely performing a more focused partial replacement -->
<%= link_to "Capture and fulfill all orders", onclick="Turbolinks.visit(url, change: ['order-count'])" %> GET vs. POSTA good chunk of Turbograft's design is driven by the fact that you'll be mutating data and then refreshing one or many sections of the page as a result of that change. Turbolinks is different in that it's purely for a faster UX on GET's. For this reason, I'm curious what flow you see the
change keys can be greedyIn the example above, if you wanted to display an open order count in your sidebar nav, anytime you change the outer key This hasn't caused us any real pain yet, but there have been a few cases where we avoided putting keys on a wrapping node, and became more selective on the child nodes instead because of this. we try to think of our keys as what-data-changes and not what-sections-changeSince Turbolinks is more GET than POST, I actually may prefer the Of note, most of our pages only have 1 or 2 unique change keys on them, with additional a rough thought on partialsA long long time ago, we had the idea that one day we may be able to hook in to rails partials to have speedier page rendering if we introduced the concept of refresh/section keys at a server level. We haven't done this, performance hasn't been an issue and it's probably an over optimization for us however I'm curious what your thoughts are on it playing more with Turbolinks. The idea was basically layouts/application.html.erb <%= render 'main_nav', "data-turbolinks-partial" => "user_account" %>
<%= yield %> views/foos/index.html.erb <%= render 'foo_table', "data-turbolink-partial" => "foos" %>
<%= render 'bar_table', "data-turbolink-partial" => "bars" %> Turbolinks.visit(url, change: ['foos']) // only ends up rendering foos partial server side. If rails would somehow acknowledge (HTTP headers?) change keys, we could make Haven't really thought about this idea in a while, but this PR sparked it again. Interested in seeing where this goes! |
Quick note. I think we'd use the name "partial" to refer to a named section, so it'd be: <h1>Dashboard</h1>
<nav data-turbolinks-partial="sidebar">
<a data-turbolinks-partial="order-count">Orders <%= @open_order_count %></a>
</nav> One concern is indeed whether partial is already an overloaded word. I'm starting to think that it probably is. Turbolinks partials wouldn't necessarily correspond fully to Rails partials. So how about At Basecamp, I see the following use cases currently: • Keep a navigation sidebar that includes per-user stuff separate from the perma pages for, say, a message. Our sidebar is lazy loaded, but once it's there, page changes shouldn't eject it. I don't think the nested issue is that big of a deal. I'm fine with the fact that if you replace a specific section, any permanent sections within it are overwritten. Interesting thoughts on the partial element, but at least for us, it's not really an issue. We russian doll cache all our partials. If something hasn't changed, then it won't take material time to render it, because it's coming out of a cache. But it's worth thinking about as a separate element. |
re: both
and
Agreed that I'm sensitive to thinking of partial replacements in the space of "what sections need updating on my page", because it feels more SJR to me. This was a big discussion early in our Turbograft days, I had summarized it here. Our end result for a simple add comment flow looks like this with turbograft. This is getting a little nitpicky but the latter example feels a little more "data driven" to me, which was one big takeaway from our Batman days. Our goal was a single render path on your view that through HTML + ERB describes as much of the flow of your app as possible. In Batman, we had DOM bound to data and the view described the flow, all you had to do was get your data in to the right state. Perhaps something more like A simple example being: <nav>
<a data-turbolinks-watch="comments">Comments (<%= @comments.count %>)</a>
</nav>
<!-- a bunch of other stuff -->
<div data-turbolinks-watch="comments">
<%= @comments.each do |comment| %>
<!-- ... -->
<% end %>
</div>
<!-- form to add a comment with -->
<!-- Turbolinks.visit(url, change: ['comments']) --> vs. <nav>
<a data-turbolinks-section="comments-nav-counter">Comments (<%= @comments.count %>)</a>
</nav>
<!-- a bunch of other stuff -->
<div data-turbolinks-section="comments-list">
<%= @comments.each do |comment| %>
<!-- ... -->
<% end %>
</div>
<!-- form to add a comment with -->
<!-- Turbolinks.visit(url, change: ['comments-nav-counter', 'comments-list']) --> I just see people naturally seeing |
I don't mind the server-side knowing which elements it just updated, and basically passing on optimization hints to the rendering phase. The cool thing about something like this: def create
@post.comments.create! comment_params
redirect_to @post, change: :comments
end Is that it'll work just as well whether you're in Turbolinks mode or not. So you still have a rendering flow that's completely unaltered regardless of whether it's a first or subsequent load or it's a POST or a GET. The second thing is that I don't think pages are going to get littered with this. Our initial intention was just to add data-turbolinks-permanent, but even with this more flexible proposal, as you've found, it's usually just 1 or 2 sections that need replacement. I'm not as big a fan of watch because we're not setting up any event flow here. The elements are not doing the watching, they're the "dumb" part of this. They just have a name that we can refer to in the place that is triggering the action and that does have the knowledge of which parts changed. |
It's funny, btw, that we're talking about the comment example. I just ripped out some code that basically did I'm finding that in a lot of cases, and I think it'll be even more true when we can do section updates like this. You just won't need as much SJR. You're still doing remote: true forms, but their result will most often just be a Turbolinks.visit response. |
I'm interested in seeing where we could go with the OK, <a remote="true" href="/orders/1/close" remote-method="POST" refresh-on-success="orders">Close order</a> This was our solution to common and simple The elements aren't watching, but they're hinting that they care about this set of data. We have no SJR in Shopify with Turbograft and mitigate repetitive |
Yeah, I think that's the part I'm the least hot on. That the dom would basically include conditional logic. To me that's a bridge too far. IMO, the controller knows best how to route this. So it could do: def create
if @post.comments.create(comment_params)
redirect_via_turbolinks_to @post, change: :comments
else
redirect_via_turbolinks_to new_post_comment_url(@post), change: :error, error: 'Something not right!'
end
end I think flow logic belongs in the controller. I'm 👍 on cutting down on SJR by having these partial updates that allow state to be retained. I think there's really something here. |
I'm not quite getting your point on conditional logic in the DOM.
|
When I say conditional, I mean refresh-on-success/refresh-on-error/full-refresh-on-error-except – that the DOM is specifiying these different paths. In my opinion, it's the responsibility of the controller to direct to what happens after an interaction – especially when it's a multi-path flow, like error handling. I think the beauty of partial rerendering is exactly that you don't need to do |
Anyway, I think you're right that it's not likely that these setups are generally going to be so deeply intricate. Most of the time you're just going to want to either refresh just once specific section, or have one section that you keep around. I think we could start by moving that stuff over, if you're interested? Then we can see what we can possibly build on from there. I think we absolutely share the same vision, though. Simply re-render whole pages most of the time. A few nibs at stable elements. And this will get us 90% of the way. |
👍. One restriction we added to partial replacement nodes is they need an ID as well as the key so we can find the corresponding note in the replacement DOM. There's also a few cases like "what if the node goes away or is created" that need to be handled. Sort of like a poor mans React diff. I'll try to find a little time this week to open a PR with some partial replacement prototyping. |
Why can't you find the nodes via ('[data-turbolinks-section=X]')? And yeah, this def is similar in many ways to the React stuff. Hopefully we can get away with far less complexity. |
The I think we also considered tracking it in a simple index like front-buffer DOM {
0: [div id="customer_list"]
1: [div id="customer_bubble"]
2: [div id="customer_modal"]
} However without some kind of unique identifier (we used back-buffer DOM {
0: [div id="customer_list"]
1: [div id="customer_modal"] // how do I know this index 1 is actually index 2 in my front buffer?
} |
Ah, yes, of course. Makes sense. Although I’d expect that the normal case would be that you only have the 1:1? But I guess we need to deal with both.
|
This discussion of the future solves a current problem for us. So 👍 |
I'm still planning on getting around to this in a couple weeks, been swamped lately. I'm going to take a shot at pulling out the partial refresh stuff from Turbograft and hook it up to a |
🤘
|
With this you can keep an audio/video playing between page navigation so 👍 |
I've been following the conversation and have an alternative idea: there's parity between the view's cached fragments and the sections that need to be updated on an XHR request. Why not have the (view) cache blocks double as the section definitions? When an xhr request is made, the page is rendered but only the cache sections that are re-rendered (non-cached) are sent back to the client (a hash of changes). Some lightweight js can find the prior section and replace it with the new render? By having a 'section' definition, the developer gets the initial Russian Doll fragment caching along with defining appropriate sections for client side updates. I'll do a proof of concept and open a separate PR. |
Yeah, that's a good point @aantix. I had suggested the same to @dhh but the only downside is that now you're coupling the fragment caching to it (maybe not necessarily a downside :)). Two considerations:
I'm also really interested in seeing if this works, feel free to ping me on PRs. I will likely play around with this at the end of Feb (we have a few hack days coming up), maybe we can bounce a few ideas around before then. |
Great considerations @kristianpd.
I'll definitely ping you on the PR when it's done. |
I think the auto-detection is an interesting experiment, but the underlying power to update selectively is still valuable. You won't always be able to follow the automatic inference, and you'll need the lower level API to do it anyway. So I'm most interested in getting the manual building blocks in place first, then trying to level-up on top of that afterwards. |
I just leave it here. |
This is an interesting discussion, and if you take it to the extreme of tracking every DOM node automatically you can get something like what I've put together: https://github.com/ssorallen/turbo-react turbo-react doesn't modify Turbolinks. It monkeypatches The HTML -> JSX -> React combination behaves similar to ideas mentioned above: it tracks DOM nodes with unique IDs ( |
Ross, that’s an interesting project. I’m curious as to when it matters to truly flow through react vs just doing these innerHTML replacements. I get it in the actual React case, because the logic is all client-side, so you’re basically just doing a redraw. But in our case, redraw only happens in response to a HTTP request. And in that case, I don’t think innerHTML is the bottleneck. The bottleneck is likely to be the remote operation, and the transfer of the response. Happy to be enlightened to otherwise. I like React because it’s the same motivation as Turbolinks: Avoid client-side state as much as possible! It’s just whether the updates are triggered from client or server.
|
@dhh Yup, I don't think innerHTML is a bottleneck, and I didn't mean to imply turbo-react was fixing a bottleneck or any problems in Turbolinks. I added React as an experiment to have finger-grained automatic mutations than css: .foo { transition: background-color 0.5s; }
.bar { background-color: yellow; }
.baz { background-color: blue; } doc1: <body>
<div class="foo bar"></div>
</body> doc2: <body>
<div class="foo baz"></div>
</body> Turbolinks replaces the body with the body tag of doc2, but turbo-react finds that the minimum change to get from doc1 to doc2 is actually |
Ross, gotcha. That seems perfectly legit for that use case. Thanks for putting it out there.
< </ — |
re: @ssorallen
That's amazing! I was just musing the other day that some kind of DOM diffing instead of outright replacements would be the only way for TurboGraft to work well with replacements & CSS transitions without adding some kind of animation lock (cc @tmlayton) |
re: turbo-react; That is great. @ssorallen any reservations about moving it beyond an experiment? |
@tmlayton I don't want to hijack this thread. Do you want to open an issue on turbo-react to chat? Happy to chat there. |
Some great ideas from https://github.com/Shopify/turbograft that we should migrate into Turbolinks. The first is partial replacement. I was initially skeptical to this idea, but have been won over by seeing how it can really work well with permanent elements of a page like shopping carts or sidebars or other pieces with state.
The big win from Turbolinks remain that the initial hit will generate the full page, and that you won't have to tailor server-side rendering on subsequent requests, even when you're only doing partial updates.
I'd like to propose the following API:
So you can name a partial with data-turbolinks-partial. These named partials can then be referred to in the Turbolinks.visit call through the change/keep options.
Marking an element with "data-turbolinks-permanent" will mean that it is not updated by any Turbolinks.visit call, except if you ALSO give the same element a partial name AND name that partial as part of a Turbolinks.visit/change call.
The text was updated successfully, but these errors were encountered: