xamarin / Xamarin.Forms

Xamarin.Forms is no longer supported. Migrate your apps to .NET MAUI.
https://aka.ms/xamarin-upgrade
Other
5.62k stars 1.87k forks source link

[Enhancement] Add VerticalScrollMode/HorizontalScrollMode ScrollView #2680

Closed samhouts closed 5 years ago

samhouts commented 6 years ago

Summary

This would allow situations where a user would like to create a ListView or a ScrollView that can temporarily or permanently disable scrolling.

ListView will only have VerticalScrollMode until #1727 is implemented.

API Changes

Add:

enum ScrollMode 
{
     Enabled,
     Disabled
}

ListView and ScrollView will have a new BindableProperty:


public ScrollMode VerticalScrollMode { get; set; } //bp

ScrollView will also have another new BindableProperty:


public  ScrollMode HorizontalScrollMode { get; set; } //bp

UWP defines these here:

Intended Use Case

User wants to add a PanGestureRecognizer or a SwipeGestureRecognizer to a ListView ViewCell that will handle Horizontal swipes. However, the ListView scrolling up or down due to touch slop causes the pan to be cancelled or ignored.

With this new API, user can disable the vertical scrolling of the ListView when the pan gesture starts and enable it when the gesture ends, like so:

ListView _parent => Parent as ListView;

void HandlePanUpdated(object sender, PanUpdatedEventArgs e)
{
    if (_parent != null)
        _parent.VerticalScrollMode = ScrollMode.Disabled;

    if (e.StatusType == GestureStatus.Started)
    {
        // do work
    }
    else if (e.StatusType == GestureStatus.Running)
    {
        // do work
    }
    else if (e.StatusType == GestureStatus.Completed || e.StatusType == GestureStatus.Canceled)
    {
        if (_parent != null)
            _parent.VerticalScrollMode = ScrollMode.Enabled;

        // do work
    }
}
samhouts commented 6 years ago

Question: Should these new properties on the ScrollView replace the OrientationProperty?

Nixon-Joseph commented 6 years ago

Another use case is one I had to use a custom renderer for. I needed a carousel view for a checkout process page, and all of the existing carousel views I could find failed me in some way or another, usually in rendering weirdness, or some touch event not working properly/consistantly.

I created my own carousel from a horizontal scrollview, and didn't want people to be able to actually swipe(scroll) manually. So I needed to disable scrolling in the renderer. Would have been much simpler if I had the property available already.

iOS:

protected override void OnElementChanged(VisualElementChangedEventArgs e)
        {
            base.OnElementChanged(e);
...

            HandleIsSwipeEnabled();
...
        }
void ElementPropertyChanged(object sender, PropertyChangedEventArgs e)
        {
...
            if (e.PropertyName == CarouselLayout.IsSwipeEnabledProperty.PropertyName)
            {
                HandleIsSwipeEnabled();
            }
...
        }
private void HandleIsSwipeEnabled()
        {
            if (this.Element is CarouselLayout el)
            {
                ScrollEnabled = el.IsSwipeEnabled;
            }
        }

Android:

void ElementPropertyChanged(object sender, PropertyChangedEventArgs e)
        {
            ...
            if (e.PropertyName == "Renderer")
            {
                ...
                _scrollView.Touch += HScrollViewTouch;
                ...
            }
            ...
        }

void HScrollViewTouch(object sender, TouchEventArgs e)
        {
            if (this.Element is CarouselLayout el && el.IsSwipeEnabled)
            {
                e.Handled = false;

                switch (e.Event.Action)
                {
                   ...
                }
            }
        }
charlesroddie commented 6 years ago

@samhouts Question: Should these new properties on the ScrollView replace the OrientationProperty?

One option is to add an element to the ScrollOrientation Enum.

int Name Description
0 Vertical Scroll vertically.
1 Horizontal Scroll Horizontally.
2 Both Scroll both horizontally and vertically.
3 Neither Scroll Disabled.

Then the bools or bool-like enums VerticalScrollMode and HorizontalScrollMode don't need to be exposed as public properties, for a simple API, and there isn't duplication or depreciation.

But however the API is done, this functionality will be great to have.

StephaneDelcroix commented 6 years ago

please make sure the SizeRequest (and MinSizeRequest) is properly computed if the scrolls are disabled

charlesroddie commented 6 years ago

Trying to iron out the API so this can be worked on.

ListView

This is not intended to scroll in two dimensions, so we should keep the Orientation and add a ScrollEnabled bool or an equivalent ScrollMode. Otherwise we have an invalid state when VerticalScrollMode and HorizontalScrollMode are both set to true.

ScrollView

Are we happy to go with a) adding the value to ScrollOrientation as I suggested above?

Alternatives: b) Implement HorizontalScrollMode and VerticalScrollMode, have OrientationProperty set the underlying bools, and depreciate OrientationProperty. Or c) Add a ScrollEnabled bool.

@AxelUser could you look into the ScrollView part please?

AxelUser commented 6 years ago

Hi there! I've made fixes (#3113) for ScrollView: added ScrollOrientation.Neither and made handling it in ScrollView.

AndreiMisiukevich commented 6 years ago

I think, everybody can use Rotation 90 degree for ListView and -90 degree for each cell for achieving such behavior for ListView

jrahma commented 5 years ago

WebView inside ScrollView not scrolling +1

onurhazar commented 4 years ago

@samhouts Hello, what is the status of this enhancement? why is it closed? I couldn't find any property like you mentioned in the title. It would be great if this feature is added to ScrollView.

samhouts commented 4 years ago

@onurhazar It was implemented as described in https://github.com/xamarin/Xamarin.Forms/pull/3113. Does that work for you? Thanks!

Pastajello commented 3 years ago

So, I return to the topic of disabling the Scroll on ScrollView. I've already posted in the #3113 my thoughts about the implementation. Basically that implementation is a bug not a feature ;) So I'd say this is not implemented at all and doest solve any issue