Blazored / Toast

A JavaScript free toast library for Blazor and Razor Component applications
https://blazored.github.io/Toast/
MIT License
663 stars 92 forks source link

Pause on hover feature #148 #203

Closed Cvijo closed 1 year ago

Cvijo commented 1 year ago

Added 2 new properties on ToastSettings available atm only on instance bool PauseProgressOnHover int ExtendedTimeout

If ExtendedTimeout is not set pause will continue on original counter, if ExtendedTimeout is set, new PeriodicTimer is created based on ExtendedTimeout value. If you mouse over again before toast is closed timer will reset to original ExtendedTimeout

https://user-images.githubusercontent.com/506853/218874858-46d59957-a341-4b16-90f9-7fc8cfe283bf.mp4

Resolves #148

Cvijo commented 1 year ago

one more thing have doubts about this code in BlazoredToasts.razor.cs

            ToastLevel.Error => new ToastSettings(
                "blazored-toast-error", 
                toastInstanceSettings.IconType ?? IconType, 
                toastInstanceSettings.Icon ?? ErrorIcon ?? "", 
                ShowProgressBar,
                ShowCloseButton,
                toastInstanceSettings.OnClick,
                toastInstanceSettings.Timeout == 0 ? Timeout : toastInstanceSettings.Timeout,
                toastInstanceSettings.PauseProgressOnHover,
                toastInstanceSettings.ExtendedTimeout),

shouldn't additional classes be applied too?

  ToastLevel.Error => new ToastSettings(
                $"blazored-toast-error {toastInstanceSettings.AdditionalClasses}", 

or those are meant only for custom components but in that case user must create his custom component instead of using provided once and add some styles. When i add additional classes i could use CSS to easy do this:

https://user-images.githubusercontent.com/506853/219750817-7ac74bd0-ce49-42cd-b85d-67e6bb8910cf.mp4

Cvijo commented 1 year ago

@chrissainty should I commit new changes or wait for a replay on my comments?

chrissainty commented 1 year ago

@Cvijo My wife and I have just had a baby (2 days ago) so forgive the delay. I'll get to this ASAP

Cvijo commented 1 year ago

@chrissainty omg !!! congratz !!! .. don't worry i thought i messed something with the comments :) ... its wonderful and take your time, nothing is more important than family!

chrissainty commented 1 year ago

one more thing have doubts about this code in BlazoredToasts.razor.cs

            ToastLevel.Error => new ToastSettings(
                "blazored-toast-error", 
                toastInstanceSettings.IconType ?? IconType, 
                toastInstanceSettings.Icon ?? ErrorIcon ?? "", 
                ShowProgressBar,
                ShowCloseButton,
                toastInstanceSettings.OnClick,
                toastInstanceSettings.Timeout == 0 ? Timeout : toastInstanceSettings.Timeout,
                toastInstanceSettings.PauseProgressOnHover,
                toastInstanceSettings.ExtendedTimeout),

shouldn't additional classes be applied too?

  ToastLevel.Error => new ToastSettings(
                $"blazored-toast-error {toastInstanceSettings.AdditionalClasses}", 

or those are meant only for custom components but in that case user must create his custom component instead of using provided once and add some styles. When i add additional classes i could use CSS to easy do this:

That's a great spot. Yes, additional classes should be passed through. I think it might be best to do that in another PR though as it's not directly related to the on pause issue.

Cvijo commented 1 year ago

@chrissainty, when you will get some time I have one more thing to check with you before pushing changes, I see now I didn't put PauseProgressOnHover to the global instance BlazoredToasts and I would like to put it there and its fine but all properties are not nullable on ToastSettings and there is no chance for me to know when creating Toast to check if instance value is set to override global boolean value.

Would it be wrong to change on Toast settings to nullable bool and leave it on global as not nullable

public bool ShowProgressBar { get; set; } -> public bool? ShowProgressBar { get; set; } in ToastSettings.cs

and then when building toast settings toastInstanceSettings.PauseProgressOnHover ?? PauseProgressOnHover

and of course everywhere when it is used i would have to change Settings.PauseProgressOnHover to Settings.PauseProgressOnHover!.Value and change the BuildCustomToastSettings method to

    private ToastSettings BuildCustomToastSettings(Action<ToastSettings>? settings)
    {
        var instanceToastSettings = new ToastSettings();
        settings?.Invoke(instanceToastSettings);
        instanceToastSettings.PauseProgressOnHover ?? PauseProgressOnHover;  <-- must add this line 

        return instanceToastSettings;
    }

Tbh it doesn't seems right to me like that, especially this part Settings.PauseProgressOnHover!.Value that's why I would ask you for advice on how to solve this, is this ok, or maybe another solution or leave it without a global instance?

Cvijo commented 1 year ago

I just saw your DisableTimeout you did exactly as I described with a nullable bool on ToastSettings so I did the same on PauseProgressOnHover

Cvijo commented 1 year ago

@chrissainty, i just saw there are conflicts now in this PR when you push DisableTimeout feature. Do I have to resolve this conflict, I cannot sync my fork because of those commits I did on this PR. I have the option to discard them, is that how should I do it and then re-commit this PR on your new main branch? I am not sure how this should be done :)