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

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

Merged
merged 3 commits into from Nov 4, 2021

Conversation

cpraehaus
Copy link
Contributor

Description of Change

RefreshAllowed is bound to ListView.RefreshCommand.CanExecute(). Often an implementation of RefreshCommand might update its CanExecute status after execution starts, e.g. to disable button when action 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.

Issues Resolved

API Changes

None

Platforms Affected

  • Android

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

Launch Xamarin.Forms.ControlGallery.Android and navigate to issue 8384. Follow the steps there to test the issue.

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

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.
@net-foundation-cla
Copy link

net-foundation-cla bot commented Nov 1, 2021

CLA assistant check
All CLA requirements met.

@jfversluis
Copy link
Member

jfversluis commented Nov 3, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jfversluis
Copy link
Member

Hey @cpraehaus! Thanks so much for this contribution and your very first, wow! Hope your experience was good :)

I will make sure to have a look at this asap!

@jfversluis
Copy link
Member

Oops seems like there is a build error, would you be able to fix that?

Issue8384.xaml.cs(24,4): error CS0103: The name 'InitializeComponent' does not exist in the current context [D:\agent\2\s\Xamarin.Forms.Core.Windows.UITests\Xamarin.Forms.Core.Windows.UITests.csproj]

…UI unit tests)

Otherwise we get 'InitializeComponent' not found error when compiling UI unit test projects.
@cpraehaus
Copy link
Contributor Author

@jfversluis Thanks for the nice feedback and the hint - build should be fixed now. Didn't have a hard time to delve into XF details since I'm working with it for quite a while now. Mostly as user of XF of course - but from time to time you want/need to look into the details/code as well :-). Since I'm submitting issues myself I thought its time to contribute and help to (hopefully) reduce the number of issues in the backlog ...

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jfversluis
Copy link
Member

Appreciate the help @cpraehaus and thanks for sharing! I see a lot of similarities with how I started :)

Let me know if you need anything!

@jfversluis jfversluis merged commit 085ad87 into xamarin:5.0.0 Nov 4, 2021
jfversluis added a commit that referenced this pull request Nov 10, 2021
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.

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