Open YummYume opened 1 year ago
Thanks for this, and glad you're having fun with it :)
I've not yet implemented animations for turbo frames as their lifecycle events are a little different (e.g. there isn't a turbo:frame-visit
event).
From my brief experiments it seems like turbo:visit
isn't fired on standard frame navigations and therefore Turn won't animate. However, when a frame visit is promoted to a page visit via a data-turbo-action
attribute, then it will trigger a turbo:visit
. Unfortunately there's currently no way to determine when a turbo:visit
, originates from a frame visit or a standard full link click. https://github.com/hotwired/turbo/pull/750 might help with that, but I reckon adding a turboFrameId
attribute to the turbo:visit
event details could also be another option.
…oh and feel free to add a PR for that tailwind typo. Thanks!
Thanks for your answer @domchristie !
Indeed, I forgot to mention it happens when the frame's navigations are promoted to visits (which makes sense). It would make sense for Turn to handle frames separately from simple visits with drive, whenever Turbo makes this possible. It would make it very easy to handle state change within a frame, even if it's not promoted to a visit.
For a quick summary of how I currently disable the animations within a frame, I use a Stimulus controller on the root html element, which exposes Turn's start
and stop
methods. Then simple :
<turbo-frame data-action="turbo:before-fetch-request->turn#stop turbo:visit@window->turn#start" data-turbo-action="advance" id="some-frame">
...
</turbo-frame>
I made a PR to fix the small typo!
I use a Stimulus controller on the root html element, which exposes Turn's start and stop methods.
That looks great! Thanks for sharing.
I had a look at adding a frameId
attribute to the turbo:visit
event details. Unfortunately it's not straightforward. We'd need to pass the frame ID into the frame options when creating the visit here: https://github.com/hotwired/turbo/blob/4593d06ce58d17af5b17495ad8524eaa9bc2f5d2/src/core/frames/frame_controller.ts#L379-L406. The frame
option already exists and it's used for programmatic visits: Turbo.visit(url, { frame: 'frame-id' })
. Session#visit
uses this option to alter it's behaviour, which I don't think we want in this case. (Although I could be wrong!)
Hi @domchristie ,
The best turn (lol) of event here would be for Turbo to add specific events to frames. Or pass a frameId
as you mentioned in the visit event. If frames could be handled separately from a simple "visit" action (much like a stream) it would allow for a better handling of things (because they would all have their own logic in what to animate).
For example, visits would animate the whole page. Frames would only animate child nodes. And streams could animate re-rendered nodes but that may be too complicated (or not).
That's only my opinion though!
There might be some other ways to determine when a link click targets a frame. A combination of the following might help:
turbo:before-fetch-request
is dispatched on the frame before turbo:click
, so we can start tracking a frame visit from thenpreventDefault
on the turbo:click
event, so we might be able to check event.defaultPrevented
Hey @domchristie, checking the event.defaultPrevented
sounds like a good idea, but it could lead to false positives if something else prevented the event? At this point, I still don't understand why we can't just have a simple way of knowing when a visit is done from a frame.
At this point, I still don't understand why we can't just have a simple way of knowing when a visit is done from a frame.
The code for tracking a click through to creating a Visit
and then dispatching the lifecycle events is quite complex. Changes must also consider the impact on the iOS/Android adapters which have restrictions on what type of JavaScript object type can be sent to them (e.g. you can't send them a <turbo-frame>
DOM node). So adding a frame
detail to lifecycle events is not straightforward.
Also, it doesn't look like external pull requests are being merged at the moment. Feel free to dig around though :) You might find a nice way to patch it.
@domchristie Thanks for making it clearer. I will perhaps try to dig around or find a workaround for it... All I could think of was that, Turbo adds a turbo-frame
header to the request it sends when navigating using a Turbo frame, so would it not be possible to somehow check if this header is present and do something with it? But it doesn't seem like Turbo dispatches anything else than the URL being navigated to (and the action).
All I could think of was that, Turbo adds a turbo-frame header to the request it sends when navigating using a Turbo frame, so would it not be possible to somehow check if this header is present and do something with it?
Oh nice idea! Yeh, it’ll be interesting to see if that could work. Off the top of my head, it still might be tricky to accurately match up the fetch event with the visit event, but an estimate might be doable
I tried playing around with it a bit, and it seems that only POST requests include the Turbo-Frame
header... Or at least, the turbo:before-fetch-request
event doesn't include the header on visits.
Hello, thanks for making this library, it's a blast :fire:
One thing I am wondering is, would it be possible to only animate the elements inside a Turbo frame (when navigation or form submit happens and targets a frame)?
From what I understand, it doesn't look like it's possible at the moment looking at the disabling animations section. But would it be possible once that is fixed? It would help a lot as right now, the only solution I found is to disable Turn when the navigation starts, show some skeleton and then re-enable Turn when the frame is rendered.
By the way, I think there's a typo in the Tailwind usage :
should be