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

[Enhancement] Bindable Repeater control #1718

Closed hartez closed 6 years ago

hartez commented 6 years ago

Rationale

Forms does not currently contain a control which displays items from a bound items source in a non-scrollable container.

Implementation

Attached properties targeting Layout will be added to support an ItemsSource and ItemsTemplate. Setting these properties will add the templated items to the target layout.

public static class BindableLayout
{
    public static readonly BindableProperty ItemsSourceProperty =
        BindableProperty.Create("ItemsSource", typeof(IList), typeof(Layout), default(IList));

    public static readonly BindableProperty ItemTemplateProperty =
        BindableProperty.Create("ItemTemplate", typeof(DataTemplate), typeof(Layout);
}

The ItemTemplate is responsible for converting the objects in the ItemsSource into View objects.

Setting the ItemsSource will clear out any existing children and update the Children collection to match the source.

Backward Compatibility

These are new properties, so backward compatibility is likely not an issue.

Difficulty: Moderate

bmacombe commented 6 years ago

I pretty much have this control implemented already as defined. It will likely just need some refinement, clean up and best practice review.

It supports DataTemplate or DataTemplateSelector. It can also handle a ItemSource which implements INotifyCollectionChanged to add/remove items as needed instead of rebuilding all the children

bmacombe commented 6 years ago

I've been looking at the Forms code base to see how to best modify my code for style, convention, etc.

I thought looking at ListView would be a good example, which lead me to ItemsView and TemplatedItemsList. Which I don't fully understand how they work yet. But my question is, does this feature need all that complication or simply add a new view to the stack layout for each item in the bound collection based on the DataTemplate?

My current solution does the simple method, which doesn't make it perform very well for large lists of items, which isn't my use case for it.

Thoughts?

GalaxiaGuy commented 6 years ago

There is an alternative route that could be taken.

I implemented ItemsSource and ItemTemplate as attached properties that could be applied to any Layout:

https://github.com/GalaxiaGuy/MobileLibraries/blob/master/XamarinForms.Layout/Layout.cs

Technically that version doesn't support DataTemplateSelector but I have implemented that in version elsewhere.

(It gets complicated after line 67 just to support INotifyCollectionChanged).

andreinitescu commented 6 years ago

I also think the idea of a separate "Repeater" control is not enough and we can have something better than that. "Repeater" is not the right solution.

bmacombe commented 6 years ago

I created my control because I had a need for a simple list of views that did not allow selection and at the time I couldn't find a way to keep ListView from doing selection.

The attached property is an interesting idea, but we probably need some feedback from the XF Team on that route.

adamped commented 6 years ago

I for one am for the original idea of expanding the StackLayout and adding the 2 properties. It is how it has been done in my previous projects. Keeps it simple.

The Repeater control is what we have been asking for, and many devs liked this approach, as per the evolution forum. If we need anything more than a Repeater, we use a ListView. This approach also gives us an awesome horizontal list :) If we need a horizontal listview, that is a different PR.

andreinitescu commented 6 years ago

@adamped Which exactly 2 properties are you suggesting to be added to ListView? Did you mean Layout? ListView already has ItemsSource and ItemTemplate.

adamped commented 6 years ago

Apologies, typo. I meant StackLayout :)

adamped commented 6 years ago

@davidortinau - FYI this one hasn't been assigned to you yet.

PureWeen commented 6 years ago

Here's the RepeaterView I've been using which is mostly the evolve one with a few more tweaks I've had to apply along the way

https://gist.github.com/PureWeen/4bf4d8c2e384a11b34398dc23201a7cc

It supports templating and has been working for me with uwp/ios/android

andreinitescu commented 6 years ago

@davidortinau We can have something better than just adding a control which is just derived from StackLayout. Otherwise, I don't think it makes sense to add a Repeater derived from StackLayout. At most, maybe add some attached properties to Layout, like how @GalaxiaGuy suggested above.

The way the XAML frameworks do this is by an ItemsControl. Here's my take on this https://github.com/andreinitescu/XFItemsControl/

Please look at the last section at the bottom, I'm sharing some issues\bugs in Xamarin Forms I've stumbled on.

adamped commented 6 years ago

I'm all for adding the 2 properties directly to the StackLayout, instead of a separate control, called repeater.

However the ItemsControl, while good, I think needs to be a further discussion or enhancement beyond this. A bindable StackLayout control is what people wanted. Its the basic simple solution to many issues.

Maybe ItemsControl should be a separate additional control, beyond this enhancement. Or a further enhancement to the StackLayout? Either way, that's my thoughts. Will see what the XF team decides.

andreinitescu commented 6 years ago

@adamped Adding the ItemsSource and ItemTemplate properties to StackLayout wouldn't be a good decision. Instead, maybe add them as attached properties, just to make everyone happy.

Otherwise, if these properties are added right to StackLayout, should them be added to other Layout classes as well? Why not also add them to Grid? How about AbsoluteLayout or RelativeLayout? Maybe add them to Layout or Layout<View>?

I personally like how Windows XAML does it. Panel class (which is Layout in Xamarin Forms) does not have ItemsSource and ItemTemplate. Panel has just one purpose: to position items. This makes it reusable and pluggable into other controls.

andreinitescu commented 6 years ago

Another thought: Like I mentioned in the readme of my project, Xamarin has a ItemsView which is close to what ItemsControl is in Windows XAML. I am sure there's a reason why Xamarin team chose to derive ListView from ItemsView.

adamped commented 6 years ago

Adding them to the StackLayout doesn't mean it is suited to other layout controls. How would it work with a Grid, there are multiple columns and rows. How would it even work with an Absolute or Relative Layout? I can't even see how that would work.

A StackLayout is designed for stacking one element after the other. That is why an ItemsSource works well, and it would make no sense for other layouts to have these options.

andreinitescu commented 6 years ago

How would it work with a Grid, there are multiple columns and rows. How would it even work with an Absolute or Relative Layout? I can't even see how that would work.

It would. Google a bit for things like 'itemscontrol with grid'. You will see old school stuff how ItemsControl in combination with different Panel derived classes can result into very interesting things.

This would allow having some really impressive stuff in Xamarin Forms especially considering it's cross-platform, code which otherwise would take a significant amount of time to write and maintain natively.

Additionally, in the future, someone could even write a ListView-like control purely in Xamarin Forms. Imagine you have a control which takes any Layout which allows any item positioning, but also have properties like SelectedItem to be able to select items (ListView in Windows XAML derives from ItemsControl)

adamped commented 6 years ago

The problem with the Grid is due to performance, and then having to setup additional properties to control the flow of the items being added. You will see from Googling a bit about ItemsSource and a Grid in Xamarin.Forms, there are many solutions, including one of mine where I did this. It isn't the best solution for a Grid.

StackLayout is the only control that makes sense with just these 2 properties. Expanding it to other controls, or making this Issue into something more complex than what was asked for, should be done in a separate issue. People experiencing this issue only want a StackLayout with an ItemsSource.

While I know others would certainly like an ItemsControl and the solution you provided, I believe it to be a completely different control, for a different set of requirements. I'm not asking and neither are many others, for an ItemsControl. All I have needed for my last several projects, is just an ItemsSource and DataTemplate on a StackLayout.

andreinitescu commented 6 years ago

People experiencing this issue only want a StackLayout with an ItemsSource.

Yes, people experience it, including myself, many times, but that doesn't mean adding the properties right to StackLayout is the good solution from a framework perspective. Things look differently from a broader perspective, otherwise if we start adding properties just to suit our immediate needs, we will end up with clutter, with little or no reusability. At least this is my personal opinion.

Another aspect is convergence to a unified XAML (XAML Standard). I'm pretty sure those properties don't make sense on StackLayout, you just need to look at Windows XAML.

StackLayout is the only control that makes sense with just these 2 properties.

To me this sounds like a hasty conclusion. Yes, sure we all need a bindable items source sometimes. Why not use ListView then? Maybe a better decision would be to have a property on the ListView, something like IsSelectionEnabled? Better than creating a Repeater control or adding properties to StackLayout and it would mean we can still have virtualization.

adamped commented 6 years ago

XAML Standard raises a good point. That may be a good cause to go the ItemsControl. We will have to see what the XF stance is on this.

And the reason we don't use the ListView is because it implements scrolling. Otherwise we would just use a Listview. But I think that highlights you are looking to a solution to a different problem. We just want a bindable list that doesn't automatically include a scrollview.

andreinitescu commented 6 years ago

Adding a new Repeater control or just adding properties to StackLayout do not appeal to me. It would definitely solve the issue but no matter how 'obvious' these solutions are, from a framework perspective, there are better ways which will add more flexibility to the framework, so why not do it now? It won't be significantly more complicated to implement. After all, what is the problem with the ItemsControl I already implemented? I looked back to the comments and the only thing I could see was "People want bindable StackLayout". Sure, but that's because people don't know about the ItemsControl :)

adamped commented 6 years ago

I know? But you are just repeating things now. As mentioned, valid points raised, we will need to wait for the XF team to finalize anything.

bmacombe commented 6 years ago

There are good points being made both ways on this issue. I suggested we consider the following

  1. There is a need for simple list to display a small number of templated items, the "Repeater" as defined meets this. It will also be very low overhead and the name clearly implies that it is not a spophisticated control.

  2. We need a better "ItemsControl" "ListView", etc. But this is a much larger issue and has much broader impact, therefore will require a lot more thought and more input from the XF Team. I would love to have a performant list control that I could display 100s of templated items, but this should be a separate proposal. I suspect many of the issues we have with ListView are because it is implemented in the individual platform provided "ListViews". So a pure XF based control might be a good solution.

But the XF team has reviewed this request and if they posted the enhancement as a work item here, they seem to be willing to consider it. Let's not let great stand in the way of good. There is room for a "Repeater" and a "ItemsControl".

I also do not like the idea of adding these properties to the StackLayout, they belong in a subclass. Layouts need to be layouts, nothing more.

hartez commented 6 years ago

In response to some of the earlier comments - we're definitely not worried about virtualization or performance with very large ItemsSources here. This is explicitly for handling small lists of templated items.

Looking at Andrei's ItemsControl, there's a lot to like; it's more flexible than a simple Repeater, and it would be trivial to use it as a Repeater (especially since it defaults to using a StackLayout). It also has the advantage of being very familiar to the UWP/WPF devs.

TBH, if we were able to merge a PR for the ItemsControl in the near future, I'd happily close this Issue because I think ItemsControl solves the problems that Repeater was intended to solve. Is there a use case for Repeater that isn't covered by ItemsControl?

bmacombe commented 6 years ago

It would be trivial to change the repeater I was working on to submit for this issue to not inherit from StackPanel and use any provided layout (or default to StackLayout).

I personally like that solution better then the attached property idea that was suggested.

We could follow the simple approach of not worrying about virtualization or performance on large lists, but just support using any layout. The control could always be enhanced later to address large lists if needed.

adamped commented 6 years ago

@hartez - it does cover all scenario's we were interested in. Happy for this to be closed and replaced with the ItemsControl. Seems better to have 1 control that solves many problems at the same time.

@andreinitescu - are you happy to implement this in a PR, or did you want someone else to pick it up? Anything listed with an F100 tag is part of a community sprint we are all trying to finish in the next 2 weeks.

fyi - @davidortinau - in case reassignment is needed.

andreinitescu commented 6 years ago

@hartez I created a ticket for ItemsControl here #1743 @adamped We need to decide first how it needs to work exactly.

Need as much feedback as possible from everyone.

jassmith commented 6 years ago

This is really good feedback, we are going to internalize it and come back with a new proposal.

andreinitescu commented 6 years ago

Can we move this one from "In Progress" ? I suggest having a column called "Under discussion" or something with issues which are under discussion by the team. Otherwise things fall through the cracks for ever like on Bugzilla.

velocitysystems commented 6 years ago

@hartez @adamped We rolled BindableStackLayout a while back. Simple to use, works well on all the platforms and is in our 'core' suite of non-factory XF layout controls.

BindableStackLayout

opcodewriter commented 6 years ago

This is really good feedback, we are going to internalize it and come back with a new proposal.

@jassmith It's been 5 months, can I ask what's the team proposal?

bmacombe commented 6 years ago

@mattrichnz your BindableStackLayout is about exactly what I did and was planning to do for this feature until it got scoped creeped to the backlog :)

andreinitescu commented 6 years ago

@jassmith Any news? last time on January you said you're discussing it with your team

weitzhandler commented 6 years ago

IMHO, ListView should have been based on this control and inheriting from this (#1743) just like in UWP and WPF (inheriting from ItemsControl). This is probably the most important thing XF lacks, bringing frustration to top levels. Additionally ListView is made with such an tremendous sight trouble, that it doesn't support horizontal layout, selection fine-tuning, size to content, size to space and other major issues. See #1727, #2531, and #2723.

Anyway, the lack of ItemsControl and/or horizontal ListView is extremely painful! Xamarin has a quirky fixation to bias everything towards mobile and completely ignore anything else. Time has passed and we want to start using Xamarin for bigger screen too, and also for desktops. The lack of horizontal support, better keyboard and mouse control is totally annoying!

Additionally, talking about this, maybe it's time we get rid of cells and have content placed directly into DataTemplates like in WPF/UWP?

opcodewriter commented 6 years ago

pinging @jassmith again ... and more people @hartez @StephaneDelcroix 🤞 :)

velocitysystems commented 6 years ago

@weitzhandler https://github.com/velocitysystems/xf-controls/blob/master/XF.Controls/XF.Controls/Layouts/BindableStackLayout.cs

weitzhandler commented 6 years ago

@mattrichnz I know there are plenty of controls out there and I also have my own, the issue is that none of them has official support and proper testing and you spend days finding the proper one until you realize making your own is best and most reliable. This's gotta stop. It ain't PHP wild west, it's tha .NET here. It's the Microsoft league. These things have to be in-the-box.

JuanuMusic commented 6 years ago

Would love to have bindable items on a StackLayout, or at least a repeater view. Upvoting for this Enhancement!

opcodewriter commented 6 years ago

This is really good feedback, we are going to internalize it and come back with a new proposal.

Hi @jassmith It's been almost 6 months now... Could you please share some thoughts? Xamarin Forms desperately needs some alternative to ListView...

Thanks.

Really nobody from Xamarin Forms gets a notification on this? @hartez @StephaneDelcroix @rmarinho @samhouts ?

jassmith commented 6 years ago

We want to see where ListView2 lands before pushing this spec forward. It contains many of the same needed components and it would be best to avoid duplicating them.

opcodewriter commented 6 years ago

What's "ListView2" ?

jassmith commented 6 years ago

Something we'll be showing off in more detail soon.

opcodewriter commented 6 years ago

OK, thanks, hopefully we'll have something working by the end of 2018 :)

dansiegel commented 6 years ago

@jassmith please tell me it won't be called ListView2...

jassmith commented 6 years ago

It wont be called ListView2

opcodewriter commented 6 years ago

I know. It's going to be called either ListViewEx or TheRealListView 😆

weitzhandler commented 6 years ago

I hope it the concept of 'cell' will vanish. A stupid idea in it's essence. Should have been just 'Content' like WPF/UWP/Silverlight/Avalonia/Uno.

dansiegel commented 6 years ago

@jassmith now that the spec for ListView2 aka TheRealListView, aka CollectionView is out... one thing I wasn't noticing was the ability to not scroll... it's pretty much one of the key features of the RepeaterView that most of us have implemented and why it typically inherits from StackLayout. My concern would be that this would lead to undesirable behavior. For example with the RepeaterView concept I can nest a RepeaterView inside of a RepeaterView since I control where the ScrollView is I don't end up with nested scrolls. I know you have been vocal about people breaking virtualization when they nested a ListView inside of a ListView....

andreinitescu commented 6 years ago

@dansiegel If CollectionView turns out to be implement how I imagine it, it will work like how ItemsControl works in conjuction with StackPanel in Windows XAML. Which should answer your question regarding scrolling. See my comment here. Like I mentioned there, CollectionView should not have item selection or scrolling or the EmptyView, It should work and be as simple as the ItemsControl. Those capabilities should be put in a derived class, which I suggested to be called like ListCollectionView (similar to what ListView is in Windows XAML)

andreinitescu commented 6 years ago

I hope it the concept of 'cell' will vanish. A stupid idea in it's essence. Should have been just 'Content' like WPF/UWP/Silverlight/Avalonia/Uno.

I think ViewCell should just be DataTemplate, which apparently it's how it will be based on the spec proposal

ederbond commented 6 years ago

This is nice. Is there plans for a FlexRepeater too ? I mean, the same as this but inheriting from FlexLayout instead of StackLayout ? Think both will be wellcomed to XF.