live_toast icon indicating copy to clipboard operation
live_toast copied to clipboard

handle_params, push_patch & put_toast

Open tomeq93 opened this issue 1 year ago • 6 comments

Hey, If I use put_toast with combination of push_patch in handle_params I do not have toast visible and it works with default put_flash, any ideas? Thanks, great lib btw :)

tomeq93 avatar Jun 15 '24 04:06 tomeq93

Interesting... I will try to replicate but if there's any way you can provide a minimal repro it would help a lot. Thanks!

srcrip avatar Jul 03 '24 12:07 srcrip

tbh. I can't reproduce it on newest version {:phoenix_live_view, "~> 1.0.0-rc.6", override: true},

¯\ (ツ)

tomeq93 avatar Jul 03 '24 12:07 tomeq93

ahh forgot it was from handle_params -> git patch with repro

From bd35de5be3994f96786a8f4266a1697405871e1b Mon Sep 17 00:00:00 2001
From: example <[email protected]>
Date: Wed, 3 Jul 2024 15:04:32 +0200
Subject: [PATCH] repro

---
 demo/lib/demo_web/live/home_live.ex | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/demo/lib/demo_web/live/home_live.ex b/demo/lib/demo_web/live/home_live.ex
index 8a0639c..ebda584 100644
--- a/demo/lib/demo_web/live/home_live.ex
+++ b/demo/lib/demo_web/live/home_live.ex
@@ -14,7 +14,7 @@ defmodule DemoWeb.HomeLive do
     "action" => nil
   }
 
-  def handle_params(_params, _uri, socket) do
+  def handle_params(params, _uri, socket) do
     socket =
       socket
       |> assign(:settings, @default_settings)
@@ -23,7 +23,15 @@ defmodule DemoWeb.HomeLive do
 
     Logger.info("Mounted")
 
-    {:noreply, socket}
+    if params["test"] do
+      {:noreply,
+       socket
+        |> put_flash(:info, "I'm working")
+      #  |> LiveToast.put_toast(:error, "I'm not")
+       |> push_patch(to: "/")}
+    else
+      {:noreply, socket}
+    end
   end
 
   def handle_event("change_settings", payload, socket) do
-- 
2.45.2

tomeq93 avatar Jul 03 '24 13:07 tomeq93

I have a plan for how to fix this. It should be ready in a few days.

srcrip avatar Jul 06 '24 13:07 srcrip

So I have a branch that fully 'fixes' this issue. It is however quite complicated. It requires basically, on every call to put_toast, pushing both a flash and a toast, and deduping by deleting the flash in the Live Component if it receives an update that contains both a flash and toast of the same kind and message.

That's a little janky, but testable and verifiable I guess. The bigger thing is it also requires passing said toast synchronously through params to the LC. That is a pretty big change that requires a lot to make work. It also means that the param most likely needs to be set to a temporary_assign. That's kind of a problem, cause whatever LV is mounted to the router would have to be responsible for that. We could however make a __using__ macro that does this and just expect the user to have to put it in there web module LV callback so it fires for every LV.

There is another alternative which is pretty simple: we can also just check the socket for the presence of the redirected attribute in put_toast, and if it's there, put a flash on instead of a toast. This works great but now the order that you call the functions matter. You'd have to call push_navigate before put_toast. It has the advantage of being an extremely simple fix though.

The final option is to just say well, if you want messages to work across navigates just use put_flash That's an option, but I don't know if I'm fully satisfied with it.

Like I said, I have a branch that kind of makes it all 'just work', but it's complicated. I think I need to test it more myself and make sure it doesn't add some crazy levels of jank. I can even push it up for others to try at some point soon. If it's not a suitable fix though I think I'll lean towards option two, which is literally a one line change and just requires you to change your user-space code a little bit.

srcrip avatar Jul 09 '24 17:07 srcrip

Thanks for detailed response, I prefer using put_flash in this cases explicitly in my codebase (would be nice to have readme description to not be surprised) instead of library doing this under the hood, regarding complicated logic maybe you could ping on elixirforum if it's the right / only way 🤔

tomeq93 avatar Jul 10 '24 04:07 tomeq93

Can we just auto-dismiss the flash after the redirect, after some number of seconds?

sergchernata avatar Nov 07 '24 22:11 sergchernata

@srcrip Is there any update on this? I didn't see the branch with the experimental fixes in this repo.

MarkoH17 avatar Dec 05 '24 17:12 MarkoH17

@MarkoH17 and others, I've just opened a PR for this issue: https://github.com/srcrip/live_toast/pull/28

Sorry this one was quite a while coming. It was a very complicated problem to get right and I didn't want to rush it out the door.

srcrip avatar Dec 18 '24 22:12 srcrip

Closing now since #28 has been implemented.

srcrip avatar Dec 27 '24 15:12 srcrip