Redesign to work across navigations #28
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hey hey hey. Sorry this was a while coming. But it was a tricky concept to get right. This should resolve some problems raised in:
push_navigate
#25State management
Originally, the functionality of the state management was very simple. There was one toasts list stored on the socket, and the flash messages, and that was pretty much that.
The problem that was raised in #25 and #9 was that this method did not work properly across page navigations. This is when you do something like this:
In this situation, what happens is we add the toasts to the socket, but because a page nav happens immediately afterwards: we don't see anything.
What we would want to happen in this scenario is for the library to somehow know that a navigation is happening and pop a flash on instead, since flashes do work across navigations. Unfortunately, there is nothing built in to Phoenix that would allow our LiveComponent to know that a navigation is occurring.
There is a workaround however. What we can do instead is make
put_toast
always send both a toast and a flash, in the LiveComponent do this:The latter is what would now happen when doing a navigation with
put_toast
, like in the snippet above. It complicates the scenario where we are not navigating however. In that case, we need to 'dedupe' the flash and toast in the LiveComponent. That code is kind of complicated in this update, but it appears to work ok.Clearing the flash
There's one last wrinkle though. If we write out 'dedupe' logic, and simply do that, then on the next update we will get the flash message coming back. It's not enough to clear it from the LC's copy of our state. We need to remove it from the socket entirely. Luckily there is functionality to do this built in to Phoenix.
This was the part that held up this change for so long. The issue was, I didn't initially have a great conception of how to make this clearing of the flash happen only if we are not navigating (the opposite problem of the first case). Eventually a solution came to me. Instead of clearing the flash on the server, we simply send a message to the frontend, and have it send a message back to use to clear the flash. In theory if you had a bunch of messages come in at once you could maybe get a glimpse of a flash that should have been removed but in practice I think this works ok.
The reason this works is because in the case of us navigating, the JS on the new page will never receive this message, and so it renders the flash on the next page just fine.
The whole thing is a very specific confluence of factors that make it all work, but it seems to work ok and solve some of the initial problems satisfactorily.
Future workarounds
If in the future we get some kind of way to detect a navigation is ongoing or something, we could probably simplify this code and not have to do some of this. It's probably not going to happen though because updates in the LC stop as soon as a navigation starts. But perhaps there is some way I didn't think of to simplify further, or perhaps future changes will open up such an opportunity.
Also it's worth pointing out that these workarounds are only necessary to support the key feature of LiveToast that it works with both flashes and the new toasts added by the library. If you wanted to use a library that just added toasts, you wouldn't have to think about any of this, but you also wouldn't be able to just call one function (
put_toast
) and have it work in all circumstances (with and without navigations) and you wouldn't be able to unit test it in your tests as easily.Other workarounds
One of the early attempts I took at this problem was to simply check on the socket the flag that gets set when we're navigating. I know I said earlier such a thing doesn't exist, but it does: it just exists on the socket. In the LC we don't have access to it.
However when we run the
put_toast
function, we can check if the socket has the navigation flag set, and then fallback to just sending a flash. The problem is if we do this the library is then dependent on the order which you call eitherput_toast
orpush_navigate
. This only works if you callpush_navigate
beforeput_toast
, and I wasn't happy with that caveat. The solution in this PR instead should work regardless of what order the two happen in, because it relies on the frontend to resolve things. Note that we could still add this check in to avoid a little unnecessary work if you do happen to call them in that specific order. I chose not to do that yet so that if there was any bugs in my code they'd be found quicker.Testing
This does complicate the logic in the library a little bit more so I'd like some feedback from people using the library, if they could test this branch, before merging this in to main.