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

ShowToast return instance #210

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

Cvijo
Copy link
Contributor

@Cvijo Cvijo commented Feb 25, 2023

@chrissainty this is the first draft of this feature.
I have fixed some bugs with custom toasts (they did now follow MaxToastCount rule and queue logic)
In code I have put my comments for some changes and will remove them when we agree on them, they all start with //todo remove comment after review
`

I am not sure about toastInstance.Dispose(), can you check if that is fine with you, if yes i will have to implement those on ClearAll methods too.

Here is screen shoot.

Novi.videozapis.mp4

@Cvijo
Copy link
Contributor Author

Cvijo commented Mar 4, 2023

@chrissainty I am not sure how to adjust unit testing (I never wrote any) on these new events that now return an instance. Do you know anyone who could help with that?
I have tried something but i don't think it is the correct way, created a mock Instance and returned it.



    internal class MockToastInstance : IToastInstance
    {
        public Guid Id => Guid.NewGuid();

        
        public void Close() {}
    }


    [Fact]
    public void OnShowInvoked_When_ShowWarningCalled()
    {
        // arrange
        var instance = new MockToastInstance(); 
        var onShowCalled = false;
        
        _sut.OnShow += (_, _, _) => { onShowCalled = true; return instance;  };
        // act
        _sut.ShowWarning("message");

        // assert
        Assert.True(onShowCalled);
    }

@chrissainty
Copy link
Member

I wouldn't mock anything. I'd render a real toast instance verify its there then call close on the instance and verify it's now gone. That way the test will be testing real stuff and no mocks

@Cvijo
Copy link
Contributor Author

Cvijo commented Mar 4, 2023

As i said i never do any kind of unit testing, now it fails because OnShow and every other event returns instance instead of void and i don't know how to change them

_sut.OnShow += (_, _, _) => onShowCalled = true;

OnShow has to return instance and to my zero knowledge on unit testing i could only make it work with code i posted.
If you can share one sample of this test how should i change it i could do the rest

    [Fact]
    public void OnShowInvoked_When_ShowSuccessCalled()

And btw. I wanted you to check if this is even an option for this feature to return ToastInstance

@chrissainty
Copy link
Member

No worries. I might be able to get a bit of time this evening to get on the laptop. I'll do my best to write up an example.

@chrissainty
Copy link
Member

@Cvijo Sorry I've not been able to help with this for so long. I think the mock instance will do for now. I can rework the tests once we merge things if there are any major issues.

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

Successfully merging this pull request may close these issues.

2 participants