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

[Bug] CollectionView can not be scrolled to bottom #13547

Open Tommigun1980 opened 3 years ago

Tommigun1980 commented 3 years ago

Description

What: Trying to programmatically scroll a CollectionView to the bottom (ie to the last item in its ItemsSource).

Use case: Chat page. Load say 100 last messages, scroll the CollectionView to the bottom to display the last messages.

Problem: There seems to be a design mistake in the CollectionView as this is almost impossible to achieve.

There seems to be some internal logic in the CollectionView that assumes its visualisation starts at the beginning of its contents list. It should be possible to have say 100 elements and scroll to the bottom (to display say the last 15 of it), without first having to reveal/draw all 85 items before it - the CollectionView is supposed to provide virtualisation, but even moreso basic functionality to scroll to an arbitrary element.

Also, while hacking around to achieve this I was also able to uncover several events that were not fired correctly.


The bugs can be summed up as such: 1) ItemsUpdatingScrollMode="KeepLastItemInView" does not work when populating a CollectionView, especially when updating it in one go with a RESET event (such as when using OptimizedObservableCollection). It either doesn't scroll at all, or scrolls a few elements only, when KeepLastItemInView is active while populating its ItemsSource.

Workaround: Use ScrollTo(lastIndex).

2) Calling ScrollTo inside an event (Scrolled, ChildAdded, SizeChanged) does nothing. ScrollTo starts working only after an unspecified amount of time has passed.

Workaround: Add a delay, say 100 milliseconds, before invoking ScrollTo (this is very bad as it's device and content dependent!).

3) Calling ScrollTo to an element (or index) at the end of its ItemsSource does not scroll to said element, unless that element/index is currently visible [or has recently been drawn]. It will randomly stop at some element before it. So if you have say 100 elements and ask it to scroll to the last element, it may stop at element number 25, 50, whatever.

Workaround: Call ScrollTo in a loop, and stop when the 'Scrolled' event says it's at the bottom. CollectionView seems to require that the element is visible [or has already been drawn] before it can be scrolled to. Furthermore, even this won't work unless ScrollTo is called with animate: true in certain cases.

4) CollectionView.Scrolled event is not invoked for all scroll actions when doing step 3), or alternatively is invoked with incorrect event data. So unfortunately even the events are not reliable.


So the problems are that: a) ItemsUpdatingScrollMode="KeepLastItemInView" does not scroll to the last message when populating data. b) ScrollTo for an arbitrary element does not fully scroll to an element unless it has drawn it at least once (thus defeating the purpose of virtualisation). This forces one to use black magic to sequentially scroll to elements in the background while keeping the CollectionView obscured by some element to not make it visible to the user. c) ScrollTo does nothing if called inside an event (Scrolled, ChildAdded, SizeChanged) -- one has to wait an arbitrary amount of time after these actions before ScrollTo does anything. d) Not all events are called (such as 'Scrolled') for all actions, further increasing the number of hacks one has to perform.


So all in all, all I want to do is to ask the CollectionView to scroll to the last element, but it's not possible without the following (timing-dependent) hacks in a page's code-behind file:

        // this.chatMessages is the CollectionView
        // this.vm.ChatMessages is the ItemsSource, of type OptimizedObservableCollection

        // CollectionView.ChildAdded event
        private async void ChatMessages_ChildAdded(object sender, ElementEventArgs e)
        {
            // TEMP HACK! Bug 1: if requested to scroll to the bottom at load time, need to wait for the first item to be drawn before scroll will work.
            // ItemsUpdatingScrollMode="KeepLastItemInView" also does nothing when loading data.
            if (this.tempHackIsInitialScrollToLastMessageRequested)
            {
                if (this.vm.ChatMessages.Count > 0)
                {
                    this.tempHackIsInitialScrollToLastMessageRequested = false;
                    await Task.Delay(20); // Bug 2: this needs to be increased the more stuff there is in the list, as scrolling instantly after this gets completely ignored
                    this.ScrollToBottom();

                    // HACK: Bug 3: because the scrolled event doesn't get called when the above does successfully manage to scroll to the last item, we have to unlock this here instead of in the scroll event...
                    await Task.Delay(250); // Bug 2: must have enough time to actually scroll to bottom
                    this.isChatScrolledToBottom = true;
                    this.HasFullyLoadedChatMessages = true;
                }
            }
        }

        // CollectionView.Scrolled event
        private async void ChatMessages_Scrolled(object sender, ItemsViewScrolledEventArgs e)
        {
            // TEMP HACK! the above scroll won't always scroll to bottom, so keep on scrolling recursively
            if (!this.HasFullyLoadedChatMessages)
            {
                if (this.vm.ChatMessages.Count > 0 && e.LastVisibleItemIndex < this.vm.ChatMessages.Count - 1)
                {
                    await Task.Delay(150); // Bug 2: this needs to be increased the more stuff there is in the list, as scrolling instantly after this gets completely ignored
                    this.ScrollToBottom();
                }
                // HACK: Bug 3: can't do this, because if the first ScrollTo (in ChatMessages_ChildAdded) manages to successfully scroll to the last item correctly,
                // then the scolled listener is event NEVER fired more than once, even though the e.LastVisibleItemIndex does change...
                /*else
                {
                    await Task.Delay(20); // Bug 2: allow scroll in of last item to happen before revealing content (ie removing overlay)
                    this.isChatScrolledToBottom = true;
                    this.HasFullyLoadedChatMessages = true;
                }*/
            }
            // this is actually working, keep this once the hacks above have been removed
            else
            {
                //Console.WriteLine($"SCROLL! Will be at bottom: {e.LastVisibleItemIndex == this.vm.ChatMessages.Count - 1}. Was at bottom: {this.isChatScrolledToBottom}. {e.LastVisibleItemIndex}/{this.vm.ChatMessages.Count - 1}");
                this.isChatScrolledToBottom = e.LastVisibleItemIndex == this.vm.ChatMessages.Count - 1;
            }
        }

        // CollectionView.SizeChanged event - iOS virtual keyboard makes the CollectionView smaller which forces it to scroll upwards, so force scroll to bottom when needed
        private async void ChatMessages_SizeChanged(object sender, EventArgs e)
        {
            //Console.WriteLine($"SIZE! Is at bottom: {this.isChatScrolledToBottom}");
            if (this.isChatScrolledToBottom)
            {
                if (this.vm.ChatMessages.Count > 0)
                {
                    await Task.Delay(20); // Bug 2 - ScrollTo doesn't do anything if called from inside this event, unless waiting a bit
                    this.ScrollToBottom(animate: true); // Bug 4: when CollectionView is made smaller and it's scrolled to last item, the scrolled event's indices will be incorrect UNLESS ANIMATED
                }
            }
        }

        private void ScrollToBottom(bool animate = false)
        {
            if (this.vm.ChatMessages.Count > 0)
                this.chatMessages.ScrollTo(this.vm.ChatMessages.Count - 1, -1, ScrollToPosition.End, animate);
        }

        // the below gets called after the CollectionView has populated -- we request it to scroll to the bottom:
                        // TEMP HACK: can't call ScrollTo while the CollectionView is being populating, as it will just be ignored...
                        //this.ScrollToBottom();
                        // ...instead request to scroll to the last message; will be handled in ChildAdded event
                        this.tempHackIsInitialScrollToLastMessageRequested = true;

This is on iOS - I haven't tested it on Android.

Also, even if ScrollTo did work properly it should not be necessary to draw the first elements if I want to display the last elements. ItemsUpdatingScrollMode="KeepLastItemInView" when populating data as a batch (ie filling the collection and sending a RESET event) should display the last items without first drawing the ones at the start of the list.

Steps to Reproduce

The repro project at https://github.com/xamarin/Xamarin.Forms/issues/13231 can be used as basis for revealing the issues. It uses a real asynchronously populated data source and not a hardcoded XAML data source, which often masks issues, and displays the ItemsUpdatingScrollMode="KeepLastItemInView" issue (contents are not scrolled to bottom).

For the ScrollTo issues etc, I think it'd make sense for you to try to try editing said repro project in a way where it scrolls to the last message. The road blocks should become evident. I can make a repro project for this if needed but I think it'd be better for you to discover the issues along the way.

It is very important to use an actual data source (like an asynchronously populated ObservableCollection) instead of hard-coding an XAML ItemsSource, as the latter masks a lot of issues.

Expected Behavior

It should be possible to scroll to the last element in a CollectionView.

Actual Behavior

Scrolling to the last element in a CollectionView is not possible without a ton of timing-dependent hacks.

Basic Information

Environment

Show/Hide Visual Studio info ``` === Visual Studio Community 2019 for Mac (Preview) === Version 8.9 Preview (8.9 build 1451) Installation UUID: 8872f256-c18e-42c9-a1d4-3d7d3b790000 GTK+ 2.24.23 (Raleigh theme) Xamarin.Mac 6.18.0.23 (d16-6 / 088c73638) Package version: 612000113 === Mono Framework MDK === Runtime: Mono 6.12.0.113 (2020-02/4fdfb5b1fd5) (64-bit) Package version: 612000113 === Xamarin Designer === Version: 16.9.0.266 Hash: c4842c761 Branch: remotes/origin/c4842c761b9b6a95407f72278ca7fb42f8f7fdf0 Build date: 2021-01-07 06:17:08 UTC === Roslyn (Language Service) === 3.9.0-3.20619.14+df59a33fd9beff9790e01a2a1ab21e4a1e6921b3 === NuGet === Version: 5.8.0.6860 === .NET Core SDK === SDK: /usr/local/share/dotnet/sdk/5.0.102/Sdks SDK Versions: 5.0.102 5.0.101 5.0.100 3.1.405 3.1.404 3.1.403 MSBuild SDKs: /Applications/Visual Studio.app/Contents/Resources/lib/monodevelop/bin/MSBuild/Current/bin/Sdks === .NET Core Runtime === Runtime: /usr/local/share/dotnet/dotnet Runtime Versions: 5.0.2 5.0.1 5.0.0 3.1.11 3.1.10 3.1.9 === .NET Core 3.1 SDK === SDK: 3.1.405 === Xamarin.Profiler === Version: 1.6.15.68 Location: /Applications/Xamarin Profiler.app/Contents/MacOS/Xamarin Profiler === Updater === Version: 11 === Apple Developer Tools === Xcode 12.2 (17535) Build 12B45b === Xamarin.Mac === Xamarin.Mac not installed. Can't find /Library/Frameworks/Xamarin.Mac.framework/Versions/Current/Version. === Xamarin.iOS === Version: 14.9.0.27 (Visual Studio Community) Hash: f4c9327fa Branch: main Build date: 2020-11-19 10:57:31-0500 === Xamarin.Android === Version: 11.2.0.0 (Visual Studio Community) Commit: xamarin-android/d16-9/f908d16 Android SDK: /Users/tommikiviniemi/Library/Developer/Xamarin/android-sdk-macosx Supported Android versions: None installed SDK Tools Version: 26.1.1 SDK Platform Tools Version: 30.0.4 SDK Build Tools Version: 30.0.2 Build Information: Mono: 5e9cb6d Java.Interop: xamarin/java.interop/d16-9@1d382be ProGuard: Guardsquare/proguard/v7.0.1@912d149 SQLite: xamarin/sqlite/3.32.2@cfe06e0 Xamarin.Android Tools: xamarin/xamarin-android-tools/main@ad80a42 === Microsoft OpenJDK for Mobile === Java SDK: /Users/tommikiviniemi/Library/Developer/Xamarin/jdk/microsoft_dist_openjdk_1.8.0.25 1.8.0-25 Android Designer EPL code available here: https://github.com/xamarin/AndroidDesigner.EPL === Android SDK Manager === Version: 16.9.0.21 Hash: 57e40ba Branch: remotes/origin/main Build date: 2021-01-08 01:57:14 UTC === Android Device Manager === Version: 16.9.0.14 Hash: 0fdccda Branch: remotes/origin/main Build date: 2021-01-08 01:57:36 UTC === Build Information === Release ID: 809001451 Git revision: cfd15313a6388ef8dada0182e22a058131c46f9d Build date: 2021-01-15 08:42:21-05 Build branch: release-8.9 Xamarin extensions: cfd15313a6388ef8dada0182e22a058131c46f9d === Operating System === Mac OS X 10.16.0 Darwin 20.2.0 Darwin Kernel Version 20.2.0 Wed Dec 2 20:39:59 PST 2020 root:xnu-7195.60.75~1/RELEASE_X86_64 x86_64 ```

Workaround

See the post for a timing-dependent workaround, but it's not a good fix because: 1) It's not performant as the CollectionView has to scroll through the entire list to reach the bottom (so the CollectionView has to create entries and visualise every single element). 2) It is completely timing dependent (presumably affected by device speed, amount of elements, and complexity of visualisation).

So a real fix is pretty much required.

Tommigun1980 commented 3 years ago

Any chance to get the CollectionView fixed so that it can scroll to the end of a list (without first having to scroll through all the items to get there)?

I am 100% dependent on getting this to work, and I think being able to scroll a list is an absolutely mandatory feature. Thanks for fixing this!

Tommigun1980 commented 3 years ago

@jsuarezruiz @hartez I'd really appreciate some correspondence on these four CollectionView bugs I have opened: https://github.com/xamarin/Xamarin.Forms/issues/13592, https://github.com/xamarin/Xamarin.Forms/issues/13547, https://github.com/xamarin/Xamarin.Forms/issues/13548, https://github.com/xamarin/Xamarin.Forms/issues/13231.

I spent a ton of time isolating the issues and making repro projects for them, and I need some kind of resolution as I have a hard time finish up my work here before they are resolved. So I'd really appreciate some comments on time estimates, or any kind of correspondence regarding them. Thank you.

jsuarezruiz commented 3 years ago

Attached a sample: CollectionViewIssues.zip

I have modified #13231 sample adding a Button to scroll to the end. This sample use the latest Xamarin.Forms 5.0 Nuget Package and the scroll seems to work as expected: issue-cv-scroll

@Tommigun1980 I am missing something? or maybe have been fixed by latest @hartez PRs?

Tommigun1980 commented 3 years ago

Attached a sample: CollectionViewIssues.zip

I have modified #13231 sample adding a Button to scroll to the end. This sample use the latest Xamarin.Forms 5.0 Nuget Package and the scroll seems to work as expected: issue-cv-scroll

@Tommigun1980 I am missing something? or maybe have been fixed by latest @hartez PRs?

Hi.

The CollectionView should be scrolled to bottom after data fills up (think a chat app - you see the most recent messages at the bottom when it loads). This doesn’t work.

The sample uses ItemsUpdatingScrollMode="KeepLastItemInView". So there should be no need for a scroll to bottom button. It should show the last messages after it loads the contents.

Tommigun1980 commented 3 years ago

Accidentally clicked close issue so reopening.

@hartez So I want to clarify that most of the workarounds in this ticket are needed to scroll to the bottom only when initially filling up the data.

The repro project does use ItemsUpdatingScrollMode="KeepLastItemInView" but it doesn’t scroll to the bottom, as evident by seeing the top of the list. So the hacks detailed in here forcefully scroll it to bottom, but for some reason these calls (like ScrollTo()) don’t work properly so even more workarounds are needed.

AnthonyIacono commented 3 years ago

@Tommigun1980 I have a work around for detecting the last visible item on demand rather than waiting for a scrolled event. Beware, it is pretty gross and can probably be cleaned up.

First, create a class (for example AppCollectionView) that extends CollectionView.

public class AppCollectionView : CollectionView
{
    public Func<int> GetLastVisibleIndexFunc { get; set; } = () => -1;
}

Introduce a custom renderer on iOS that does this:

[assembly: ExportRenderer(typeof(AppCollectionView), typeof(AppCollectionViewRenderer))]

public class AppCollectionViewRenderer : CollectionViewRenderer
{
    protected override void OnElementChanged(ElementChangedEventArgs<GroupableItemsView> e)
    {
        base.OnElementChanged(e);

        if (e.NewElement is AppCollectionView appCollectionView)
        {
            appCollectionView.GetLastVisibleIndexFunc = () =>
            {
                if (!(this.ViewController is UICollectionViewController vc))
                {
                    return -1;
                }

                var collectionView = vc.CollectionView;

                var indexPathsForVisibleItems = collectionView.IndexPathsForVisibleItems.OrderBy(x => x.Row).ToList();

                if (indexPathsForVisibleItems.Count == 0)
                {
                    return -1;
                }

                return (int)indexPathsForVisibleItems.Last().Item;
            };
        }
    }
}

Start using AppCollectionView in your XAML/code behind instead. Now you can call CollectionView.GetLastVisibleIndexFunc() at any point to get the last visible item index instead of waiting for an event to (maybe) fire.

Let me know if this helps you get to a slightly less hacky solution. I am working on a very similar problem for a chat app.

AnthonyIacono commented 3 years ago

Just made an Android custom renderer to do the same thing:

[assembly: ExportRenderer(typeof(AppCollectionView), typeof(AppCollectionViewRenderer))]

public class AppCollectionViewRenderer : CollectionViewRenderer
{
    /// <summary>
    /// Initializes a new instance of the <see cref="AppCollectionViewRenderer"/> class.
    /// </summary>
    /// <param name="context">An instance of <see cref="Context"/>.</param>
    public AppCollectionViewRenderer(Context context)
        : base(context)
    {
    }

    protected override void OnElementChanged(ElementChangedEventArgs<ItemsView> elementChangedEvent)
    {
        base.OnElementChanged(elementChangedEvent);

        if (elementChangedEvent.NewElement is AppCollectionView appCollectionView)
        {
            appCollectionView.GetLastVisibleIndexFunc = () =>
            {
                if (this.GetLayoutManager() is LinearLayoutManager linearLayoutManager)
                {
                    return linearLayoutManager.FindLastVisibleItemPosition();
                }

                return -1;
            };
        }
    }
}

Now the following code works on both platforms:

                        var lastItemIndex = this.ChatItemCount - 1;
                        var scrollComplete = this.CollectionView.GetLastVisibleIndexFunc() == lastItemIndex;

                        while (!scrollComplete)
                        {
                            await Task.Delay(250);
                            this.CollectionView.ScrollTo(lastItemIndex, position: ScrollToPosition.End, animate: true);
                            scrollComplete = this.CollectionView.GetLastVisibleIndexFunc() == lastItemIndex;
                        }

Last thing: I noticed animate: false works but it kind of makes items render strangely when you scroll back up. For me, I'm okay with animate: true since I'm gonna hide the collection view with a view rendering on top of it until I detect I am fully scrolled down.

Tommigun1980 commented 3 years ago

Thank you @AnthonyIacono.

@hartez @jsuarezruiz Do you guys think that scrolling a CollectionView to bottom could be fixed so it would work out of the box? I mean it's one of the most important features in any kind of list view and I'd really appreciated if it could be fixed. Thank you.

Tommigun1980 commented 3 years ago

@hartez @jsuarezruiz Hi. I don't think I can finish my application unless the CollectionView can be fixed so that it can be loaded in a state where the last items are visible. The 'KeepLastItemInView' setting does not work when the data is populated, which makes it almost impossible to do a chat feature with the CollectionView. Scrolling is an important concept and I don't think one can make apps without it working. Any chance this could get fixed? Let me know if something is not clear in the repro project. Thank you.

coen22 commented 3 years ago

Thank you for those who provided a workaround. Although on Android it only scrolls to the third to last element.

But I've got another problem too. The cells that I use have some more components for the purpose of styling. Scrolling performance is therefore really jittery (on Android, iOS is fine). Is there a workaround for that too?

        public void Init(ImageSource imageSource, DateTime time, bool isMine = true) {
            BindingContext = this;

            if (imageSource == null)
                imageSource = StubContent.ProfilePicture;

            var formattedString = new FormattedString();

            var messageSpan = new Span {
                FontSize = Device.GetNamedSize(NamedSize.Medium, typeof(Label)),
                TextColor = (isMine) ? Color.White : Color.FromHex("#111")
            };
            messageSpan.BindingContext = this;
            messageSpan.SetBinding(Span.TextProperty, "Text");
            formattedString.Spans.Add(messageSpan);

            var timeSpan = new Span {
                Text = " " + time.ToShortTimeString(),
                FontSize = Device.GetNamedSize(NamedSize.Small, typeof(Label)),
                TextColor = (isMine) ? Color.White.MultiplyAlpha(0.5) : Color.FromHex("#111").MultiplyAlpha(0.5),
                LineHeight = 1.5
            };
            formattedString.Spans.Add(timeSpan);

            var messageText = new Label {
                HorizontalOptions = LayoutOptions.Start,
                FormattedText = formattedString
            };

            var bubble = new RoundFrame {
                HasShadow = false,
                CornerRadius = 20,
                Padding = 15,
                HorizontalOptions = (isMine) ? LayoutOptions.EndAndExpand : LayoutOptions.StartAndExpand,
                VerticalOptions = LayoutOptions.StartAndExpand,
                BackgroundColor = (isMine) ? Color.FromHex("#F44336") : Color.White,
                Content = messageText,
                Margin = new Thickness((isMine) ? 32 : 0, 0, (isMine) ? 0 : 32, 0)
            };

            Content = new StackLayout() {
                Orientation = StackOrientation.Horizontal,
                VerticalOptions = LayoutOptions.Fill
            }.AddContent(new RoundFrame {
                CornerRadius = 100,
                HasShadow = false,
                MinimumWidthRequest = 32,
                WidthRequest = 32,
                HeightRequest = 32,
                Padding = 0,
                HorizontalOptions = LayoutOptions.Start,
                VerticalOptions = LayoutOptions.Center,
                BackgroundColor = Color.Transparent,
                Content = (!isMine) ? new CachedImage {
                    FadeAnimationEnabled = true,
                    BitmapOptimizations = true,
                    DownsampleToViewSize = true,
                    Source = imageSource,
                    Aspect = Aspect.AspectFill,
                } : null
            }, bubble);

            TouchEffect.SetNativeAnimation(bubble, true);
            if (isMine)
                TouchEffect.SetNativeAnimationColor(bubble, Color.White);
        }
trampster commented 3 years ago

We have an app that does messaging that has this bug, the work around it not great because it requires slowing scrolling through all the messages to get to the end. Which is to slow if there are a large number of messages.

We have implemented the work around from above but with a few changes:

Tommigun1980 commented 3 years ago

@hartez @jsuarezruiz Pinging about this yet again. The CollectionView can't be scrolled to bottom when data is loaded so it's not really usable atm. Is there any chance you guys could fix this? This is an absolute blocker for me and probably quite a lot of other people. Scrolling a CollectionView is a core feature.

Thank you!

developer9969 commented 3 years ago

@Hartez hi is the intention not to fix the collection view till maui is out? We are really struggling and we have to do all sorts of workaround to make it sort of work.

I can see that this release was just done to keep people quiet but to be honest the fixes that mattered were not there. The collectionview together with grid are the most important controls and needs urgent patch fixes.

Maui will suffer from the same problems no?

Can someone who calls the shot reply?

unamed000 commented 3 years ago

Hi @Tommigun1980

How do you handle the keyboard appear and load more when scrolling top on your side?

On my side, i wrote some custom function like InsertRange like this:

/// <summary>
        /// Add multiple items and notify to the layout only once
        /// </summary>
        /// <param name="itemsToAdd"></param>
        /// <exception cref="ArgumentNullException"></exception>
        public void InsertRange( int index, IEnumerable<T> itemsToAdd)
        {
            if (itemsToAdd == null) {
                throw new ArgumentNullException(nameof(itemsToAdd));
            }

            lock (Items) {
                CheckReentrancy();

                int startIndex = index;
                var changedItems = itemsToAdd is List<T> ? (List<T>)itemsToAdd : new List<T>(itemsToAdd);

                foreach (var item in changedItems) {
                    Items.Insert(index++, item);
                }

                OnPropertyChanged(new PropertyChangedEventArgs(nameof(Count)));
                OnPropertyChanged(new PropertyChangedEventArgs("Item[]"));
                OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, changedItems, startIndex));
            }
        } // end AddRange

But when using together with KeepLastItemInView, the list will always jump to the end when it called ...

Also, when the keyboard is appeared, the scrolling position is not kept anymore. Which make me have to do a hack that it will wait for the keyboard appear (changing the bottom margin) and scroll to the bottom, which is not very nice. =_=

And yes, there are a lot of scrolling issues in the CollectionView, i'm having this when making may chat app as well!

aaronsmithuk commented 3 years ago

Slightly off topic, but if anyone is desperate for a solution that simply works, try SfListView, it has a Loaded event that will execute ScrollToRowIndex perfectly. Commercial, but it does work and is a drop-in replacement.