scenius-software / angular-toastify

A somewhat minimalistic clone of React Toastify.
https://scenius-software.github.io/angular-toastify/
17 stars 5 forks source link

Prevent duplicates (toast with same message) #33

Closed JoranLive closed 1 year ago

JoranLive commented 1 year ago

Hi,

This my first pull request, so please be indulgent.

I added an input parameter on "ToastifyToastContainerComponent" that prevents duplicated toasts (toasts with same message).

The input parameter "preventDuplicates" is set to false by default to keep the existing behaviour. Once it is set to true, when a toast is added, 2 cases:

Tested with the demo component.

No unit test were written for now.

I didnt update the changelog.

Lerke commented 1 year ago

Hey @JoranLive, thanks for the PR! I have tested it, and it seems to work well. I've also added two commits, one adding the ability to test this feature in the demo project, and one updating the version + changelog. That said, the total toast time of an already present toast does not seem to increase when creating an identical toast multiple times. Did I misread this feature:

if a toast with the same message already exist => the toast time is updated

https://user-images.githubusercontent.com/3271167/208665648-d21f31bd-336f-4156-b317-d53233f384fe.mp4

JoranLive commented 1 year ago

I did the same on the demo app but removed it before push because I wasnt know if it was wanted.

Regarding the toast expiration: you didnt misread. I was expecting that updating time was changing the expiration but I didnt test this use case (sorry for that). I'm busy for now but I will check the source code to know how I can manage this behaviour.

Lerke commented 1 year ago

It looks like the toast component closes itself by using a single-fire timer, see also toastify-toast.component.ts:59. Once the duration of autoCloseRemaining (by default = 5000 / 5 sec) elapses. We'd have to stop the current timer, and create a new one with a different duration. (Or come up with a different way to manage toast lifetime, of course)

JoranLive commented 1 year ago

Hello @Lerke ,

I did an update on my branch. I changed a little bit the behaviour to avoid complexity and refactoring.

Rather than updating the current duplicate, I remove all the duplicates before to create the new toast. We could consider another advantage with this behaviour: you see an animation when an identical toast pop again.

In another hand, you could think that it can be disturbing if client pops the same toast every seconds. However:

Thank you for credit!

Lerke commented 1 year ago

Hey @JoranLive , I haven't forgotten about this one :). I've been trying to get the feature to instead reset the already present toast to it's original start duration, and maybe move the toast back up to the top. I think removing + it reappearing looks a bit too busy, personally.

JoranLive commented 1 year ago

Hi @Lerke ,

Sorry for my lately reply. I was in holidays.

I wish you an happy new year.

Any news on this ?

Lerke commented 1 year ago

Hey @JoranLive, happy new year to you too. I've also been a bit preoccupied as of late, so I have not had much further progress on this. Been trying a lot of (admittedly) hacky ways to get a toast to reset its duration when a second one gets created, but with little luck. I'll give it another shot this weekend! :)

Lerke commented 1 year ago

@JoranLive Sorry for the delay (: I finally got around looking at this. What do you think about this? I now reset the timer on a toast if a duplicate is found. It's a bit of a silly workaround, which I hope doesn't destroy performance too much :|

https://user-images.githubusercontent.com/3271167/214035352-6de33f04-0176-4a36-a26e-3d99b27b77f8.mp4

Lerke commented 1 year ago

Also made the progress bar a bit nicer by masking the background, instead of just altering the width. It looked a bit odd with the rainbow bar.

https://user-images.githubusercontent.com/3271167/214043920-928821ef-6e1c-421e-b066-27669d1f6570.mp4

JoranLive commented 1 year ago

Hi @Lerke ,

Thank you for your time! It looks fine!

Regarding the performance, I imagine that it will be a little bit slower, as the toast components will be rendered every seconds. However, I'm not sure it will be perceptible. We should test on mobile devices.

Otherwise, the only solution that I think about is to keep animation in css and to trigger restart by triggering a dom reflow. Or may be, it is easier to do with angular/animations module?

Lerke commented 1 year ago

I think you're right, since I now don't alter the length of the loading bar using an animation anymore, I don't see why this can't be done with an animation instead. I'll try that out :)

JoranLive commented 1 year ago

Hi @Lerke ,

May be I found a more elegant way to manage the progress bar.

I let you to have a look on it.

JoranLive commented 1 year ago

Oh, may be I forgot to integrate your improvement regarding the rainbow progressbar: should be esay for you to reenable it...

Lerke commented 1 year ago

@JoranLive Looks good - sorry for my inactivity on this PR. I have re-applied the change I made for the progress bar cover :). Seems to me we can merge this and push a new version to npm, finally. WHat do you think?

JoranLive commented 1 year ago

Hi @Lerke , That's all good for me. I think we can release.

Lerke commented 1 year ago

Hey @JoranLive , finally merged your PR and published a new version on npm! Should be available with version tag 1.0.5. Thanks again!