Skip to content

Conversation

@twsouthwick
Copy link
Contributor

@twsouthwick twsouthwick commented Feb 4, 2017

This adds support to AdvancedCollectionView to sort on the object itself and also provides a way to add customer comparers if desired. This is done by adding three new overloads to SortDescription

  • SortDescription(SortDirection) will use the IComparable that the object itself defines (if available)
  • SortDescription(SortDirection, IComparer) provides a custom comparer for the object itself
  • SortDescription(string propertyName, SortDirection, IComparer) provides a custom comparer for the property indicated

Edit

If this goes in 1.3, we can just use an optional parameter (if it were to go in 1.4, that's a breaking binary change), and have two overloads for the constructor:

  • SortDescription(SortDirection, IComparer = null) will sort on the object itself
  • SortDescription(string propertyName, SortDirection, IComparer = null) sorts on the property with the given name

Fixes #881

This way the Comparer property on SortDescription is never null
cy = pi.GetValue(y);
}

try
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there's any chance this will make it into the current release, can we remove this try/catch? All it ends up doing is catching user errors that should be surfaced.

Copy link
Contributor

Choose a reason for hiding this comment

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

please update it. I will review and merge it in 1.3

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I've also switched it to use optional constructor args in SortDescription to reduce the constructor count from 4 to 2. If this doesn't make it into 1.3, then I'll revert that to explicitly list the constructors as that would be a breaking change.

@deltakosh deltakosh merged commit 84a81da into CommunityToolkit:dev Feb 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants