microsoft / microsoft-ui-xaml

Windows UI Library: the latest Windows 10 native controls and Fluent styles for your applications
MIT License
6.31k stars 676 forks source link

ItemsRepeater inside a ScrollViewer with odd behaviour when it changes size #9584

Closed torleifat closed 2 months ago

torleifat commented 5 months ago

Describe the bug

So I encountered some odd behavior when modifying the visibility of items in a ItemsRepeater that was inside a ScrollViewer: When the number of items exceeded a certain threshold I could no longer scroll properly without the ScrollViewer becoming jittery after I had collapsed an item.

Example: 1726_2

After some investigation I observed that after an item had been collapsed the SizeChanged event on the ItemsRepeater kept being called whenever I scrolled. I expected that SizeChanged would be called once when I collapsed the item, but it kept being called and the Scrollbar on the ScrollViewer kept changing size. Also, in some cases I could no longer scroll to the end of the ScrollViewer.

So the workaround I used was to increase the HorizontalCacheLength of the ItemsRepeater. For the purposes of my application that should work out fine.

More Generally I don't think this an issue specifically with collapsing an item, but rather the change in size of the ItemRepeater's items or when items have a non-uniform size. I guess there is a cache limit at play here given increasing the cache length helps, but I still think that having a too low cache limit should not result in scrolling leading to the ItemRepeater resizing over and over again.

Edit: After fiddling some more with this I noticed that it is enough that one of Items have initially a different size to make it trigger resizes for ItemsRepeater.

Steps to reproduce the bug

There are probably easier ways of reproducing this, but here's what I did:

Here's the full code for the page I used to test it and generated the gifs underneath with: MainPage.xaml

<?xml version="1.0" encoding="utf-8"?>
<Page
    x:Class="ScrollViewerAndItemsRepeaterIssue.MainPage"
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:local="using:ScrollViewerAndItemsRepeaterIssue"
    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
    mc:Ignorable="d" Background="Aquamarine">

    <Page.Resources>
        <ResourceDictionary>
            <StackLayout x:Name="HorizontalStackLayout" Orientation="Horizontal" Spacing="0"/>
            <NonVirtualizingLayout x:Name="nvl" ></NonVirtualizingLayout>

            <DataTemplate x:Key="ItemTemplate" x:DataType="x:String">
                <Border Background="Red" BorderBrush="Black" BorderThickness="1" Width="64" Height="64" Tapped="ItemTapped">
                    <TextBlock FontSize="38" HorizontalAlignment="Center" VerticalAlignment="Center" Text="{x:Bind}"></TextBlock>
                </Border>
            </DataTemplate>

        </ResourceDictionary>
    </Page.Resources>

    <StackPanel HorizontalAlignment="Center" VerticalAlignment="Center">
        <ScrollViewer Height="64" Width="300" Background="Green" HorizontalScrollMode="Enabled" HorizontalScrollBarVisibility="Visible">
            <ItemsRepeater x:Name="Repeater" Layout="{StaticResource HorizontalStackLayout}" SizeChanged="RepeaterSizeChanged" ItemsSource="{x:Bind Items}" ItemTemplate="{StaticResource ItemTemplate}"/>
        </ScrollViewer>
        <StackPanel Orientation="Horizontal" HorizontalAlignment="Center">
            <TextBlock Text="ItemRepeater SizeChanged event count: "/>
            <TextBlock x:Name="RepaterSizeChangeCounter"/>
        </StackPanel>

    </StackPanel>
</Page>

MainPage.idl

namespace ScrollViewerAndItemsRepeaterIssue
{
    [default_interface]
    runtimeclass MainPage : Microsoft.UI.Xaml.Controls.Page
    {
        MainPage();
        Windows.Foundation.Collections.IObservableVector<String> Items{ get; };
    }
}

MainPage.h

#pragma once

#include "MainPage.g.h"

namespace winrt::ScrollViewerAndItemsRepeaterIssue::implementation
{
    struct MainPage : MainPageT<MainPage>
    {
        MainPage();

        void ItemTapped(winrt::Windows::Foundation::IInspectable const& sender, winrt::Microsoft::UI::Xaml::Input::TappedRoutedEventArgs const& e);

        winrt::Windows::Foundation::Collections::IObservableVector<hstring> Items();

        winrt::Windows::Foundation::Collections::IObservableVector<hstring> m_Items;
        void RepeaterSizeChanged(winrt::Windows::Foundation::IInspectable const& sender, winrt::Microsoft::UI::Xaml::SizeChangedEventArgs const& e);
    };
}

namespace winrt::ScrollViewerAndItemsRepeaterIssue::factory_implementation
{
    struct MainPage : MainPageT<MainPage, implementation::MainPage>{};
}

MainPage.cpp

#include "pch.h"
#include "MainPage.xaml.h"
#if __has_include("MainPage.g.cpp")
#include "MainPage.g.cpp"
#endif

using namespace winrt;
using namespace Microsoft::UI::Xaml;

namespace winrt::ScrollViewerAndItemsRepeaterIssue::implementation
{
    MainPage::MainPage()
    {
        m_Items = winrt::single_threaded_observable_vector<hstring>();
        for (int i = 0; i < 20; i++)
        {
            m_Items.Append(L"☺️");
        }
    }

    winrt::Windows::Foundation::Collections::IObservableVector<hstring> MainPage::Items()
    {
        return m_Items;
    }

    void MainPage::ItemTapped(winrt::Windows::Foundation::IInspectable const& sender, winrt::Microsoft::UI::Xaml::Input::TappedRoutedEventArgs const& e)
    {
        auto item = sender.as<winrt::Microsoft::UI::Xaml::Controls::Border>();
        item.Visibility(winrt::Microsoft::UI::Xaml::Visibility::Collapsed);
    }

    void MainPage::RepeaterSizeChanged(winrt::Windows::Foundation::IInspectable const& sender, winrt::Microsoft::UI::Xaml::SizeChangedEventArgs const& e)
    {
        static int32_t SizeCounter = 1;
        RepaterSizeChangeCounter().Text(std::to_wstring(SizeCounter++).c_str());
    }
}

Expected behavior

That the ScrollViewer would be able to figure out how much it can scroll.

Screenshots

Some gifs of the behavior that I am seeing (note the SizeChanged counter under the ItemsRepeater that increments each time it is called).

Here is normal behavior, no items having been collapsed 1726_1

Here I collapse one of the items. Notice how the Scrollbar's size fluctuates: 1726_2

Here I try to drag quickly to the end of the ScrollView, but it is struggling to reach the end. 1726_3

Here it is completely stuck and cannot scroll to the end. 1726_4

NuGet package version

WinUI 3 - Windows App SDK 1.5.2: 1.5.240404000

Windows version

Windows 11 (22H2): Build 22621

Additional context

No response

github-actions[bot] commented 5 months ago

Hi I'm an AI powered bot that finds similar issues based off the issue title.

Please view the issues below to see if they solve your problem, and if the issue describes your problem please consider closing this one. Thank you!

Open similar issues:

Closed similar issues:

Note: You can give me feedback by thumbs upping or thumbs downing this comment.

codendone commented 5 months ago

Some of this looks like a duplicate of #9308, but that might not cover all of the odd behaviors reported here.

torleifat commented 5 months ago

Yeah I think it's probably related. But I thought the way scrolling after the ItemsRepeater has changed size once results in more SizeChanged calls was curious enough to warrant a report.

kaismic commented 4 months ago

I have the same issue here and can't scroll to the end. Are there any possible workarounds on this issue?

https://github.com/microsoft/microsoft-ui-xaml/assets/72960101/ffdb56d2-b36c-42d5-a6ca-3561066327e4

torleifat commented 4 months ago

I have the same issue here and can't scroll to the end. Are there any possible workarounds on this issue?

scrollviewer_bug1.mp4

So the work-around we used was increasing the HorizontalCacheLength property of the ItemsRepeater. So you can try increase that. There's possibly some performance hit there, but for our application it was an acceptable tradeoff.

RBrid commented 2 months ago

@torleifat, my comments in https://github.com/microsoft/microsoft-ui-xaml/issues/9775 apply here too.

"When the number of items exceeded a certain threshold I could no longer scroll properly without the ScrollViewer becoming jittery after I had collapsed an item." That's expected because you're using the virtualizing StackLayout. It looks at the realized items sizes and extrapolates to figure the total extent of the ItemsRepeater. As you scroll, the average realized item size changes, and the approximated ItemsRepeater size changes too. Thus the ScrollBar has to jump.

"After some investigation I observed that after an item had been collapsed the SizeChanged event on the ItemsRepeater kept being called whenever I scrolled. I expected that SizeChanged would be called once when I collapsed the item, but it kept being called and the Scrollbar on the ScrollViewer kept changing size." Yes - my comments above should explain this now.

"Also, in some cases I could no longer scroll to the end of the ScrollViewer." I'd suggest you apply my recommendations in issue #9775. You should then no longer run into these problems.

"So the workaround I used was to increase the HorizontalCacheLength of the ItemsRepeater. For the purposes of my application that should work out fine." Correct - by increasing the HorizontalCacheLength value you get rid of the virtualization aspect of the StackLayout. So all items become realized and there's no more guessing about what the total extent is.

"More Generally I don't think this an issue specifically with collapsing an item, but rather the change in size of the ItemRepeater's items or when items have a non-uniform size. I guess there is a cache limit at play here given increasing the cache length helps," You are correct. The item size variability and item virtualization combined result in your observations. But remember that using the ItemsRepeater.ElementPrepared (as shown in #9775) addresses is a strict minimum, and modifying the ItemsSource is the best approach.

" but I still think that having a too low cache limit should not result in scrolling leading to the ItemRepeater resizing over and over again." That's by design for the StackLayout, as explained above.

Hope this helps. Thanks, -Régis.