Skip to content
New issue

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

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

Already on GitHub? # to your account

Pause on hover feature #148 #203

Merged
merged 20 commits into from
Feb 23, 2023
Merged

Conversation

Cvijo
Copy link
Contributor

@Cvijo Cvijo commented Feb 14, 2023

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

Blazored.Toast.Server.-.Google.Chrome.2023-02-14.22-01-26.mp4

Resolves #148

@Cvijo
Copy link
Contributor Author

Cvijo commented Feb 17, 2023

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:

Blazored.Toast.Server.-.Google.Chrome.2023-02-17.19-26-00.mp4

@Cvijo
Copy link
Contributor Author

Cvijo commented Feb 20, 2023

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

@chrissainty
Copy link
Member

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

@Cvijo
Copy link
Contributor Author

Cvijo commented Feb 20, 2023

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

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

Cvijo commented Feb 21, 2023

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

Cvijo commented Feb 21, 2023

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

Cvijo commented Feb 22, 2023

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

Matija Gluhak added 5 commits February 22, 2023 18:21
- added better description/message to sample project 2 new buttons
- added description to DisableTimout property
- fixed BuildCustomToastSettings method with missing properties
@chrissainty chrissainty merged commit eed68ee into Blazored:main Feb 23, 2023
Cvijo added a commit to Cvijo/Toast that referenced this pull request Feb 23, 2023
@Cvijo Cvijo deleted the pause_on_hover_feature branch February 23, 2023 18:13
@chrissainty chrissainty added the Feature New feature that will be added to the project label Feb 23, 2023
chrissainty added a commit that referenced this pull request Feb 23, 2023
* added ability to use toast position on each toast instance instead of single global position

* - passing additinal css classes to toast (#207)

* Pause on hover feature #148 (#203)

---------

Co-authored-by: Matija Gluhak <matija.gluhak@vipdata.hr>
Co-authored-by: Chris Sainty <chrissainty@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Feature New feature that will be added to the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] Ability to pause timeout when actively hovering/mousing over the toast
2 participants