Improve pull-to-refresh usability in RefreshContainer

[gentledepp]

What does the pull request do?

Pull to refresh is non-optimal in its current implementation:

1. you need to select the first item in a list to start a pull2refresh.
2. It is not supported on desktop devices and cannot optionally be added without rewriting a bunch of internal code.

What is the current behavior?

#15529 goes into great detail.
#17496 is the original pull request - but it seems to be abandoned.

So I:

• cherry-picked all changes from Improve Refresh-Container usability #17496 (the pr seems to be abandoned)
• incorporated your feedback (the changes you requested in Improve Refresh-Container usability #17496)
• added the possibility, to also enable pull2refresh on desktops (mouse-controlled devices)

What is the updated/expected behavior with this PR?

Pull to refresh works seemlessly for mobile and (optionally) also for desktop devices

How was the solution implemented (if it's not obvious)?

see #17496 for more details.

Checklist

• Added unit tests (if possible)?
• Added XML documentation to any related classes?
• Consider submitting a PR to https://github.com/AvaloniaUI/avalonia-docs with user documentation

I will consider submitting a PR to the documentation, if the desktop support property is accepted and the name is fixed.

Breaking changes

Obsoletions / Deprecations

Fixed issues

Fixes #15529

[avaloniaui-bot]

You can test this PR using the following package version. 12.0.999-cibuild0055974-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[cla-avalonia]

• All contributors have signed the CLA.

[thevortexcloud]

Does this mean it works with a mouse? Or is this just for testing? I personally always found it weird some of the gesture recognisers are touch only.

[gentledepp]

This is just a draft - please ignore - I needed the nuget package.
Its the pull2refresh PR including support for desktop apps (so yes, pull to refresh works on windows too)

[thevortexcloud]

Sounds like a significant improvement over the existing pull gesture recogniser then, which is impossible to test with without a touch screen.

[gentledepp]

@alexander.marek,

Please read the following Contributor License Agreement (CLA). If you agree with the CLA, please reply with the following:

@cla-avalonia agree

Contributor License Agreement
1 out of 2 committers have signed the CLA.

• @nickodei
• @alexander.marek

alexander.marek doesn't seem to be a GitHub user. Please add the emails used in your commits to your Github account accordingly.

@cla-avalonia agree

[gentledepp]

This is actually mainly the code of #17496, with one addition:

Added a styled property "IsEnabledOnDesktop" that allows to optionally enable the pulltorefresh gesture on Desktop applications as well.

[avaloniaui-bot]

You can test this PR using the following package version. 12.0.999-cibuild0056035-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[emmauss]

Could you make your pr description more descriptive? Even if it's based on #17496 , it should still follow contribution guidelines.

[gentledepp]

@emmauss

Essentially, I

• cherry-picked all changes from Improve Refresh-Container usability #17496
• incorporated your feedback (the changes you requested in Improve Refresh-Container usability #17496)
• added the possibility, to also enable pull2refresh on desktops (mouse-controlled devices)

The third feature is important. It allows for

• easier testing of pull2refresh (mac and windows)
• opt in pull2refresh for customers, that are confused why "this works on my phone but not my computer" (yes, they exist...)

IsEnabledOnDesktop is a styled property
So it is disabled by default, but can easily be enabled using a global style.

Note that the propertyname is not of my concern.
I understand that you have API design meetings and will most likely change it. You've got my blessings to do so (although you do not need them ;-))

[avaloniaui-bot]

You can test this PR using the following package version. 12.0.999-cibuild0056624-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[emmauss]

@emmauss

Essentially, I

* cherry-picked all changes from [Improve Refresh-Container usability #17496](https://github.com/AvaloniaUI/Avalonia/pull/17496)

* incorporated your feedback (the changes you requested in [Improve Refresh-Container usability #17496](https://github.com/AvaloniaUI/Avalonia/pull/17496))

* added the possibility, to also enable pull2refresh on desktops (mouse-controlled devices)

The third feature is important. It allows for

* easier testing of pull2refresh (mac and windows)

* opt in pull2refresh for customers, that are confused why "this works on my phone but not my computer" (yes, they exist...)

IsEnabledOnDesktop is a styled property So it is disabled by default, but can easily be enabled using a global style.

Note that the propertyname is not of my concern. I understand that you have API design meetings and will most likely change it. You've got my blessings to do so (although you do not need them ;-))

It would be best if you put this in the PR description and follow the section guides.

[MrJul]

Public API for review:

 namespace Avalonia.Input.GestureRecognizers
 {
     public class ScrollGestureRecognizer : GestureRecognizer
     {
+        public static readonly DirectProperty<ScrollGestureRecognizer, Vector?> OffsetProperty;
+        public static readonly DirectProperty<ScrollGestureRecognizer, Size?> ExtentProperty;
+        public static readonly DirectProperty<ScrollGestureRecognizer, Size?> ViewportProperty;
+        public Size? Extent { get; private set; }
+        public Size? Offset { get; private set; }
+        public Size? Viewport { get; private set; }
     }
 }
 
 namespace Avalonia.Controls
 {
     public class RefreshContainer : ContentControl
     {
+        public static readonly StyledProperty<bool> IsEnabledOnDesktopProperty;
+        public bool IsEnabledOnDesktop { get; set; }
     }
 }

[thevortexcloud]

I personally think it needs a better name than IsEnabledOnDesktop. Since that implies it wont work at all on a desktop. Which from my understanding is not true, it just enables mouse controls. Perhaps a better name would be SupportAllPointerDevices or UseAllPointerDevices?

Alternatively, allowing customisation of the whitelist for what devices it accepts would also be better and more flexible. The current flag is kind of an all or touch only approach.

[rubiomiguel06]

Thanks for the great development. Releasing this PR will finally fix my gesture orchestrating issues in android! Hopefully it happens soon

[MrJul]

Notes from the API review:

Remove the properties added to ScrollGestureRecognizer

Exposing Extent, Offset and Viewport shouldn't be necessary. We don't want to add every possible scroll info property as part of the public API of the gesture recognizer.

Instead, the ScrollGestureRecognizer should listen to property change notifications from its parent presenter, which is accessible through the existing Target property.

Rename IsEnabledOnDesktop to IsMouseEnabled

The property isn't really related to desktop: it simply enables the mouse to be recognized, even on mobile platforms.

[gentledepp]

Notes from the API review:

Remove the properties added to ScrollGestureRecognizer

Exposing Extent, Offset and Viewport shouldn't be necessary. We don't want to add every possible scroll info property as part of the public API of the gesture recognizer.

Instead, the ScrollGestureRecognizer should listen to property change notifications from its parent presenter, which is accessible through the existing Target property.

Rename IsEnabledOnDesktop to IsMouseEnabled

The property isn't really related to desktop: it simply enables the mouse to be recognized, even on mobile platforms.

Could you please give me a hint on how to do this?

Because IScrollable and ScrollViewer are actually defined in Avalonia.Controls which I do not have a reference to.

So simply deleting those properties and "using those of the Target" is not possible:

The Renaming has been done ;-)

[MrJul]

Because IScrollable and ScrollViewer are actually defined in Avalonia.Controls which I do not have a reference to.

Sorry for the late reply, you're completely right.

I'm not sure why ScrollGestureRecognizer is in the Base assembly when it's only used in Controls but that ship has sailed.

[avaloniaui-bot]

You can test this PR using the following package version. 12.0.999-cibuild0057964-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[avaloniaui-bot]

You can test this PR using the following package version. 12.0.999-cibuild0058042-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[gentledepp]

I reworked the recognizer - at some points the pull to refresh gesture had no more effect.
Also added test.

[avaloniaui-bot]

You can test this PR using the following package version. 12.1.999-cibuild0065280-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[avaloniaui-bot]

You can test this PR using the following package version. 12.1.999-cibuild0066439-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[avaloniaui-bot]

You can test this PR using the following package version. 12.1.999-cibuild0066720-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

[MrJul]

Note: I've updated the tests to avoid reflection.

[MrJul]

Re-tested, LGTM!