Fix DisplaySnackBarAsync Task never completes if another SnackBar appears, and completes before SnackBar action (#1357)

maxkoshevoi · jsuarezruiz · pictos · web-flow · commit fe2aeb2ebbf5 · 2021-11-25T12:52:14.000Z
* Update SnackBar.android.cs * Fix #1340 * Pass exception into result * Fix build * Add test * Use tabs * Extract OnActionClick method * Update SnackBar.tizen.cs * Update ToastOptions.shared.cs Make SetResult and SetException internal Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com> Co-authored-by: Pedro Jesus <pedrojesus.cefet@gmail.com> Co-authored-by: Brandon Minnick <13558917+brminnick@users.noreply.github.com> Co-authored-by: Vladislav Antonyuk <33021114+VladislavAntonyuk@users.noreply.github.com> Co-authored-by: Vladislav Antonyuk <vlad.antonyuk@gmail.com>
diff --git a/samples/XCT.Sample/Pages/TestCases/SnackBarActionExceptionPage.xaml b/samples/XCT.Sample/Pages/TestCases/SnackBarActionExceptionPage.xaml
@@ -0,0 +1,14 @@
+﻿<?xml version="1.0" encoding="utf-8" ?>
+<pages:BasePage
+    x:Class="Xamarin.CommunityToolkit.Sample.Pages.TestCases.SnackBarActionExceptionPage"
+    xmlns="http://xamarin.com/schemas/2014/forms"
+    xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
+    xmlns:pages="clr-namespace:Xamarin.CommunityToolkit.Sample.Pages">
+    
+    <ContentPage.Content>
+        <Grid RowDefinitions="auto, *, *">
+            <Button Text="App shouldn't crash when you press this and execute action." Clicked="ShowSnackBar" />
+        </Grid>
+    </ContentPage.Content>
+    
+</pages:BasePage>
diff --git a/samples/XCT.Sample/Pages/TestCases/SnackBarActionExceptionPage.xaml.cs b/samples/XCT.Sample/Pages/TestCases/SnackBarActionExceptionPage.xaml.cs
@@ -0,0 +1,22 @@
+﻿using System;
+using Xamarin.CommunityToolkit.Extensions;
+
+namespace Xamarin.CommunityToolkit.Sample.Pages.TestCases
+{
+	public partial class SnackBarActionExceptionPage
+	{
+		public SnackBarActionExceptionPage() => InitializeComponent();
+
+		async void ShowSnackBar(object sender, EventArgs e)
+		{
+			try
+			{
+				await this.DisplaySnackBarAsync("Execute action to throw exception", "Throw exception", () => throw new Exception());
+			}
+			catch
+			{
+				await this.DisplayToastAsync("Exception caught");
+			}
+		}
+	}
+}
diff --git a/samples/XCT.Sample/ViewModels/TestCases/TestCasesGalleryViewModel.cs b/samples/XCT.Sample/ViewModels/TestCases/TestCasesGalleryViewModel.cs
@@ -12,7 +12,7 @@ protected override IEnumerable<SectionModel> CreateItems() => new[]
 			new SectionModel(
 				typeof(TouchEffectButtonPage),
 				"TouchEffect + Button",
-				"TouchEffect must automatically invoke button'c command execution."),
+				"TouchEffect must automatically invoke button's command execution."),
 
 			new SectionModel(
 				typeof(TouchEffectCollectionViewPage),
@@ -38,6 +38,11 @@ protected override IEnumerable<SectionModel> CreateItems() => new[]
 				typeof(LinkerCameraViewPage),
 				"Linker for CameraView",
 				"Make sure that Linker is keeping the MediaCaptured and MediaCaptureFailed events if they are used."),
+
+			new SectionModel(
+				typeof(SnackBarActionExceptionPage),
+				"SnackBar Action Exception",
+				"Exception in SnackBar's action doesn't crash the app."),
 		};
 	}
 }
diff --git a/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/Helpers/SnackBarLayout.uwp.wpf.cs b/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/Helpers/SnackBarLayout.uwp.wpf.cs
@@ -73,8 +73,17 @@ public SnackBarLayout(SnackBarOptions options)
 					Command = new Forms.Command(async () =>
 					{
 						OnSnackBarActionExecuted?.Invoke();
-						if (action.Action != null)
-							await action.Action();
+						try
+						{
+							if (action.Action != null)
+								await action.Action();
+
+							options.SetResult(true);
+						}
+						catch (Exception ex)
+						{
+							options.SetException(ex);
+						}
 					}),
 					Padding = new Thickness(action.Padding.Left,
 						action.Padding.Top,
diff --git a/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/Options/ToastOptions.shared.cs b/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/Options/ToastOptions.shared.cs
@@ -51,6 +51,8 @@ public class ToastOptions
 		/// </summary>
 		public TaskCompletionSource<bool> Result { get; }
 
-		public void SetResult(bool result) => Result.SetResult(result);
+		internal bool SetResult(bool result) => Result.TrySetResult(result);
+
+		internal bool SetException(Exception exception) => Result.TrySetException(exception);
 	}
 }
diff --git a/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/SnackBar.android.cs b/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/SnackBar.android.cs
@@ -15,9 +15,9 @@
 
 namespace Xamarin.CommunityToolkit.UI.Views
 {
-	class SnackBar
+	partial class SnackBar
 	{
-		internal async ValueTask Show(VisualElement sender, SnackBarOptions arguments)
+		internal partial async ValueTask Show(VisualElement sender, SnackBarOptions arguments)
 		{
 			var renderer = await GetRendererWithRetries(sender) ?? throw new ArgumentException("Provided VisualElement cannot be parent to SnackBar", nameof(sender));
 			var snackBar = AndroidSnackBar.Make(renderer.View, arguments.MessageOptions.Message, (int)arguments.Duration.TotalMilliseconds);
@@ -87,11 +87,7 @@ internal async ValueTask Show(VisualElement sender, SnackBarOptions arguments)
 
 			foreach (var action in arguments.Actions)
 			{
-				snackBar.SetAction(action.Text, async v =>
-				{
-					if (action.Action != null)
-						await action.Action();
-				});
+				snackBar.SetAction(action.Text, async _ => await OnActionClick(action, arguments).ConfigureAwait(false));
 				if (action.ForegroundColor != Forms.Color.Default)
 				{
 					snackBar.SetActionTextColor(action.ForegroundColor.ToAndroid());
@@ -154,15 +150,10 @@ public override void OnDismissed(Java.Lang.Object transientBottomBar, int e)
 			{
 				base.OnDismissed(transientBottomBar, e);
 
-				switch (e)
-				{
-					case DismissEventTimeout:
-						arguments.SetResult(false);
-						break;
-					case DismissEventAction:
-						arguments.SetResult(true);
-						break;
-				}
+				if (e == DismissEventAction)
+					return;
+
+				arguments.SetResult(false);
 			}
 		}
 	}
diff --git a/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/SnackBar.gtk.cs b/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/SnackBar.gtk.cs
@@ -1,4 +1,5 @@
-﻿using System.Linq;
+﻿using System;
+using System.Linq;
 using System.Threading.Tasks;
 using System.Timers;
 using Gtk;
@@ -10,11 +11,11 @@
 
 namespace Xamarin.CommunityToolkit.UI.Views
 {
-	class SnackBar
+	partial class SnackBar
 	{
 		Timer? snackBarTimer;
 
-		public ValueTask Show(VisualElement visualElement, SnackBarOptions arguments)
+		internal partial ValueTask Show(VisualElement visualElement, SnackBarOptions arguments)
 		{
 			var mainWindow = (Platform.GetRenderer(visualElement).Container.Child as Forms.Platform.GTK.Controls.Page)?.Children[0] as VBox;
 			var snackBarLayout = GetSnackBarLayout(mainWindow, arguments);
@@ -55,14 +56,10 @@ HBox GetSnackBarLayout(Container? container, SnackBarOptions arguments)
 				button.ModifyBg(StateType.Normal, action.BackgroundColor.ToGtkColor());
 				button.ModifyFg(StateType.Normal, action.ForegroundColor.ToGtkColor());
 
-				button.Clicked += async (sender, e) =>
+				button.Clicked += async (_, _) =>
 				{
 					snackBarTimer?.Stop();
-
-					if (action.Action != null)
-						await action.Action();
-
-					arguments.SetResult(true);
+					await OnActionClick(action, arguments).ConfigureAwait(false);
 					container?.Remove(snackBarLayout);
 				};
 
diff --git a/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/SnackBar.ios.macos.cs b/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/SnackBar.ios.macos.cs
@@ -15,9 +15,9 @@
 
 namespace Xamarin.CommunityToolkit.UI.Views
 {
-	class SnackBar
+	partial class SnackBar
 	{
-		internal ValueTask Show(VisualElement sender, SnackBarOptions arguments)
+		internal partial ValueTask Show(VisualElement sender, SnackBarOptions arguments)
 		{
 			var snackBar = NativeSnackBar.MakeSnackBar(arguments.MessageOptions.Message)
 							.SetDuration(arguments.Duration)
@@ -123,11 +123,7 @@ internal ValueTask Show(VisualElement sender, SnackBarOptions arguments)
 				actionButton.SetAction(async () =>
 				{
 					snackBar.Dismiss();
-
-					if (action.Action != null)
-						await action.Action();
-
-					arguments.SetResult(true);
+					await OnActionClick(action, arguments).ConfigureAwait(false);
 				});
 
 				snackBar.Actions.Add(actionButton);
diff --git a/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/SnackBar.netstandard.tvos.watchos.cs b/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/SnackBar.netstandard.tvos.watchos.cs
@@ -5,8 +5,8 @@
 
 namespace Xamarin.CommunityToolkit.UI.Views
 {
-	class SnackBar
+	partial class SnackBar
 	{
-		internal ValueTask Show(VisualElement sender, SnackBarOptions arguments) => throw new PlatformNotSupportedException();
+		internal partial ValueTask Show(VisualElement sender, SnackBarOptions arguments) => throw new PlatformNotSupportedException();
 	}
 }
diff --git a/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/SnackBar.shared.cs b/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/SnackBar.shared.cs
@@ -0,0 +1,27 @@
+﻿using System;
+using System.Threading.Tasks;
+using Xamarin.CommunityToolkit.UI.Views.Options;
+using Xamarin.Forms;
+
+namespace Xamarin.CommunityToolkit.UI.Views
+{
+	partial class SnackBar
+	{
+		internal partial ValueTask Show(VisualElement sender, SnackBarOptions arguments);
+
+		async Task OnActionClick(SnackBarActionOptions action, SnackBarOptions arguments)
+		{
+			try
+			{
+				if (action.Action != null)
+					await action.Action();
+
+				arguments.SetResult(true);
+			}
+			catch (Exception ex)
+			{
+				arguments.SetException(ex);
+			}
+		}
+	}
+}
diff --git a/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/SnackBar.tizen.cs b/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/SnackBar.tizen.cs
@@ -6,9 +6,9 @@
 
 namespace Xamarin.CommunityToolkit.UI.Views
 {
-	class SnackBar
+	partial class SnackBar
 	{
-		internal ValueTask Show(Forms.VisualElement sender, SnackBarOptions arguments)
+		internal partial ValueTask Show(Forms.VisualElement sender, SnackBarOptions arguments)
 		{
 			var snackBarDialog =
 				Forms.Platform.Tizen.Native.Dialog.CreateDialog(Forms.Forms.NativeParent,
@@ -25,19 +25,15 @@ internal ValueTask Show(Forms.VisualElement sender, SnackBarOptions arguments)
 			{
 				var ok = new EButton(snackBarDialog) { Text = action.Text };
 				snackBarDialog.NeutralButton = ok;
-				ok.Clicked += async (s, evt) =>
+				ok.Clicked += async (_, _) =>
 				{
 					snackBarDialog.Dismiss();
-
-					if (action.Action != null)
-						await action.Action();
-
-					arguments.SetResult(true);
+					await OnActionClick(action, arguments).ConfigureAwait(false);
 				};
 			}
 
-			snackBarDialog.TimedOut += (s, evt) => DismissSnackBar();
-			snackBarDialog.BackButtonPressed += (s, evt) => DismissSnackBar();
+			snackBarDialog.TimedOut += (_, _) => DismissSnackBar();
+			snackBarDialog.BackButtonPressed += (_, _) => DismissSnackBar();
 			snackBarDialog.Show();
 
 			return default;
diff --git a/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/SnackBar.uwp.cs b/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/SnackBar.uwp.cs
@@ -9,7 +9,7 @@
 
 namespace Xamarin.CommunityToolkit.UI.Views
 {
-	class SnackBar
+	partial class SnackBar
 	{
 		DispatcherTimer? snackBarTimer;
 
@@ -35,7 +35,7 @@ class SnackBar
 			return null;
 		}
 
-		internal ValueTask Show(Forms.VisualElement visualElement, SnackBarOptions arguments)
+		internal partial ValueTask Show(Forms.VisualElement visualElement, SnackBarOptions arguments)
 		{
 			var snackBarLayout = new SnackBarLayout(arguments);
 			var pageControl = Platform.GetRenderer(visualElement).ContainerElement.Parent;
@@ -55,7 +55,6 @@ internal ValueTask Show(Forms.VisualElement visualElement, SnackBarOptions argum
 				grid.Children.Remove(snackBarLayout);
 				grid.RowDefinitions.Remove(snackBarRow);
 				snackBarTimer.Stop();
-				arguments.SetResult(true);
 			};
 			snackBarTimer.Start();
 			grid.RowDefinitions.Add(snackBarRow);
diff --git a/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/SnackBar.wpf.cs b/src/CommunityToolkit/Xamarin.CommunityToolkit/Views/Snackbar/SnackBar.wpf.cs
@@ -11,11 +11,11 @@
 
 namespace Xamarin.CommunityToolkit.UI.Views
 {
-	class SnackBar
+	partial class SnackBar
 	{
 		Timer? snackBarTimer;
 
-		internal ValueTask Show(Forms.VisualElement visualElement, SnackBarOptions arguments)
+		internal partial ValueTask Show(Forms.VisualElement visualElement, SnackBarOptions arguments)
 		{
 			var formsAppBar = System.Windows.Application.Current.MainWindow.FindChild<FormsAppBar>("PART_BottomAppBar");
 			var currentContent = formsAppBar.Content;
@@ -31,7 +31,6 @@ internal ValueTask Show(Forms.VisualElement visualElement, SnackBarOptions argum
 			{
 				formsAppBar.Content = currentContent;
 				snackBarTimer.Stop();
-				arguments.SetResult(true);
 			};
 			snackBarTimer.Start();
 			var border = new Border

Original file line number	Diff line number	Diff line change
`@@ -51,6 +51,8 @@ public class ToastOptions`
`51`	`51`	`/// </summary>`
`52`	`52`	`public TaskCompletionSource<bool> Result { get; }`
`53`	`53`
`54`		`- public void SetResult(bool result) => Result.SetResult(result);`
	`54`	`+ internal bool SetResult(bool result) => Result.TrySetResult(result);`
	`55`	`+`
	`56`	`+ internal bool SetException(Exception exception) => Result.TrySetException(exception);`
`55`	`57`	`}`
`56`	`58`	`}`
Original file line number	Diff line number	Diff line change
`@@ -15,9 +15,9 @@`
`15`	`15`
`16`	`16`	`namespace Xamarin.CommunityToolkit.UI.Views`
`17`	`17`	`{`
`18`		`- class SnackBar`
	`18`	`+ partial class SnackBar`
`19`	`19`	`{`
`20`		`- internal async ValueTask Show(VisualElement sender, SnackBarOptions arguments)`
	`20`	`+ internal partial async ValueTask Show(VisualElement sender, SnackBarOptions arguments)`
`21`	`21`	`{`
`22`	`22`	`var renderer = await GetRendererWithRetries(sender) ?? throw new ArgumentException("Provided VisualElement cannot be parent to SnackBar", nameof(sender));`
`23`	`23`	`var snackBar = AndroidSnackBar.Make(renderer.View, arguments.MessageOptions.Message, (int)arguments.Duration.TotalMilliseconds);`
`@@ -87,11 +87,7 @@ internal async ValueTask Show(VisualElement sender, SnackBarOptions arguments)`
`87`	`87`
`88`	`88`	`foreach (var action in arguments.Actions)`
`89`	`89`	`{`
`90`		`- snackBar.SetAction(action.Text, async v =>`
`91`		`- {`
`92`		`- if (action.Action != null)`
`93`		`- await action.Action();`
`94`		`- });`
	`90`	`+ snackBar.SetAction(action.Text, async _ => await OnActionClick(action, arguments).ConfigureAwait(false));`
`95`	`91`	`if (action.ForegroundColor != Forms.Color.Default)`
`96`	`92`	`{`
`97`	`93`	`snackBar.SetActionTextColor(action.ForegroundColor.ToAndroid());`
`@@ -154,15 +150,10 @@ public override void OnDismissed(Java.Lang.Object transientBottomBar, int e)`
`154`	`150`	`{`
`155`	`151`	`base.OnDismissed(transientBottomBar, e);`
`156`	`152`
`157`		`- switch (e)`
`158`		`- {`
`159`		`- case DismissEventTimeout:`
`160`		`- arguments.SetResult(false);`
`161`		`- break;`
`162`		`- case DismissEventAction:`
`163`		`- arguments.SetResult(true);`
`164`		`- break;`
`165`		`- }`
	`153`	`+ if (e == DismissEventAction)`
	`154`	`+ return;`
	`155`	`+`
	`156`	`+ arguments.SetResult(false);`
`166`	`157`	`}`
`167`	`158`	`}`
`168`	`159`	`}`
Original file line number	Diff line number	Diff line change
`@@ -5,8 +5,8 @@`
`5`	`5`
`6`	`6`	`namespace Xamarin.CommunityToolkit.UI.Views`
`7`	`7`	`{`
`8`		`- class SnackBar`
	`8`	`+ partial class SnackBar`
`9`	`9`	`{`
`10`		`- internal ValueTask Show(VisualElement sender, SnackBarOptions arguments) => throw new PlatformNotSupportedException();`
	`10`	`+ internal partial ValueTask Show(VisualElement sender, SnackBarOptions arguments) => throw new PlatformNotSupportedException();`
`11`	`11`	`}`
`12`	`12`	`}`