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

Bindable layouts #4052

Merged

Conversation

andreinitescu
Copy link
Contributor

@andreinitescu andreinitescu commented Oct 10, 2018

Description of Change

It allows any Layout<View> to generate its content (child views) by binding to an items source, with option to set item template or item template selector:

<StackLayout BindableLayout.ItemsSource="{Binding Items}" />

<StackLayout BindableLayout.ItemsSource="{Binding Items}"
             BindableLayout.ItemTemplate="{Binding MyItemTemplate}" />

<StackLayout BindableLayout.ItemsSource="{Binding Items}"
             BindableLayout.ItemTemplateSelector="{Binding MyItemTemplateSelector}" />

In the first example, where neither item template nor item template selector are set, every item is a Label. If both item template and selector are set, the item template is used. If Items property is set, it is ignored.

Issues Resolved

API Changes

Added static class BindableLayout with following attached properties to Layout<View>:

  • public IEnumerable ItemsSource{ get; set; }
  • public DataTemplate ItemTemplate { get; set; }
  • public DataTemplateSelector ItemTemplateSelector { get; set; }
  • private BindableLayoutController BindableLayoutController { get; set; }

Added private class BindableLayoutController which encapsulates updating layout children based on changes on the collection, item template or item template selector.

Platforms Affected

  • Core/XAML (all platforms)

Testing Procedure

I added a gallery page (see screenshot below) which helps with testing all operations on a collection.

Before/After Screenshots

This change doesn't affect layout appearance, no matter the layout.
However I attached a screenshot which shows the gallery page:
image

The buttons should be self explanatory. "Replace" turns integers into chars to demonstrate replacing items in the observable collection. "Move" moves first item to last.

PR Checklist

  • Has automated tests (See BindableLayoutTests unit tests)
  • Rebased on top of the target branch at time of PR
  • Changes adhere to coding standard

Xamarin.Forms.Core/BindableLayout.cs Outdated Show resolved Hide resolved
Xamarin.Forms.Core/BindableLayout.cs Outdated Show resolved Hide resolved
Xamarin.Forms.Core/BindableLayout.cs Outdated Show resolved Hide resolved
Xamarin.Forms.Core/BindableLayout.cs Outdated Show resolved Hide resolved
Xamarin.Forms.Core/BindableLayout.cs Outdated Show resolved Hide resolved
@samhouts samhouts moved this from In Review to In Progress in v3.6.0 Oct 11, 2018
@andreinitescu
Copy link
Contributor Author

@StephaneDelcroix All requested changes done

Copy link
Member

@rmarinho rmarinho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @andreinitescu thanks for your contribution. We will need some unit tests.
UI Tests and a page on the gallery will also be great so we can try this out and test it.
You should also update the description related with the advance scneario as SetController is private now.
Screenshots will also be great.

Thanks

Xamarin.Forms.Core/BindableLayout.cs Outdated Show resolved Hide resolved
Xamarin.Forms.Core/BindableLayout.cs Outdated Show resolved Hide resolved
@xamarin xamarin deleted a comment Oct 12, 2018
@xamarin-release-manager xamarin-release-manager added the API-change Heads-up to reviewers that this PR may contain an API change label Oct 12, 2018
@StephaneDelcroix
Copy link
Member

@rmarinho UITests aren't required, it can all be unit-tested

@andreinitescu
Copy link
Contributor Author

andreinitescu commented Oct 15, 2018

@StephaneDelcroix @rmarinho @hartez

Updates:
✔️ Few fixes and naming per your suggestion (controller property name, event name, replaced ItemsSource return type from IList to IEnumerable)
✔️ Made few changes in the controller which reflect that it cannot be publicly set anymore (removed virtual methods from default controller)
✔️ Added gallery page (BindableLayoutGalleryPage)
✔️ Added unit tests (see BindableLayoutTests.cs)
✔️ Updated the ticket description and added screenshot with gallery page

@rmarinho Regarding your suggestion to update description with relevant advanced scenario for controller:
Since now setting controller is private per @StephaneDelcroix suggestion, advanced scenario is not applicable anymore.

@andreinitescu andreinitescu force-pushed the fix-1718-Repeater-Control branch 3 times, most recently from 4676497 to a3d8a74 Compare October 16, 2018 15:53
@andreinitescu
Copy link
Contributor Author

Squashed the commits

@andreinitescu
Copy link
Contributor Author

@StephaneDelcroix @rmarinho @hartez Would appreciate your feedback, thanks!

@rmarinho
Copy link
Member

Just tested, works fine for me. All my feedback was address. Good job.

@PureWeen PureWeen assigned StephaneDelcroix and unassigned hartez Oct 18, 2018
@PureWeen PureWeen removed the request for review from hartez October 18, 2018 16:22
Copy link
Member

@StephaneDelcroix StephaneDelcroix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm aware of the missed opportunity, we can consider opening this later

@StephaneDelcroix StephaneDelcroix merged commit 58010eb into xamarin:master Oct 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API-change Heads-up to reviewers that this PR may contain an API change community-sprint F100 hacktoberfest 🍻 t/enhancement ➕
Projects
No open projects
v3.5.0
  
Done
Development

Successfully merging this pull request may close these issues.

[Enhancement] Bindable Repeater control
6 participants