Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[Enhancement] Bindable Repeater control #1718

Closed
hartez opened this issue Jan 26, 2018 · 55 comments · Fixed by #4052
Closed

[Enhancement] Bindable Repeater control #1718

hartez opened this issue Jan 26, 2018 · 55 comments · Fixed by #4052

Comments

@hartez
Copy link
Contributor

hartez commented Jan 26, 2018

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

andreinitescu commented Jan 27, 2018

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
Copy link
Contributor

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
Copy link
Contributor

adamped commented Jan 27, 2018

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
Copy link
Contributor

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

@adamped
Copy link
Contributor

adamped commented Jan 28, 2018

Apologies, typo. I meant StackLayout :)

@adamped
Copy link
Contributor

adamped commented Jan 29, 2018

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

@PureWeen
Copy link
Contributor

PureWeen commented Jan 29, 2018

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

@davidortinau davidortinau self-assigned this Jan 29, 2018
@davidortinau davidortinau added this to In Progress in Enhancements Jan 29, 2018
@andreinitescu
Copy link
Contributor

@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
Copy link
Contributor

adamped commented Jan 29, 2018

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
Copy link
Contributor

andreinitescu commented Jan 29, 2018

@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
Copy link
Contributor

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
Copy link
Contributor

adamped commented Jan 29, 2018

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
Copy link
Contributor

andreinitescu commented Jan 29, 2018

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
Copy link
Contributor

adamped commented Jan 29, 2018

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
Copy link
Contributor

andreinitescu commented Jan 29, 2018

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
Copy link
Contributor

adamped commented Jan 29, 2018

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
Copy link
Contributor

andreinitescu commented Jan 29, 2018

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
Copy link
Contributor

adamped commented Jan 29, 2018

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
Copy link
Contributor

bmacombe commented Jan 29, 2018

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 hartez removed this from New in Triage Jan 29, 2018
@hartez
Copy link
Contributor Author

hartez commented Jan 29, 2018

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
Copy link
Contributor

bmacombe commented Jan 29, 2018

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.

@JuanuMusic
Copy link

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

@opcodewriter
Copy link

opcodewriter commented Jun 5, 2018

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
Copy link

jassmith commented Jun 5, 2018

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
Copy link

What's "ListView2" ?

@jassmith
Copy link

jassmith commented Jun 5, 2018

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

@opcodewriter
Copy link

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

@dansiegel
Copy link
Contributor

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

@jassmith
Copy link

jassmith commented Jun 5, 2018

It wont be called ListView2

@opcodewriter
Copy link

opcodewriter commented Jun 5, 2018

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

@weitzhandler
Copy link

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
Copy link
Contributor

@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
Copy link
Contributor

andreinitescu commented Jun 28, 2018

@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
Copy link
Contributor

andreinitescu commented Jun 28, 2018

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
Copy link
Contributor

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.

@StephaneDelcroix
Copy link
Member

ItemsSource and ItemTemplate should be attached bindable properties, so it could be applied to Grid, StackLayout, Flex, ...

@hartez
Copy link
Contributor Author

hartez commented Sep 25, 2018

I have updated the proposed spec to reflect the attached property suggestions from GalaxiaGuy and StephaneDelcroix.

@hartez
Copy link
Contributor Author

hartez commented Sep 25, 2018

RE: #1718 (comment)

After careful consideration, CollectionView will not cover the scenario from this proposal: #3172 (comment)

So this proposal is back on the table.

@GalaxiaGuy
Copy link
Contributor

@hartez Typo on the updated spec, both bindable properties are called ItemTemplate. :)

@hartez
Copy link
Contributor Author

hartez commented Sep 28, 2018

@hartez Typo on the updated spec, both bindable properties are called ItemTemplate. :)

D'oh! Thanks, fixed.

@andreinitescu andreinitescu mentioned this issue Oct 10, 2018
3 tasks
@samhouts samhouts added this to In Progress in v3.6.0 Oct 10, 2018
@samhouts samhouts removed this from Backlog in Enhancements Oct 11, 2018
StephaneDelcroix pushed a commit that referenced this issue Oct 26, 2018
@samhouts samhouts moved this from In Progress to Done in v3.6.0 Oct 29, 2018
@samhouts samhouts removed this from Done in v3.6.0 Jan 3, 2019
@samhouts samhouts added this to In Progress in v3.5.0 Jan 11, 2019
@samhouts samhouts moved this from In Progress to Done in v3.5.0 Jan 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
v3.5.0
  
Done
Development

Successfully merging a pull request may close this issue.