fix(InAppNotifications): use timer to detect notification dismiss event after timeout #1434
Conversation
|
Wondering if it would make sense to do this with a timer instead that you simply cancel when it's dismissed before the timer runs out? |
|
@nmetulev yeah, that's what I had suggested too. |
|
@michael-hawker Sorry, I forgot your comment on this. It is a better solution indeed. Thanks. |
|
I like this better too. Could you also update the documentation with the change of the method? |
|
@nmetulev Sorry. Good catch. |
michael-hawker
left a comment
There was a problem hiding this comment.
This is great! Just one small code ordering/timing issue + doc updates like @nmetulev said.
| { | ||
| await Task.Delay(duration); | ||
| Dismiss(); | ||
| _timer.Stop(); |
There was a problem hiding this comment.
You should move all this timer stop code to be first to ensure the timer doesn't fire between when you call the make visible state and stopping the timer.
michael-hawker
left a comment
There was a problem hiding this comment.
You left the 'await's at the start everywhere...
docs/controls/InAppNotification.md
Outdated
| if (isTemplatePresent && inAppNotificationWithButtonsTemplate is DataTemplate) | ||
| { | ||
| await ExampleInAppNotification.ShowAsync(inAppNotificationWithButtonsTemplate as DataTemplate); | ||
| await ExampleInAppNotification.Show(inAppNotificationWithButtonsTemplate as DataTemplate); |
There was a problem hiding this comment.
Don't forget to remove the await here.
docs/controls/InAppNotification.md
Outdated
|
|
||
| ```c# | ||
| await ExampleInAppNotification.ShowAsync("Some text.", 2000); // the notification will appear for 2 seconds | ||
| await ExampleInAppNotification.Show("Some text.", 2000); // the notification will appear for 2 seconds |
michael-hawker
left a comment
There was a problem hiding this comment.
Thanks for the quick turnarounds on this!
fix #1415
@bkaankose I tried to use CancellationToken but it added so much code and it is seemed a little messy to me. The unique id do the trick but yes the Task.Delay is not cancelled immediately...