srcrip / live_toast

A beautiful drop-in replacement for the Phoenix Flash system.
https://toast.src.rip
MIT License
187 stars 15 forks source link

handle_params, push_patch & put_toast #9

Open tomeq93 opened 5 months ago

tomeq93 commented 5 months ago

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 :)

srcrip commented 4 months ago

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

tomeq93 commented 4 months ago

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

¯\ (ツ)

tomeq93 commented 4 months ago

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

From bd35de5be3994f96786a8f4266a1697405871e1b Mon Sep 17 00:00:00 2001
From: example <example@example.com>
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
srcrip commented 4 months ago

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

srcrip commented 4 months ago

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.

tomeq93 commented 4 months ago

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 🤔

sergchernata commented 4 days ago

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