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

Items vs ItemsSource #7553

Closed
AndrejBunjac opened this issue Feb 7, 2022 · 7 comments · Fixed by #10590
Closed

Items vs ItemsSource #7553

AndrejBunjac opened this issue Feb 7, 2022 · 7 comments · Fixed by #10590

Comments

@AndrejBunjac
Copy link
Contributor

Hi!
I would like to ask can you consider naming of some properties to match their more common use cases in WPF.
An example for this is Items vs ItemsSource.

In my humble opinion, it would make more sense to have your Items property named ItemsSource simply because the way one uses it in binding more closely resembles how one uses ItemsSource in WPF. Normalizations like this to existing XAML standards might make it easier for people to port apps from WPF to Avalonia, especially with the prospect of automating such transitions where possible (and it seems to be possible for a lot of layout related stuff).

I am unaware if there is a specific reason why this isn't the case and if it was brought up somewhere, can you please direct me to it so I can read?

Thank you!
Andrej

@robloo
Copy link
Contributor

robloo commented Feb 8, 2022

Having only a single Items property is intentional in Avalonia. It was originally considered somewhat confusing that there are two properties for this in WPF. The docs really need to expand on this though: https://docs.avaloniaui.net/misc/wpf/itemscontrol

That said, I've seen other discussions where this is likely going to change:

@grokys
Copy link
Member

grokys commented Feb 8, 2022

Yeah this is something I decided to do back at the beginning of the project when I didn't expect anyone to be using it ;)

In hindsight it was a mistake, not just because of porting issues, but also for performance reasons.

Unfortunately changing the name of the property at this point would break pretty much everyone who uses Avalonia. One solution might be to add ItemsSource and make it do exactly the same as Items, but I'm not sure if that might cause other problems. If we're going to do this we should probably decide to do it for the next major version this year.

@robloo
Copy link
Contributor

robloo commented Feb 8, 2022

If we're going to do this we should probably decide to do it for the next major version this year.

If that means "do it now or never" then my vote is change it now. I think there are enough cases you have discovered to justify it. It also helps the compatibility story as mentioned here.

@Splitwirez
Copy link
Contributor

If it's hurting performance, then I'm...hesitantly in agreement with @robloo on this. Not going to pretend I see the whole picture though...but does anyone, really?

@myplaths
Copy link

I think even though its called avalonia if they made the same name of the properties then people wouldn't have to ask so many new questions about stuff.

@grokys
Copy link
Member

grokys commented Nov 22, 2022

@kekekeks asked me in chat "how exactly is the Items property hurting performance?" and I think it'd be a good idea to explain here:

In WPF, there are two items container properties, with only one of them being settable at any time:

  • Items which contains a list of inline item containers
  • ItemsSource which points to a collection from which item containers will be created

In Avalonia the Items collection can be both: it can contain both inline item containers and items from which item containers will be created.

The thing that hurts performance is the fact that inline items need to be part of the logical tree even before the ItemsControl is added to the visual tree. To give an example:

<TabControl>
  <TabItem Header="Empty/>
  <TabItem Header="Items">
    <ListBox>
      <ListBoxItem/>
    </ListBox>
  </TabItem>
</TabControl>

When the "Empty" tab is open here, the ListBox (and by extension ListBoxItem) isn't part of the visual tree, but the ListBoxItem still needs to be visible in the logical tree.

What this means is that every time an item is added to ItemsControl.Items we need to check if the item is a control and if so add it to the logical tree.

If we had a separate Items and ItemsSource property this wouldn't be necessary: anything added to Items would get immediately added to the logical tree, anything added to ItemsSource would only cause a control to be added to the logical tree when a container gets created for the item.

@grokys grokys added this to ItemsControl Refactor in @grokys todos Dec 2, 2022
@grokys
Copy link
Member

grokys commented Feb 16, 2023

I've looked into doing this, but I think we need to do #9944 first, as that's going to affect how this is implemented.

@grokys todos automation moved this from ItemsControl to Done Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
@grokys todos
  
Done
Development

Successfully merging a pull request may close this issue.

5 participants