Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vertical notification animation broken #7395

Open
zepfietje opened this issue Aug 2, 2023 · 11 comments
Open

Vertical notification animation broken #7395

zepfietje opened this issue Aug 2, 2023 · 11 comments
Assignees
Labels
Milestone

Comments

@zepfietje
Copy link
Member

zepfietje commented Aug 2, 2023

Donate 💰 to fund this issue

  • You can donate funding to this issue. We receive the money once the issue is completed & confirmed by you.
  • 100% of the funding will be distributed between the Filament core team to run all aspects of the project.
  • Thank you in advance for helping us make maintenance sustainable!
Fund with Polar
@zepfietje zepfietje added bug Something isn't working ui labels Aug 2, 2023
@zepfietje zepfietje added this to the v3 milestone Aug 2, 2023
@github-project-automation github-project-automation bot moved this to Todo in Roadmap Aug 2, 2023
@fseesys

This comment was marked as off-topic.

@zepfietje

This comment was marked as off-topic.

@fseesys

This comment was marked as off-topic.

@ju5t
Copy link
Contributor

ju5t commented Sep 14, 2024

@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
Copy link
Member Author

zepfietje commented Sep 15, 2024

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
Copy link
Contributor

ju5t commented Sep 15, 2024

@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
Copy link
Member Author

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
Copy link
Contributor

ju5t commented Sep 16, 2024

@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
Copy link
Contributor

ju5t commented Sep 17, 2024

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
Copy link

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
Copy link
Contributor

ju5t commented Nov 19, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Todo
Development

No branches or pull requests

5 participants