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

Implement Android single selection in CollectionView #4651

Open
wants to merge 8 commits into
base: 3.5.0
from

Conversation

@hartez
Member

hartez commented Dec 5, 2018

Description of Change

Implement single item selection for the CollectionView on Android.

Issues Resolved

None

API Changes

Added:

  • SelectionMode SelectableItemsView.SelectionMode
  • object SelectableItemsView.SelectedItem
  • ICommand SelectableItemsView.SelectionChangedCommand
  • object SelectableItemsView.SelectionChangedCommandParameter
  • event EventHandler SelectionChanged
  • enum SelectionMode
  • class SelectionChangedEventArgs

Platforms Affected

  • Core/XAML (all platforms)
  • Android

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

In Control Gallery, navigate to CollectionView Gallery -> Selection Galleries

PR Checklist

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

reviewed the Core part only, left some comments. do whatever you want with those. 👍

Show resolved Hide resolved Xamarin.Forms.Core/Items/ItemsView.cs Outdated
Show resolved Hide resolved Xamarin.Forms.Core/Items/ItemsView.cs Outdated
Show resolved Hide resolved Xamarin.Forms.Core/Items/SelectableItemsView.cs
Show resolved Hide resolved Xamarin.Forms.Core/Items/SelectableItemsView.cs Outdated
Show resolved Hide resolved Xamarin.Forms.Core/Items/SelectableItemsView.cs
@StephaneDelcroix

This comment has been minimized.

Member

StephaneDelcroix commented Dec 6, 2018

👍 for the latest changes

@samhouts samhouts added this to In Review in vNext (Target 3.5.0) Dec 6, 2018

@samhouts samhouts added the e/5 🕔 label Dec 6, 2018

@samhouts samhouts added this to In progress in Sprint 145 via automation Dec 6, 2018

Show resolved Hide resolved Xamarin.Forms.Core/Items/SelectableItemsView.cs Outdated
Show resolved Hide resolved Xamarin.Forms.Platform.Android/CollectionView/ItemsViewRenderer.cs
Show resolved Hide resolved ...rin.Forms.Platform.Android/CollectionView/SelectableItemsViewRenderer.cs
{
public static readonly BindableProperty SelectionModeProperty =
BindableProperty.Create(nameof(SelectionMode), typeof(SelectionMode), typeof(SelectableItemsView),
SelectionMode.None);

This comment has been minimized.

@paymicro

paymicro Dec 7, 2018

Collaborator

Can you clear the selected items when you change SelectionMode property?
Now if you select an item and set to None, the item will remain selected...
I think the algorithm should be like this:
None > clear selection
Single > if many items are selected, select the item with the smallest index of them

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Dec 11, 2018

Member

I agree with @paymicro, you should add a propertyChanged event handler

This comment has been minimized.

@hartez

hartez Dec 13, 2018

Member

@paymicro @StephaneDelcroix I'm not sure we should be so prescriptive here.

How should the control behave if the SelectionMode is 'None' and the user sets the SelectedItem?

@StephaneDelcroix

Approving this, but please figure out what's going on with the .csproj

Show resolved Hide resolved Xamarin.Forms.Controls/Xamarin.Forms.Controls.csproj Outdated
{
public static readonly BindableProperty SelectionModeProperty =
BindableProperty.Create(nameof(SelectionMode), typeof(SelectionMode), typeof(SelectableItemsView),
SelectionMode.None);

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix Dec 11, 2018

Member

I agree with @paymicro, you should add a propertyChanged event handler

Show resolved Hide resolved Xamarin.Forms.Core/Items/SelectableItemsView.cs Outdated

Sprint 145 automation moved this from In progress to Ready for Review (PRs) Dec 11, 2018

hartez and others added some commits Dec 5, 2018

Split files by class; make SelectionChangedEventArgs constructors int…
…ernal;

Optimizations for SelectionChangedEventArgs;
Fire selection changed event/command from property changed handler;
Update Xamarin.Forms.Core/Items/SelectableItemsView.cs
Co-Authored-By: StephaneDelcroix <stephane@delcroix.org>

@hartez hartez force-pushed the cv-android-single-select branch from 69367ff to ec5dfd9 Dec 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment