Xamarin.forms: [Bug] RefreshView pull to refresh command firing twice on a single pull

[davidbritch]

Description

In 4.3-pre3, it looks like RefreshView Command is firing twice on a pull gesture. This occurs on both iOS and Android. The method in my view model that's executed when the command fires is called twice.

This was working just fine when I tried it in a CollectionView (4.3-pre1), and I'm pretty certain it was fine when I tried it with a CarouselView in 4.3-pre2.

Steps to Reproduce

1. Run the attached sample.
2. Browse to the pull to fresh page.
3. Pull to refresh.

Expected Behavior

10 items changes to 20 items.

Actual Behavior

10 items changes to 30 items.

Basic Information

• Version with issue: 4.3-pre3
• Last known good version: I'm pretty certain this was working fine in 4.3-pre2
• IDE: VSMac 8.3.1
• Platform Target Frameworks:
  
  
  
  • iOS: 13.0
  
  
  • Android: API 28
  
  

Screenshots

Prior to the refresh (10 items):

After the refresh (30 items - should be 20):

Reproduction Link

CarouselViewDemos.zip

[davidbritch]

I thought this might be because I'm setting the AnimalsViewModel.IsRefreshing property (which RefreshView.IsRefreshing binds to) to true, so I commented out that code, and the command still fires twice.

[adrianknight89]

@samhouts This issue exists in CollectionView as well.

[jsuarezruiz]

Related PR: https://github.com/xamarin/Xamarin.Forms/pull/7840

[FranVillaVainilla]

Related to this Issue, i think setting IsRefreshing=true should not fire the command.

In my opinion, pull gesture should simply fire the refresh command and let the command the responsibility to set some property you can bind to IsRefreshing, true at the beginning and false at the end.

I think this way is more flexible, and is simplest to use the same command both for pull to refresh an , as for example, to load data when a page is presented.

[PureWeen]

This should be resolved by
https://github.com/xamarin/Xamarin.Forms/pull/7866

@davidbritch let me know if that doesn't seem correct

@FranVillaVainilla
Tying IsRefreshing to the visualization and firing the command is good for consistency. When you perform the pull gesture that sets the native component to a state of "IsRefreshing" that state at the native layer needs to be mirrored exactly to the cross platform IsRefreshing bit so that the developer knows what's happening and can deactivate it if needed.

[davidbritch]

@PureWeen All good from my end.