Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[Bug] ListView RefreshCommand ActivityIndicator does disappear on Android if CanExecute is changed to false #8384

Closed
kwintAl opened this issue Nov 5, 2019 · 1 comment · Fixed by #14816
Labels
e/5 🕔 5 help wanted We welcome community contributions to any issue, but these might be a good place to start! inactive Issue is older than 6 months and needs to be retested p/Android t/bug 🐛 up-for-grabs We welcome community contributions to any issue, but these might be a good place to start!

Comments

@kwintAl
Copy link

kwintAl commented Nov 5, 2019

Description

A Command is used in a ToolbarItem.Command and in a ListView.RefreshCommand. The Command starts a Bluetooth discovery and is disabled (CanExecute returns false) while the discovery is running.

In case the command is executed via ToolbarItem the ActivityIndicator appears on screen.
In case the command is executed via ListView.RefreshCommand(PullToRefresh) the ActivityIndicator disappears when CanExecute is changed to false.

It looks like ListView.RefreshAllowed is set to false

  void OnCommandCanExecuteChanged(object sender, EventArgs eventArgs)
  {
  	RefreshAllowed = RefreshCommand.CanExecute(null);
  }

and this causes SwipeRefreshLayout.Enabled = false

  void UpdateIsSwipeToRefreshEnabled()
  {
  	if (_refresh != null)
  		_refresh.Enabled = Element.IsPullToRefreshEnabled && (Element as IListViewController).RefreshAllowed;
  }

From Xamarin documentation:

When defining a RefreshCommand, the CanExecute method of the command can be specified to enable or disable the command.

Is that the desired behavior? If yes, why do I need additional properties IsPullToRefreshEnabled and IsRefreshing?

I think CanExecute should be checked to avoid executing a Command twice but the ActivityIndicator should not disappear. For this logic we have the IsRefreshing property.

My current workaround:

using System.ComponentModel;
using Mono.Device.Discovery.Example.Android;
using Xamarin.Forms;
using Xamarin.Forms.Platform.Android;

[assembly: ExportRenderer(typeof(ListView), typeof(ListRenderer))]
namespace Mono.Device.Discovery.Example.Android
{
	public class ListRenderer : ListViewRenderer
	{
		protected override void OnElementPropertyChanged(object sender, PropertyChangedEventArgs e)
		{
			
			if (e.PropertyName == nameof(ListView.RefreshAllowed))
			{
				return;
			}
			base.OnElementPropertyChanged(sender, e);
		}
	}
}

Steps to Reproduce

  1. Create a Command
  2. PullToRefresh
  3. Command should return CanExecute false
  4. ActivityIndicator disappears

Expected Behavior

ActivityIndicator is visible

Actual Behavior

ActivityIndicator is not visible

Basic Information

  • Version with issue:
  • Last known good version:
  • IDE: Rider 2019.2.3
  • Platform Target Frameworks:
    • iOS:
    • Android: 9.0 API 28
    • UWP:
  • Android Support Library Version:
  • Nuget Packages: Xamarin.Forms 4.3.0.947036
  • Affected Devices:

Screenshots

Reproduction Link

@kwintAl kwintAl added s/unverified New report that has yet to be verified t/bug 🐛 labels Nov 5, 2019
@pauldipietro pauldipietro added this to New in Triage Nov 5, 2019
@hartez
Copy link
Contributor

hartez commented Nov 6, 2019

Yep. If CanExecute changes while the refresh indicator is up, the indicator disappears. Seems like it should wait until the Command is finished.

_8384 Repro.zip

@hartez hartez added p/Android and removed s/unverified New report that has yet to be verified labels Nov 6, 2019
@hartez hartez added this to To do in Android Ready For Work via automation Nov 6, 2019
@hartez hartez removed this from New in Triage Nov 6, 2019
@hartez hartez added the e/5 🕔 5 label Nov 6, 2019
@samhouts samhouts added inactive Issue is older than 6 months and needs to be retested help wanted We welcome community contributions to any issue, but these might be a good place to start! up-for-grabs We welcome community contributions to any issue, but these might be a good place to start! and removed inactive Issue is older than 6 months and needs to be retested help wanted We welcome community contributions to any issue, but these might be a good place to start! labels Jul 17, 2020
@samhouts samhouts added inactive Issue is older than 6 months and needs to be retested help wanted We welcome community contributions to any issue, but these might be a good place to start! and removed inactive Issue is older than 6 months and needs to be retested labels Jul 31, 2020
@samhouts samhouts added this to the 5.0.0 milestone Aug 13, 2020
@samhouts samhouts added this to To do in vNext+1 (5.0.0) Aug 14, 2020
@samhouts samhouts added the inactive Issue is older than 6 months and needs to be retested label Sep 18, 2020
@samhouts samhouts removed this from the 5.0.0 milestone Nov 2, 2020
cpraehaus added a commit to cpraehaus/Xamarin.Forms that referenced this issue Nov 1, 2021
cpraehaus added a commit to cpraehaus/Xamarin.Forms that referenced this issue Nov 1, 2021
indicator on Android, fixes xamarin#8384

RefreshAllowed is bound to ListView.RefreshCommand.CanExecute().
Often an implementation of RefreshCommand might update its CanExecute status
after execution starts. This caused ListViewRenderer to immediately
disable the SwipeRefreshLayout, thereby cancelling the refresh/activity
indicator on top of the list view.

The solution is to NOT disable it while refreshing, but waiting for the
next chance when current refresh activity/command is done.
Android Ready For Work automation moved this from To do to Done Nov 4, 2021
jfversluis pushed a commit that referenced this issue Nov 4, 2021
…or on Android, fixes #8384 (#14816)

* Add test for issue #8384

* ListView: avoid that disabling RefreshAllowed cancels refresh
indicator on Android, fixes #8384

RefreshAllowed is bound to ListView.RefreshCommand.CanExecute().
Often an implementation of RefreshCommand might update its CanExecute status
after execution starts. This caused ListViewRenderer to immediately
disable the SwipeRefreshLayout, thereby cancelling the refresh/activity
indicator on top of the list view.

The solution is to NOT disable it while refreshing, but waiting for the
next chance when current refresh activity/command is done.

* Only enable Issue8384 constructor for XF controls app build (not for UI unit tests)

Otherwise we get 'InitializeComponent' not found error when compiling UI unit test projects.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
e/5 🕔 5 help wanted We welcome community contributions to any issue, but these might be a good place to start! inactive Issue is older than 6 months and needs to be retested p/Android t/bug 🐛 up-for-grabs We welcome community contributions to any issue, but these might be a good place to start!
3 participants