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

Remove extra properties from CarouselView API #7456

Merged
merged 5 commits into from
Sep 13, 2019
Merged

Conversation

hartez
Copy link
Contributor

@hartez hartez commented Sep 9, 2019

Description of Change

These changes adjust the inheritance tree to move some properties out of CarouselView's API which don't really make sense for the control. Also removes the question of what happens if you try to give the CarouselView a GridItemsLayout.

Also changes ListItemsLayout to LinearItemsLayout, which makes more sense for controls like CarouselView.

API Changes

ListItemsLayout -> LinearItemsLayout

CarouselView:

Removes Header, HeaderTemplate, Footer, FooterTemplate

IItemsLayout ItemsLayout -> LinearLayout ItemsLayout.

Platforms Affected

  • Core/XAML (all platforms)
  • iOS
  • Android
  • UWP

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

PR Checklist

  • Has automated tests
  • Rebased on top of the target branch at time of PR
  • Changes adhere to coding standard

@hartez hartez removed the request for review from StephaneDelcroix September 9, 2019 16:40
@hartez hartez changed the title Rework CarouselView API Remove extra properties from CarouselView API Sep 9, 2019
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.

Seems good, I think it makes sense.


public static readonly BindableProperty ItemTemplateProperty =
BindableProperty.Create(nameof(ItemTemplate), typeof(DataTemplate), typeof(ItemsView));
// public abstract IItemsLayout ItemsLayout { get; }
Copy link
Member

Choose a reason for hiding this comment

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

drop comment

}

public static readonly BindableProperty FooterProperty =
BindableProperty.Create(nameof(Footer), typeof(object), typeof(ItemsView), null);
Copy link
Member

Choose a reason for hiding this comment

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

Should be typeof(StructuredItemsView) ?

@@ -153,13 +153,23 @@ public object PositionChangedCommandParameter
set => SetValue(PositionChangedCommandParameterProperty, value);
}

public static readonly BindableProperty ItemsLayoutProperty =
Copy link
Member

Choose a reason for hiding this comment

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

I like how the PR title says "remove properties" when it actually adds some 👍

@samhouts samhouts added the API-change Heads-up to reviewers that this PR may contain an API change label Sep 12, 2019
@rmarinho rmarinho merged commit a68dd21 into 4.3.0 Sep 13, 2019
@samhouts samhouts added this to the 4.3.0 milestone Sep 18, 2019
felipebaltazar pushed a commit to felipebaltazar/Xamarin.Forms that referenced this pull request Oct 16, 2019
* Remove Header/Footer properties from CarouselView

* Limit CarouselView to LinearItemsLayout

* Rework inheritance for UWP

* Split files by class

* Fix up Tizen renderer
felipebaltazar pushed a commit to felipebaltazar/Xamarin.Forms that referenced this pull request Oct 16, 2019
* Remove Header/Footer properties from CarouselView

* Limit CarouselView to LinearItemsLayout

* Rework inheritance for UWP

* Split files by class

* Fix up Tizen renderer
@hartez hartez deleted the carouselview-api branch October 23, 2019 14:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants