Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ScrollView's scroll position is not notified correctly on Android. #19515

Open
cat0363 opened this issue Dec 20, 2023 · 8 comments
Open

ScrollView's scroll position is not notified correctly on Android. #19515

cat0363 opened this issue Dec 20, 2023 · 8 comments
Labels
Milestone

Comments

@cat0363
Copy link
Contributor

cat0363 commented Dec 20, 2023

Description

When observing the scroll position of ScrolledEvent with ScrollView's Orientation set to Both, the correct position is not stored in ScrolledEventArgs.

Below is a reproduction video that follows the steps to reproduce.

Android.Emulator.-.pixel_2_-_api_30_5556.2023-12-20.13-47-40.mp4

ScrollX and ScrollY of ScrollEventArgs when scrolling are displayed at the bottom of the screen.

As stated in the reproduction steps, the values ​​stored in ScrollX and ScrollY of ScrollEventArgs are not continuous, and 0 is sometimes mixed in with each.
0 is mixed, so if you try to link multiple ScrollViews together, it will not work properly on Android.

Steps to Reproduce

The steps to reproduce are as follows.

  1. Launch the app uploaded to github with the device on Android
  2. Scroll diagonally down the scroll area in the top half of the app.

As of step 2, the ScrollX and ScrollY values ​​of ScrolledEventArgs observed within the ScrolledEvent are not continuous, and 0 is sometimes mixed.

Screenshot_1703047669

The 0 that is sometimes stored in ScrollX and ScrollY is not intended.

In step 2, I was expected that there would be continuity in the values ​​stored in ScrollX and ScrollY of ScrollEventArgs.

Link to public reproduction project repository

https://github.com/cat0363/Maui-IssueScrollViewPosition.git

Version with bug

8.0.3

Is this a regression from previous behavior?

Not sure, did not test other versions

Last version that worked well

Unknown/Other

Affected platforms

Android

Affected platform versions

Android 11.0, 12.0, 13.0

Did you find any workaround?

None.

Relevant log output

No response

@cat0363 cat0363 added the t/bug Something isn't working label Dec 20, 2023
@PureWeen
Copy link
Member

possibly related #14594

@PureWeen PureWeen added this to the Backlog milestone Dec 20, 2023
@ghost
Copy link

ghost commented Dec 20, 2023

We've added this issue to our backlog, and we will work to address it as time and resources allow. If you have any additional information or questions about this issue, please leave a comment. For additional info about issue management, please read our Triage Process.

@Eilon Eilon added the area-layout StackLayout, GridLayout, ContentView, AbsoluteLayout, FlexLayout, ContentPresenter label Dec 20, 2023
@cat0363
Copy link
Contributor Author

cat0363 commented Jan 12, 2024

@PureWeen , This issue may not be related to Issue #14594.

I found a workaround for this problem.

If Android's ScrollView specifies Both for Orientation and moves both horizontally and vertically, the coordinates notified in one Scrolled event will have either the X or Y coordinate set to 0. You will be notified.

This is probably because when nesting NestedScrollView and HorizontalScrollView and notifying the coordinates in the ScrollChange event when each scroll position changes, the value of the direction different from the scrolled direction is intentionally set to 0.

(x, y) = (90, 0)
(x, y) = (0, 74)

For this reason, the following notifications will not be sent all at once within the Scrolled event.

(x, y) = (90, 74)

The reason why 0's are sometimes mixed in is due to this implementation.

I solved this problem by reimplementing MauiScrollView as below.

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;
    }
}

I defined a class variable that holds the current scroll position so that it will hold them when the scroll position is signaled.
The coordinates that are notified in one notification are always notified with either the X coordinate or the Y coordinate as 0, so we saved the coordinates that are not 0 and used them in the notification.
When updating the coordinates to 0, the coordinates are updated to 0 only if the previous coordinates were not 0 and the current coordinates are 0.

Below is the verification video.

Android.Emulator.-.pixel_2_-_api_30_5554.2024-01-12.11-08-34.mp4

You can see that the X and Y coordinates of the coordinates being notified are consecutively notified without any 0s being mixed in.

If you want to use the custom scroll view handler from earlier, add the following description.

            var builder = MauiApp.CreateBuilder();
            builder
                .UseMauiApp<App>()
                .ConfigureMauiHandlers(handlers =>
                {
#if ANDROID
                    handlers.AddHandler<ScrollView, CustomScrollViewHandler>();
#endif
                })
                .ConfigureFonts(fonts =>
                {
                    fonts.AddFont("OpenSans-Regular.ttf", "OpenSansRegular");
                    fonts.AddFont("OpenSans-Semibold.ttf", "OpenSansSemibold");
                });

@cat0363
Copy link
Contributor Author

cat0363 commented Jan 12, 2024

I changed the code below on the MAUI side and it worked as expected.

[src\Core\src\Handlers\ScrollView\ScrollViewHandler.Android.cs]

int _currentScrollX;
int _currentScrollY;

void ScrollChange(object? sender, AndroidX.Core.Widget.NestedScrollView.ScrollChangeEventArgs e)
{
    var context = (sender as View)?.Context;

    if (context == null)
    {
        return;
    }

    if (e.ScrollX != 0 && e.ScrollY == 0)
    {
        _currentScrollX = e.ScrollX;
    }
    else if (e.ScrollY != 0 && e.ScrollX == 0)
    {
        _currentScrollY = e.ScrollY;
    }
    else if (e.ScrollX == 0 && e.OldScrollX != 0)
    {
        _currentScrollX = e.ScrollX;
    }
    else if (e.ScrollY == 0 && e.OldScrollY != 0)
    {
        _currentScrollY = e.ScrollY;
    }

    VirtualView.VerticalOffset = Context.FromPixels(_currentScrollY);
    VirtualView.HorizontalOffset = Context.FromPixels(_currentScrollX);
}

@awp-sirius
Copy link

awp-sirius commented Jan 31, 2024

In version 8.0.6 the problem is relevant.

I made it work by adding my ScrollViewHandler for the Android platform.

using Microsoft.Maui.Handlers;
using Microsoft.Maui.Platform;
using View = Android.Views.View;

namespace YourApp.Platforms.Android.Handlers;

public class BothScrollViewHandler : ScrollViewHandler
{
    protected override void ConnectHandler(MauiScrollView platformView)
    {
        platformView.ScrollChange += ScrollChange;
    }

    protected override void DisconnectHandler(MauiScrollView platformView)
    {
        platformView.ScrollChange -= ScrollChange;
    }

    private int _currentScrollX;
    private int _currentScrollY;

    private void ScrollChange(object? sender, AndroidX.Core.Widget.NestedScrollView.ScrollChangeEventArgs e)
    {
        if ((sender is View view) && (view.Context != null))
        {
            if (e.ScrollX != 0 && e.ScrollY == 0)
            {
                _currentScrollX = e.ScrollX;
            }
            else if (e.ScrollY != 0 && e.ScrollX == 0)
            {
                _currentScrollY = e.ScrollY;
            }
            else if (e.ScrollX == 0 && e.OldScrollX != 0)
            {
                _currentScrollX = e.ScrollX;
            }
            else if (e.ScrollY == 0 && e.OldScrollY != 0)
            {
                _currentScrollY = e.ScrollY;
            }

            VirtualView.VerticalOffset = Context.FromPixels(_currentScrollY);
            VirtualView.HorizontalOffset = Context.FromPixels(_currentScrollX);
        }
    }
}

Use the ConfigureMauiHandlers extension method to add my handler:

var builder = MauiApp.CreateBuilder();
...
builder.ConfigureMauiHandlers(handlers =>
{
#if __ANDROID__
    handlers.AddHandler(typeof(ScrollView), typeof(Platforms.Android.Handlers.BothScrollViewHandler));
#endif
});

it works! But I'm worried because I couldn't add
platformView.CrossPlatformArrange = VirtualView.CrossPlatformArrange;
in override ConnectHandler

@RobDaytona
Copy link

RobDaytona commented Jun 6, 2024

@cat0363 @awp-sirius Is the above code a work around? The issue seems to remain..
#16417

@cat0363
Copy link
Contributor Author

cat0363 commented Jun 6, 2024

@RobDaytona , The code I presented is a workaround.
Only one of the X and Y coordinates will be updated in a single notification. Therefore, by temporarily saving the coordinates that will not be updated, this workaround prevents the coordinates from being unintentionally initialized to 0.
This is a workaround, not a solution.

@RobDaytona
Copy link

RobDaytona commented Jun 7, 2024

@cat0363 I implemented this into my solution and it works great, thank you for your workaround solution!

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

No branches or pull requests

6 participants