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

[Bug] RefreshView "disabling" does not work on UWP #13749

Open
ivanv-microsoft opened this issue Feb 12, 2021 · 3 comments
Open

[Bug] RefreshView "disabling" does not work on UWP #13749

ivanv-microsoft opened this issue Feb 12, 2021 · 3 comments

Comments

@ivanv-microsoft
Copy link

Description

On UWP, if the RefreshView uses event subscription for refreshing, and you set RefreshView as IsEnabled = false, the entire collection (so all child elements) will be disabled (you cannot scroll or select any of the tiles).
That would be a natural behavior for a regular collection view, but according to RefreshView documentation, that's not the case for it. It should instead only disable the "pull to refresh" functionality. -- This is also how it works on Android (I did not check on iOS).

Alternatively, if the RefreshView uses a Command binding, the IsEnabled property is ignored, instead figuring out how to behave based on the Command.CanExecute. If that returns false, the behavior is the same as described above for UWP - meaning the entire collection is disabled, instead of just pull to refresh.
Works fine on Android.

Steps to Reproduce

  1. Open the attached solution
  2. Look at code in MainForm.xaml - there are the 2 examples described above. Uncomment whichever one you want to look at
  3. Run the project, and try to scroll or select items.

Expected Behavior

Should behave as on Android.
Also, to be honest ,the IsEnabled is a bad name for the property based on what it should do - maybe something like IsRefreshEnabled would be a better more clear name.

Actual Behavior

Everything is disabled

Basic Information

  • Version with issue: 4.8.0.1821
  • Last known good version: Unknown
  • IDE and IDE version: VS 2019
  • Platform with the issue:
  • Android: 10.0
  • UWP: Win 10 latest (not sure which build)

Reproduction Link

RefreshViewCanExecuteCommandBug.zip

Workaround

Have not found any workaround (apart from just always allowing pull to refresh, and doing nothing in the refresh method). Looks ugly, but at least the rest of the app works.

@ivanv-microsoft ivanv-microsoft added s/unverified New report that has yet to be verified t/bug 🐛 labels Feb 12, 2021
@jsuarezruiz jsuarezruiz self-assigned this Feb 16, 2021
@jsuarezruiz jsuarezruiz added p/UWP a/refreshview 🔄 e/3 🕒 3 and removed s/unverified New report that has yet to be verified labels Feb 16, 2021
@jsuarezruiz
Copy link
Contributor

By passing the IsEnabled property to the native control, in this case in UWP we use RefreshContainer, we disable the native control.

Then, the native control, disables the content. We need to review if we can use another approach.

@ivanv-microsoft
Copy link
Author

@jsuarezruiz , note that maybe this needs to also be split into 2 separate issues.
Command.CanExecute does not set IsEnabled, I assume, but still behaves the same way on UWP?

@cpraehaus
Copy link
Contributor

@ivanv-microsoft @jsuarezruiz In #14837 we added a new IsRefreshAllowed property which is currently not part of the official API (its an internal property). With this PR the behavior for Command-bindings changes: previously, Command.CanExecute = false disabled the RefreshView control with unwanted side effects (e.g. refresh animation immediately disabled). After the change, CanExecute does not toggle RefreshView.IsEnabled anymore but RefreshView.IsRefreshAllowed. This will only disable the "pull to refresh" functionality.

I also think that disabling all child content is not really what you want here. Therefore a dedicate new property makes sense. The PR calls it RefreshView.IsRefreshAllowed. If the PR gets accepted we could think of making this new property part of the public API for users which want to control the RefreshView directly, i.e. don't use the Command property.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants