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
21.84k stars 1.67k forks source link

ScrollView with Orientation=Both does not have correct values in ScrollX and ScrollY properties #16417

Open JimmiDK opened 11 months ago

JimmiDK commented 11 months ago

Description

When srolling the ScrollView controll where Orientation=Both the property values of ScrollX and ScrollY are incorrect. At any given time only one of the properties will have the correct value. After scrolling vertically the value of ScrollY is correct but the value of ScrollX is incorrectly always 0. Scrolling horizontally have the opposite error where the value of ScrollY is incorrectly always 0.

Problem seen on Android. iOS works. Other platforms have not been tested.

Steps to Reproduce

1: Build and run related repro sample located at https://github.com/JimmiDK/MAUI-Bug-report-16417---Repro-Sample

2: Scroll the ScrollView and observe the values of the ScrollX and ScrollY properties displayed at the buttom of the screen.

Actual Result: Only one of the ScrollX and ScrollY properties will have the correct value at the same time. Expected Result: Both ScrollX and ScrollY should always have the correct value.

Link to public reproduction project repository

https://github.com/JimmiDK/MAUI-Bug-report-16417---Repro-Sample

Version with bug

8.0.0-preview.5.8529

Last version that worked well

Unknown/Other

Affected platforms

Android

Affected platform versions

Android 11

Did you find any workaround?

No

Relevant log output

No response

ghost commented 11 months ago

Hi @JimmiDK. 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.

JimmiDK commented 11 months ago

Link to repro sample added

XamlTest commented 11 months ago

Verified this on Visual Studio Enterprise 17.8.0 Preview 1.0. Repro on Android 13.0-API33 .NET 8 with below Project: 16417.zip

RobDaytona commented 1 month ago

Having the same issue. Can't accurately read ScrollX and ScrollY properties of the scrollview, Scrolled Event also fires with e.ScrollX and ScrollY (one of them always is zero!) , for me its critical to get this fixed, I have a Map Image in a scrollview, the player can scroll around on it. There is a minimap with a frame showing where on the larger map the player is, I need to be able to update this frame accurately as the player scrolls around the main map. But the frame in the minimap keep jumping to 0 in any one of the dimenisons, so it looks stupid and does not work. This is the same problem as described here, please can you get this fixed asap. thanks!

RobDaytona commented 1 month ago

This needs fixing please bump it up!

RobDaytona commented 1 month ago

Thanks Javier / @jsuarezruiz for adding it to the backlog, please do not take this the wrong way, but this is a critical, bug to fix for the new game I am developing as explained in my initial post. I am going to be commenting here a lot.

RobDaytona commented 1 month ago

Can someone @jsuarezruiz @jfversluis possibly help me by giving me a link to the relevant source code for me to download and take a look myself please if it is indeed possible? - Thanks Javier & Gerald as ever for your tips and suggestions!

jfversluis commented 1 month ago

This is the Android control we're actually using here: src/Core/src/Platform/Android/MauiScrollView.cs, this is probably where it originates from.

I see two lines child.ScrollBy((int)dX, 0); and _parentScrollView.ScrollBy(0, (int)dY); that look suspicious since they both provide 0 as a hardcoded value, but might be a red herring.

Things to maybe explore:

RobDaytona commented 1 month ago

@jfversluis Thanks! That does indeed look suspicious unless its a single scroll direction part of the code, in which case it would be correct, but for scrollgrid with both scrolls it looks wrong for sure.

I am a total beginner at Github I have never used it for code, I did a search for the file you mentioned and found this URL: https://github.com/dotnet/maui/blob/5ab0e9ceff42ce7eb4903c58d2cc2fb1660b2545/src/Core/src/Platform/Android/MauiScrollView.cs#L4

But I have no idea how to work on this code file? What steps do I nee to take, presumably I need to download an entire solution or project containing this file? I'd then have to compile that and then reference that compiled code locally from my local project instead of Microsoft.Maui like I would otherwise do? Any resources that show me how to do this would be appreciated, I am searching but find a lot of stuff to read and watch that is not exactly the right thing to be consuming. thanks Rob

RobDaytona commented 1 month ago

I am looking at the file @jfversluis suggested, just as a single file I downloaded to read.. and this bit definately looks very wrong: // The nested ScrollViews will allow us to scroll EITHER vertically OR horizontally in a single gesture. // This will allow us to also scroll diagonally. // We'll fall through to the base event so we still get the fling from the ScrollViews. // We have to do this in both ScrollViews, since a single gesture will be owned by one or the other, depending // on the initial direction of movement (i.e., horizontal/vertical). if (_isBidirectional) // // See also MauiHorizontalScrollView notes in OnInterceptTouchEvent { float dX = LastX - ev.RawX;

            LastY = ev.RawY;
            LastX = ev.RawX;
            if (ev.Action == MotionEventActions.Move)
            {
                foreach (MauiHorizontalScrollView child in this.GetChildrenOfType<MauiHorizontalScrollView>())
                {
                    child.ScrollBy((int)dX, 0);   //<----- SHOULD BE child.ScrollBy((int)dX, (int)dY);  for diagonal scrolling as per comments and name of this method
                    break;
                }
                // Fall through to base.OnTouchEvent, it'll take care of the Y scrolling                
            }
        }

        and the same here:

                    // If the touch is caught by the horizontal scrollview, forward it to the parent 
        _parentScrollView.ShouldSkipOnTouch = true;
        _parentScrollView.OnTouchEvent(ev);

        // The nested ScrollViews will allow us to scroll EITHER vertically OR horizontally in a single gesture.
        // This will allow us to also scroll diagonally.
        // We'll fall through to the base event so we still get the fling from the ScrollViews.
        // We have to do this in both ScrollViews, since a single gesture will be owned by one or the other, depending
        // on the initial direction of movement (i.e., horizontal/vertical).
        if (IsBidirectional)
        {
            float dY = _parentScrollView.LastY - ev.RawY;

            _parentScrollView.LastY = ev.RawY;
            _parentScrollView.LastX = ev.RawX;
            if (ev.Action == MotionEventActions.Move)
            {
                _parentScrollView.ScrollBy(0, (int)dY);    //<----- SHOULD BE child.ScrollBy((int)dX, (int)dY);  for diagonal scrolling as per comments and name of this method
                // Fall through to base.OnTouchEvent, it'll take care of the X scrolling                    
            }
        }

        Looks a very simple fix, but its beyond me to do more than read this and tell you for certain this is wrong. I can't compile or test this.. 

       Can any one help further? Please.

        There is a comment that says this:
         // Fall through to base.OnTouchEvent, it'll take care of the X scrolling  (similarily for Y)
         So that may be correct for iOS, but there seems to be no fall through for Android, or if I got that statement wrong, then its the fall through that simply does not work... 

         My hunch is there is no fall through, and it should be child.ScrollBy((int)dX, (int)dY);  in both cases..
RobDaytona commented 1 month ago

I see this issue is the same and its closed: Related / The Same: https://github.com/dotnet/maui/issues/19515 I think it got closed down with a work around, but I don't understand exactly how to implement it at this stage.

RobDaytona commented 1 month ago

I implemented the workaround proposed in the similar issue I mentioned in my previous comment and it worked perfectly, here is the step by step full workaround:

Step1 Create a new file named CustomMauiScrollViewHandlingCode in the android folder and add these 2 classes into it:

using Android.Content; using Android.Views; using Microsoft.Maui.Controls.Platform; using Microsoft.Maui.Platform; using Microsoft.Maui.Handlers; using Microsoft.Maui.Controls;

namespace XXX.Platforms.Android { public class CustomMauiScrollView : MauiScrollView { private int currentScrollX = 0; private int currentScrollY = 0;

    public CustomMauiScrollView(Context context) : base(context)
    {
    }

    public CustomMauiScrollView(Context context, IAttributeSet attrs) : base(context, attrs)
    {
    }

    public CustomMauiScrollView(Context context, IAttributeSet attrs, int defStyleAttr) : base(context, attrs, defStyleAttr)
    {
    }

    protected CustomMauiScrollView(nint javaReference, JniHandleOwnership transfer) : base(javaReference, transfer)
    {
    }

    protected override void OnScrollChanged(int scrollX, int scrollY, int oldScrollX, int oldScrollY)
    {
        if (scrollX != 0 && scrollY == 0)
        {
            currentScrollX = scrollX;
        }
        else if (scrollY != 0 && scrollX == 0)
        {
            currentScrollY = scrollY;
        }
        else if (scrollX == 0 && oldScrollX != 0)
        {
            currentScrollX = scrollX;
        }
        else if (scrollY == 0 && oldScrollY != 0)
        {
            currentScrollY = scrollY;
        }
        base.OnScrollChanged(currentScrollX, currentScrollY, oldScrollX, oldScrollY);
    }
}

public class CustomScrollViewHandler : ScrollViewHandler
{
    protected override MauiScrollView CreatePlatformView()
    {
        var platformView = new CustomMauiScrollView(
            new ContextThemeWrapper(MauiContext!.Context, Resource.Style.scrollViewTheme), null!,
            Resource.Attribute.scrollViewStyle)
        {
            ClipToOutline = true,
            FillViewport = true
        };

        return platformView;
    }
}

} Step 2: Register Custom Scroll View Handler in MauiProgram Modify your MauiProgram class to register the custom scroll view handler for Android. Use conditional compilation to ensure it’s only applied on Android:

using CommunityToolkit.Maui; using Microsoft.Extensions.Logging; using Microsoft.Maui.Hosting; using Plugin.Maui.Audio; using Microsoft.Maui.Controls.Hosting; using Microsoft.Maui.Controls.Handlers;

namespace XXX { public static class MauiProgram { public static MauiApp CreateMauiApp() { var builder = MauiApp.CreateBuilder(); builder .UseMauiApp() .ConfigureFonts(fonts => { fonts.AddFont("arial.ttf", "Arial"); //you can use any font }) .UseMauiCommunityToolkitMediaElement(); //you probably don't use this

        // Register platform-specific services

if ANDROID

        builder.ConfigureMauiHandlers(handlers =>
        {
            handlers.AddHandler(typeof(ScrollView), typeof(XXX.Platforms.Android.CustomScrollViewHandler));
        });

endif

        // Register MainPage and App as singletons
        builder.Services.AddSingleton<MainPage>();
        builder.Services.AddSingleton<App>();

if DEBUG

        builder.Logging.AddDebug();

endif

        return builder.Build();
    }
}

} Step 3: Use ScrollView in Shared Code In your shared code, you can use ScrollView as normal.

RobDaytona commented 1 month ago

Not sure why my post was chopped up like that? - is there a way to avoid that?

RobDaytona commented 1 month ago

PLEASE DO NOT CLOSE THIS TICKET - A proper permanent solution is needed, and from the looks of it, it should be very simple to implement. thanks. I have spent far too long on this issue, and wasted so much time, please do not let the same happen to others, please fix it properly. Thanks!