RefreshView: add new internal property IsRefreshAllowed to fix #14350

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue14350.xaml

@@ -0,0 +1,36 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<ContentPage xmlns="http://xamarin.com/schemas/2014/forms"
+             xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
+             xmlns:d="http://xamarin.com/schemas/2014/forms/design"
+             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
+             mc:Ignorable="d"
+			 Title="Test 14350"
+             x:Class="Xamarin.Forms.Controls.Issues.Issue14350">
+    <StackLayout>
+        <Label Text="Test for issue #14350" />
+        <Label Text="Refresh enabled" IsVisible="{Binding IsRefreshAllowed}" TextColor="Green"/>
+        <Button Text="Refresh now" Command="{Binding Refresh}" />
+        <StackLayout>
+            <StackLayout Orientation="Horizontal">
+                <Label Text="Enable refresh:" HorizontalOptions="FillAndExpand"/>
+                <Switch IsToggled="{Binding IsRefreshAllowed}" />
+            </StackLayout>
+        </StackLayout>
+        <RefreshView IsRefreshing="{Binding IsRefreshing}" Command="{Binding Refresh}" IsEnabled="{Binding IsRefreshAllowed}">
+            <ScrollView>
+                <StackLayout>
+                    <Label Text="Pull-to-refresh here" />
+                    <Label Text="Refreshing ..." IsVisible="{Binding IsRefreshing}" TextColor="Orange"/>
+                    <BoxView HeightRequest="30" />
+                    <StackLayout>
+                        <Label Text="Test steps:" />
+                        <Label Text="1. Swipe to refresh" />
+                        <Label Text="2. Refresh/busy indicator appears at the top" />
+                        <Label Text="3. Refresh/busy indicator should remain visible until content changes (refresh is finished). Due to issue #14350 the indicator vanishes too early if Command.CanExecute toggles (in this test after 1 second)." />
+                        <Label Text="4. Repeat steps 1 to 3 multiple times" />
+                    </StackLayout>
+                </StackLayout>
+            </ScrollView>
+        </RefreshView>
+    </StackLayout>
+</ContentPage>

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue14350.xaml.cs

@@ -0,0 +1,26 @@
+using System.ComponentModel;
+
+using Xamarin.Forms;
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+
+namespace Xamarin.Forms.Controls.Issues
+{
+	// Learn more about making custom code visible in the Xamarin.Forms previewer
+	// by visiting https://aka.ms/xamarinforms-previewer
+	[DesignTimeVisible(false)]
+	[Preserve(AllMembers = true)]
+	[Issue(IssueTracker.Github, 14350,
+		"[Bug] RefreshView to completes instantly when using AsyncCommand or Command with CanExecute functionality",
+		PlatformAffected.Android)]
+	public partial class Issue14350 : ContentPage
+	{
+		public Issue14350()
+		{
+#if APP
+			InitializeComponent();
+			BindingContext = new ViewModelIssue8384();
+#endif
+		}
+	}
+}

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue8384.xaml.cs

@@ -55,6 +55,7 @@ public bool Allow
 
 		private List<string> _items;
 		private bool _isRefreshing;
+		private bool _isRefreshAllowed = true;
 		private MyCommand _refresh;
 
 		static readonly List<string> FIRST_LIST = new List<string>() {
@@ -78,6 +79,19 @@ public bool IsRefreshing
 			}
 		}
 
+		public bool IsRefreshAllowed
+		{
+			get
+			{
+				return _isRefreshAllowed;
+			}
+			set
+			{
+				_isRefreshAllowed = value;
+				OnPropertyChanged("IsRefreshAllowed");
+			}
+		}
+
 		public ViewModelIssue8384()
 		{
 			Items = FIRST_LIST;

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems

@@ -1795,6 +1795,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Issue11795.xaml.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue14528.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue14286.xaml.cs" />
+	<Compile Include="$(MSBuildThisFileDirectory)Issue14350.xaml.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue14613.xaml.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue13930.xaml.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue11211.xaml.cs" />
@@ -2282,6 +2283,9 @@
     </EmbeddedResource>
     <EmbeddedResource Include="$(MSBuildThisFileDirectory)Issue14286.xaml">
       <Generator>MSBuild:UpdateDesignTimeXaml</Generator>
+    </EmbeddedResource>
+	<EmbeddedResource Include="$(MSBuildThisFileDirectory)Issue14350.xaml">
+      <Generator>MSBuild:UpdateDesignTimeXaml</Generator>
     </EmbeddedResource>
     <EmbeddedResource Include="$(MSBuildThisFileDirectory)Issue14613.xaml">
       <Generator>MSBuild:UpdateDesignTimeXaml</Generator>

Xamarin.Forms.Core.UnitTests/RefreshViewTests.cs

@@ -35,22 +35,25 @@ public void StartsEnabled()
 		{
 			RefreshView refreshView = new RefreshView();
 			Assert.IsTrue(refreshView.IsEnabled);
+			Assert.IsTrue(refreshView.IsRefreshAllowed);
 		}
 
 		[Test]
-		public void CanExecuteDisablesRefreshView()
+		public void CanExecuteDisablesRefresh()
 		{
 			RefreshView refreshView = new RefreshView();
 			refreshView.Command = new Command(() => { }, () => false);
-			Assert.IsFalse(refreshView.IsEnabled);
+			Assert.IsTrue(refreshView.IsEnabled);
+			Assert.IsFalse(refreshView.IsRefreshAllowed);
 		}
 
 		[Test]
-		public void CanExecuteEnablesRefreshView()
+		public void CanExecuteEnablesRefresh()
 		{
 			RefreshView refreshView = new RefreshView();
 			refreshView.Command = new Command(() => { }, () => true);
 			Assert.IsTrue(refreshView.IsEnabled);
+			Assert.IsTrue(refreshView.IsRefreshAllowed);
 		}
 
 		[Test]
@@ -64,16 +67,18 @@ public void CanExecuteChangesEnabled()
 
 			canExecute = false;
 			command.ChangeCanExecute();
-			Assert.IsFalse(refreshView.IsEnabled);
+			Assert.IsTrue(refreshView.IsEnabled);
+			Assert.IsFalse(refreshView.IsRefreshAllowed);
 
 
 			canExecute = true;
 			command.ChangeCanExecute();
 			Assert.IsTrue(refreshView.IsEnabled);
+			Assert.IsTrue(refreshView.IsRefreshAllowed);
 		}
 
 		[Test]
-		public void CommandPropertyChangesEnabled()
+		public void CommandPropertyChangesRefreshEnabled()
 		{
 			RefreshView refreshView = new RefreshView();
 
@@ -82,26 +87,40 @@ public void CommandPropertyChangesEnabled()
 			refreshView.CommandParameter = true;
 			refreshView.Command = command;
 
-			Assert.IsTrue(refreshView.IsEnabled);
+			Assert.IsTrue(refreshView.IsRefreshAllowed);
 			refreshView.CommandParameter = false;
-			Assert.IsFalse(refreshView.IsEnabled);
+			Assert.IsFalse(refreshView.IsRefreshAllowed);
 			refreshView.CommandParameter = true;
+			Assert.IsTrue(refreshView.IsRefreshAllowed);
+		}
+
+		[Test]
+		public void CanExecuteDoesNotDisableRefreshView()
+		{
+			// NOTE: RefreshView.IsEnabled should NOT be set to false based on CanExec to avoid problems like:
+			// https://github.com/xamarin/Xamarin.Forms/issues/14350
+			RefreshView refreshView = new RefreshView();
+			Assert.IsTrue(refreshView.IsEnabled);
+			refreshView.Command = new Command(() => { }, () => false);
 			Assert.IsTrue(refreshView.IsEnabled);
 		}
 
 		[Test]
-		public void RemovedCommandEnablesRefreshView()
+		public void RemovedCommandEnablesRefresh()
 		{
 			RefreshView refreshView = new RefreshView();
 
 			bool canExecute = true;
 			var command = new Command(() => { }, () => false);
 			refreshView.Command = command;
-			Assert.IsFalse(refreshView.IsEnabled);
+			Assert.IsTrue(refreshView.IsEnabled);
+			Assert.IsFalse(refreshView.IsRefreshAllowed);
 			refreshView.Command = null;
 			Assert.IsTrue(refreshView.IsEnabled);
+			Assert.IsTrue(refreshView.IsRefreshAllowed);
 			refreshView.Command = command;
-			Assert.IsFalse(refreshView.IsEnabled);
+			Assert.IsTrue(refreshView.IsEnabled);
+			Assert.IsFalse(refreshView.IsRefreshAllowed);
 		}
 
 		[Test]

Xamarin.Forms.Core/RefreshView.cs

@@ -1,4 +1,5 @@
 using System;
+using System.ComponentModel;
 using System.Runtime.CompilerServices;
 using System.Windows.Input;
 using Xamarin.Forms.Platform;
@@ -49,6 +50,9 @@ static object OnIsRefreshingPropertyCoerced(BindableObject bindable, object valu
 			if (!view.IsEnabled)
 				return false;
 
+			if (!view.IsRefreshAllowed)
+				return false;
+
 			if (view.Command == null)
 				return value;
 
@@ -98,12 +102,22 @@ public object CommandParameter
 			set { SetValue(CommandParameterProperty, value); }
 		}
 
+		public static readonly BindableProperty IsRefreshAllowedProperty =
+			BindableProperty.Create(nameof(IsRefreshAllowed), typeof(bool), typeof(RefreshView), true, BindingMode.TwoWay);
+
+		[EditorBrowsable(EditorBrowsableState.Never)]
+		public bool IsRefreshAllowed
+		{
+			get { return (bool)GetValue(IsRefreshAllowedProperty); }
+			set { SetValue(IsRefreshAllowedProperty, value); }
+		}
+
 		void RefreshCommandCanExecuteChanged(object sender, EventArgs eventArgs)
 		{
 			if (Command != null)
-				SetValueCore(IsEnabledProperty, Command.CanExecute(CommandParameter));
+				SetValueCore(IsRefreshAllowedProperty, Command.CanExecute(CommandParameter));
 			else
-				SetValueCore(IsEnabledProperty, true);
+				SetValueCore(IsRefreshAllowedProperty, true);
 		}
 
 		public static readonly BindableProperty RefreshColorProperty =

Xamarin.Forms.Platform.Android/Renderers/RefreshViewRenderer.cs

@@ -57,6 +57,9 @@ public override bool Refreshing
 					RefreshView.SetValueFromRenderer(RefreshView.IsRefreshingProperty, _refreshing);
 
 				base.Refreshing = _refreshing;
+
+				// Allow to disable SwipeToRefresh layout AFTER refresh is done
+				UpdateIsEnabled();
 			}
 		}
 
@@ -140,8 +143,16 @@ void UpdateColors()
 		}
 
 		void UpdateIsRefreshing() => Refreshing = RefreshView.IsRefreshing;
-
-		void UpdateIsEnabled() => SwipeRefreshLayout.Enabled = RefreshView.IsEnabled;
+
+		void UpdateIsEnabled()
+		{
+			// NOTE: only disable while NOT refreshing, otherwise Command bindings CanExecute behavior will effectively
+			// cancel refresh animation. If not possible right now we will be called by UpdateIsRefreshing().
+			// For details see https://github.com/xamarin/Xamarin.Forms/issues/14350
+			var isEnabled = RefreshView.IsEnabled && RefreshView.IsRefreshAllowed;
+			if (isEnabled || !SwipeRefreshLayout.Refreshing)
+				SwipeRefreshLayout.Enabled = isEnabled;
+		}
 
 		bool CanScrollUp(AView view)
 		{
@@ -207,6 +218,8 @@ void HandlePropertyChanged(object sender, PropertyChangedEventArgs e)
 				UpdateContent();
 			else if (e.PropertyName == VisualElement.IsEnabledProperty.PropertyName)
 				UpdateIsEnabled();
+			else if (e.PropertyName == RefreshView.IsRefreshAllowedProperty.PropertyName)
+				UpdateIsEnabled();
 			else if (e.PropertyName == RefreshView.IsRefreshingProperty.PropertyName)
 				UpdateIsRefreshing();
 			else if (e.IsOneOf(RefreshView.RefreshColorProperty, VisualElement.BackgroundColorProperty, VisualElement.BackgroundProperty))

Xamarin.Forms.Platform.UAP/RefreshViewRenderer.cs

@@ -105,6 +105,8 @@ protected override void OnElementPropertyChanged(object sender, PropertyChangedE
 				UpdateContent();
 			else if (e.PropertyName == VisualElement.IsEnabledProperty.PropertyName)
 				UpdateIsEnabled();
+			else if (e.PropertyName == RefreshView.IsRefreshAllowedProperty.PropertyName)
+				UpdateIsEnabled();
 			else if (e.PropertyName == RefreshView.IsRefreshingProperty.PropertyName)
 				UpdateIsRefreshing();
 			else if (e.PropertyName == RefreshView.RefreshColorProperty.PropertyName)
@@ -135,7 +137,7 @@ void UpdateContent()
 
 		void UpdateIsEnabled()
 		{
-			Control.IsEnabled = Element.IsEnabled;
+			Control.IsEnabled = Element.IsEnabled && Element.IsRefreshAllowed;
 		}
 
 		void UpdateIsRefreshing()

Xamarin.Forms.Platform.iOS/Renderers/RefreshViewRenderer.cs

@@ -38,6 +38,9 @@ public bool IsRefreshing
 						TryOffsetRefresh(this, IsRefreshing);
 					}
 				}
+
+				// Allow to disable UIRefreshControl layout AFTER refresh is done
+				UpdateIsEnabled();
 			}
 		}
 
@@ -66,7 +69,7 @@ protected override void OnElementChanged(ElementChangedEventArgs<RefreshView> e)
 
 					_refreshControl = new UIRefreshControl();
 					_refreshControl.ValueChanged += OnRefresh;
-					_refreshControlParent = this;
+					_refreshControlParent = null;
 				}
 			}
 
@@ -81,6 +84,8 @@ protected override void OnElementPropertyChanged(object sender, PropertyChangedE
 
 			if (e.PropertyName == VisualElement.IsEnabledProperty.PropertyName)
 				UpdateIsEnabled();
+			else if (e.PropertyName == RefreshView.IsRefreshAllowedProperty.PropertyName)
+				UpdateIsEnabled();
 			else if (e.PropertyName == RefreshView.IsRefreshingProperty.PropertyName)
 				UpdateIsRefreshing();
 			else if (e.IsOneOf(RefreshView.RefreshColorProperty, VisualElement.BackgroundColorProperty))
@@ -155,7 +160,12 @@ bool TryOffsetRefresh(UIView view, bool refreshing)
 
 		bool TryRemoveRefresh(UIView view, int index = 0)
 		{
-			_refreshControlParent = view;
+			// Ensure refresh control is in idle state before removing 
+			// Should avoid ios warnings of the form "Attempting to change the refresh control while
+			// it is not idle is strongly discouraged and probably won't work properly."
+			PerformWithoutAnimation(() => {
+				_refreshControl.EndRefreshing();
+			});
 
 			if (_refreshControl.Superview != null)
 				_refreshControl.RemoveFromSuperview();
@@ -183,8 +193,6 @@ bool TryRemoveRefresh(UIView view, int index = 0)
 
 		bool TryInsertRefresh(UIView view, int index = 0)
 		{
-			_refreshControlParent = view;
-
 			if (view is UIScrollView scrollView)
 			{
 				bool addedRefreshControl = false;
@@ -252,18 +260,30 @@ void UpdateIsRefreshing()
 
 		void UpdateIsEnabled()
 		{
-			bool isRefreshViewEnabled = Element.IsEnabled;
-			_refreshControl.Enabled = isRefreshViewEnabled;
+			// Do not disable while refresh control is active
+			if (IsRefreshing)
+				return;
 
 			UserInteractionEnabled = true;
 
-			if (IsRefreshing)
-				return;
+			bool isRefreshViewEnabled = Element.IsEnabled && Element.IsRefreshAllowed;
+
+			_refreshControl.Enabled = isRefreshViewEnabled;
 
 			if (isRefreshViewEnabled)
-				TryInsertRefresh(_refreshControlParent);
+			{
+				if (_refreshControlParent == null && TryInsertRefresh(this))
+				{
+					_refreshControlParent = this;
+				}
+			}
 			else
-				TryRemoveRefresh(_refreshControlParent);
+			{
+				if (_refreshControlParent != null && TryRemoveRefresh(_refreshControlParent))
+				{
+					_refreshControlParent = null;
+				}
+			}
 
 			UserInteractionEnabled = true;
 		}