dotnet / maui

.NET MAUI is the .NET Multi-platform App UI, a framework for building native device applications spanning mobile, tablet, and desktop.
https://dot.net/maui
MIT License
22.22k stars 1.75k forks source link

[Enhancement] Fix the RefreshView implementation #1171

Open roubachof opened 3 years ago

roubachof commented 3 years ago

Since I saw you are currently working on RefreshView I thought you could maybe fix the old Xamarin.Forms implementation that is inconsistent with Android and iOS.

On XF, if you set IsRefreshing to true, it will automatically triggers the bound Command. This is not the case on Android and iOS, setting the equivalent to IsRefreshing just display the refresher view without triggering anything. Sometimes you just need to display the refresher ui while updating something in the background, you don't want this to trigger the command, it's just an UI feedback.

reference: https://developer.android.com/reference/androidx/swiperefreshlayout/widget/SwipeRefreshLayout https://developer.apple.com/documentation/uikit/uirefreshcontrol

ghost commented 2 years ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

ghost commented 1 year ago

Hi @roubachof. We have added the "s/needs-repro" label to this issue, which indicates that we require steps and sample code to reproduce the issue before we can take further action. Please try to create a minimal sample project/solution or code samples which reproduce the issue, ideally as a GitHub repo that we can clone. See more details about creating repros here: https://github.com/dotnet/maui/blob/main/.github/repro.md

This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

roubachof commented 1 year ago

Hi mister bot! This is not a bug but an enhancement cause the refresh view hasn't the same behavior than the Android or iOS implementation. So it doesn't require sample or repro repo

PureWeen commented 1 year ago

Sometimes you just need to display the refresher ui

I'd recommend a new API vs overloading "IsRefreshing"

The initial spec for RefreshView recommended an API for the Visualization. This API could also be used to supply a custom Visualization.

berhir commented 1 year ago

Changing the current behavior would be really great. Currently, we must use workarounds to ensure that the command does not get triggered twice. Sometimes we want to trigger the command from code (eg. when the user navigates to a page) and show the busy indicator.

We could set a property bound to IsRefreshing to true to trigger the command, but this breaks the MVVM pattern. Because in this case, the View Model relies on the UI to trigger the command. It won't work for example in automated tests where we test only the View Models without binding them to the UI.

roubachof commented 1 year ago

I created a "BehavingRefreshView" to correct the bad manners of the XF one:

    public class BehavingRefreshView : RefreshView
    {
        public static new readonly BindableProperty IsRefreshingProperty = BindableProperty.Create(
            nameof(IsRefreshing),
            typeof(bool),
            typeof(RefreshView),
            false,
            BindingMode.TwoWay,
            coerceValue: OnIsRefreshingPropertyCoerced,
            propertyChanged: OnIsRefreshingPropertyChanged);

        public new event EventHandler Refreshing;

        public new bool IsRefreshing
        {
            get => (bool)GetValue(IsRefreshingProperty);
            set => SetValue(IsRefreshingProperty, value);
        }

        private static void OnIsRefreshingPropertyChanged(BindableObject bindable, object oldValue, object newValue)
        {
            bool value = (bool)newValue;

            if (!value)
            {
                return;
            }

            var refreshView = (BehavingRefreshView)bindable;
            refreshView.Refreshing?.Invoke(bindable, EventArgs.Empty);
        }

        private static object OnIsRefreshingPropertyCoerced(BindableObject bindable, object value)
        {
            var view = (RefreshView)bindable;
            bool newValue = (bool)value;

            // IsRefreshing can always be toggled to false
            if (!newValue)
            {
                return value;
            }

            if (!view.IsEnabled)
            {
                return false;
            }

            if (view.Command == null)
            {
                return value;
            }

            if (!view.Command.CanExecute(view.CommandParameter))
            {
                return false;
            }

            return value;
        }
    }