filamentphp / filament

A collection of beautiful full-stack components for Laravel. The perfect starting point for your next app. Using Livewire, Alpine.js and Tailwind CSS.
https://filamentphp.com
MIT License
19.6k stars 2.99k forks source link

Vertical notification animation broken #7395

Open zepfietje opened 1 year ago

zepfietje commented 1 year ago

Donate 💰 to fund this issue

Fund with Polar

fseesys commented 10 months ago

While playing with the demo site at v3.2.2, I noticed that the Notification Animation does slide in animated, but the direction seems to be reversed. It slides in from the left-to-right. It would be perfect for it to be corrected back to right-to-left.

Thanks.

zepfietje commented 10 months ago

@fseesys, could you please create a bug report for what you're experiencing? I can't reproduce it in the demo.

fseesys commented 9 months ago

@fseesys, could you please create a bug report for what you're experiencing? I can't reproduce it in the demo.

Sorry for replying late, but it seems to be fixed in v3.2.27 of the demo.

Thanks!

ju5t commented 2 months ago

@zepfietje I'm not sure if this is related, if you need to me open a new issue please let me know.

We have the following code:

$quote->wasRecentlyCreated
    ? $this->redirectRoute('quotes.edit', $quote->id)
    : Notification::make()->title(__('general.saved'))->success()->send();

$this->dispatch('log:fresh');

The event is used to refresh our audit log. The audit log has its own Livewire component.

When the notification is sent, it drops down from the top right to the bottom right of the screen. As soon as we remove the dispatch or remove #[On] from the audit log component, the notification slides in from the right, as always does.

zepfietje commented 2 months ago

Once this issue is fixed, the issue you're experiencing will probably be fixed too, @ju5t. If you want to look into a fix, any help is much appreciated since I've spent a lot of time on this one already (happy to help work on a fix though 🙂).

ju5t commented 2 months ago

@zepfietje I will schedule some time to look into it tomorrow. Can't promise I find a solution though, my JS is a bit rusty. Are there any other places to look for the animation aside from notification.js#L76?

zepfietje commented 2 months ago

No worries, @ju5t, any insights are welcome! You linked the right place indeed. It worked fine in v2, but couldn't get it to work with Livewire v3 yet.

ju5t commented 2 months ago

@zepfietje I found something.

When we're not dispatching the log:fresh event there are 3 requests in the network tab:

  1. We call our action. This will send the notification.
  2. The second request has the notificationsSent event.
  3. And finally we see notificationClosed when the notification disappears.

When we dispatch our log:fresh event it's different. There are two notificationSent events. The first is combined with our event, the other is on its own.

It's related to NotificationsServiceProvider.php#L52. This uses:

on('dehydrate', function (Component $component) {
...
});

In our situation, two components are dehydrating at about the same time. The Notification component hasn't had the chance yet to pull the notification from the session. This results in two events getting dispatched. Edit: it doesn't result in two events being displayed though.

Our component is called ActivityLog. As a workaround I added:

if ($component instanceof ActivityLog) {
    return;
}

This prevents notifications being sent from that component and solved it for us. Obviously this is not a solution, I haven't had the time to think about that yet. And I don't know if this is the cause of other issues with notifications either.

Food for thought :)

ju5t commented 2 months ago

I spent some more time on this today, but I can't find a clean solution.

What worked for our issue is 'abusing' sessions to see if we've dispatched the event yet. Perhaps someone with more knowledge on the Livewire internals can come up with a cleaner way of doing this. Preferably so we can ditch the dehydrate logic altogether as it's somewhat redundant.

Here's our code to try out. In the service provider:

if (session()->get('filament.notificationsSent', false) === false) {
    $component->dispatch('notificationsSent');
    session()->put('filament.notificationsSent', true);
}

And inside the Notifications Livewire component we add this to the pullNotificationsFromSession() method:

session()->forget('filament.notificationsSent');
mateusgalasso commented 1 week ago

I spent some more time on this today, but I can't find a clean solution.

What worked for our issue is 'abusing' sessions to see if we've dispatched the event yet. Perhaps someone with more knowledge on the Livewire internals can come up with a cleaner way of doing this. Preferably so we can ditch the dehydrate logic altogether as it's somewhat redundant.

Here's our code to try out. In the service provider:

if (session()->get('filament.notificationsSent', false) === false) {
    $component->dispatch('notificationsSent');
    session()->put('filament.notificationsSent', true);
}

And inside the Notifications Livewire component we add this to the pullNotificationsFromSession() method:

session()->forget('filament.notificationsSent');

Could you explain better how you did this?

ju5t commented 1 week ago

@mateusgalasso this is not user land code. It needs to be solved in Filament.