AvaloniaUI / Avalonia.Controls.ItemsRepeater

ItemsRepeater is a light-weight control to generate and present a collection of items.
MIT License
2 stars 1 forks source link

Nested ItemsRepeater issues #5

Open amwx opened 3 years ago

amwx commented 3 years ago

Describe the bug When nesting ItemsRepeaters, a couple things... 1- When scrolling, primarily with the scrollbar thumb, the garbage collector runs almost constantly. I can't tell if this occurs in WinUI since that mostly lives in native memory

2- When scrolling, especially large jumps quickly, the scrollviewer will freeze. Resizing a parent container (like the window) will fix it. I think this is related to the items of different height issue, but I haven't seen any reports of freezing yet. I also can't replicate this in WinUI, I can get choppy scrolling but it never fully freezes. Sometimes it takes aggressive scrolling to cause this (like in the video I attached), and sometimes one large quick scroll will do it.

(see attached video for demo of 1 & 2)

3- There may or may not be a memory leak. I do think this also occurs upstream in WinUI, as process memory slowly climbs when scrolling. When first loaded, 10 ListBoxItems are present Scroll up and down the list once: 179 ListBoxItems are present Scroll randomly: 301 Scrolling some more: 438

To Reproduce Example code: Group installed fonts alphabetically and display using nested ItemsRepeaters

public class ViewModel
{
        public ViewModel()
        {
            Items = new List<string>(FontManager.Current.GetInstalledFontFamilyNames());
            var query = from item in Items
                        group item by item.Substring(0, 1).ToUpper() into g
                        orderby g.Key
                        select new GroupInfo(g) { Key = g.Key };
            Groups = new List<GroupInfo>(query);
        }

        public List<string> Items { get; }
        public List<GroupInfo> Groups { get; }
}

public class GroupInfo : List<string>
{
        public string Key { get; set; }
        public GroupInfo(IEnumerable<string> items) : base(items) { }
}

Window Content:

<Panel>
      <Border Width="350" Height="400"
              BorderBrush="{DynamicResource SystemControlHighlightBaseLowBrush}"
              BorderThickness="1">
          <ScrollViewer HorizontalScrollBarVisibility="Disabled">
              <ItemsRepeater Items="{Binding Groups}">
                  <ItemsRepeater.ItemTemplate>
                      <DataTemplate>
                          <StackPanel>
                              <Border Height="40">
                                  <TextBlock Text="{Binding Key}"
                                             VerticalAlignment="Center"
                                             Margin="12 0"
                                             FontWeight="Bold"/>
                              </Border>
                              <ItemsRepeater Items="{Binding}">
                                  <ItemsRepeater.ItemTemplate>
                                      <DataTemplate>
                                          <ListBoxItem Height="40" Content="{Binding}" />
                                      </DataTemplate>
                                  </ItemsRepeater.ItemTemplate>
                              </ItemsRepeater>
                          </StackPanel>
                      </DataTemplate>
                  </ItemsRepeater.ItemTemplate>
              </ItemsRepeater>
          </ScrollViewer>
      </Border>
  </Panel>

Expected behavior A clear and concise description of what you expected to happen.

Screenshots https://user-images.githubusercontent.com/40413319/111887602-0ebf6c00-89a4-11eb-80c9-889c42d62ce5.mp4

Desktop (please complete the following information):

maxkatz6 commented 3 years ago

Nested ItemsRepeater doesn't look like a correct design. I also don't think it will work with virtualization, so you probably should disable it on StackLayout.

I would use mixed items in the source like new [] { groupHeader1, item1, item2, groupHeader2, item3, item4 }, and set different templates for headers and items with custom template factory. It will be simpler solution.

Better solution might be creating Layout, that knows about headers, and will arrange headers and items correctly. Not sure how easy it is, and if it's even possible, but it will allow setting up custom layout placement for headers and items (important for grid-like layouts).

jp2masa commented 3 years ago

Nesting ItemsRepeaters is actually recommended in the UWP docs: https://docs.microsoft.com/en-us/windows/uwp/design/controls-and-patterns/items-repeater#display-grouped-items.

maxkatz6 commented 3 years ago

@jp2masa interesting, I hope it's really supported by design. Although as the example they used horizontal lists inside of the group.

amwx commented 3 years ago

The WinUI test app has an example of vertical stack within vertical stack, and the NavigationView also uses this for hierarchical items (tho virtualization is disabled in the NavView for some unrelated bug)

image

robloo commented 3 years ago

Nesting ItemsRepeaters is actually recommended in the UWP docs: https://docs.microsoft.com/en-us/windows/uwp/design/controls-and-patterns/items-repeater#display-grouped-items.

@jp2masa interesting, I hope it's really supported by design. Although as the example they used horizontal lists inside of the group.

Nesting ItemsRepeater MUST be supported for future "ItemsRepeater as ItemsPresenter" Avalonia plans. Nesting arbitrary controls is also a pretty fundamental XAML concept.

That said, I've seen nothing but issues with the edge cases using ItemsRepeater. I'm not even sure the fundamental design works in all theoretical edge cases. However, it seems like it should and it came from some smart minds at Microsoft. I guess it just doesn't have the mileage yet to have solved all the problems.