Add observable group API

[vgromfeld]

PR Type

What kind of change does this PR introduce?

• Feature

What is the new behavior?

When using grouping in ListView or GridView, we often have to create our own group classes and re-implement the same kind of logic.

In this PR, I'm proposing a set of classes to simplify this:

• ObservableGroup<TKey, TValue>: an observable list of TValue with a group name (TKey)
• ObservableGroupedCollection<TKey, TValue>: an observable list of groups. It can be created from a LINQ query result.
• ReadOnlyObservableGroup<TKey, TValue>: a read-only ObservableGroup<TKey, TValue>
• ReadOnlyObservableGroupedCollection<TKey, TValue>: a read-only ObservableGroupedCollection<TKey, TValue>.

PR Checklist

Please check if your PR fulfills the following requirements:

• Tested code with current supported SDKs
• Pull Request has been submitted to the documentation repository Add documentation of observable groups MicrosoftDocs/WindowsCommunityToolkitDocs#317
• Sample in sample app has been added / updated (for bug fixes / features)
  • Icon has been created (if new sample) following the Thumbnail Style Guide and templates
• Tests for the changes have been added (for bug fixes / features) (if applicable)
• Header has been added to all new source files (run build/UpdateHeaders.bat)
• Contains NO breaking changes

[ghost]

Thanks vgromfeld for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

[Sergio0694]

This looks great, nice work! 😄
I had a very similar version of these same APIs in my own apps, and I know of other UWP devs that have basically rebuilt pretty much the same APIs from scratch as well, so it'll be nice to have them available right from the toolkit! I just have a couple of suggestions out of experience of what I noticed while working with these APIs in my own app, and a general questions in the 3rd review comment.

Other than that, here's also another feature suggestion, not adding this as a review comment since it's not directly tied to any file in particular. I noticed that adding new items to a grouped collection can get quite verbose, especially if you find yourself adding new groups with a single item. Basically, in that case you'd need to write the following:

Source.Add(new ObservableGroup<SomeKeyType, SomeGroupItemValueType>(key, value));

Which especially when done often, makes the code a bit crowded. I came up with the extension class that I've been using in my app, would it make sense to add it to this PR as well?

/// <summary>
/// An extension <see langword="class"/> for <see cref="ObservableCollection{T}"/> type.
/// </summary>
public static class ObservableCollectionExtensions
{
    /// <summary>
    /// Adds a key-value <see cref="ObservableGroup{TKey,TElement}"/> item into a target <see cref="ObservableCollection{T}"/>.
    /// </summary>
    /// <typeparam name="TKey">The type of the group key.</typeparam>
    /// <typeparam name="TElement">The type of the elements in the group.</typeparam>
    /// <param name="source">The source <see cref="ObservableCollection{T}"/> instance.</param>
    /// <param name="key">The key to add.</param>
    /// <param name="element">The element to add.</param>
    public static void AddGroup<TKey, TElement>(
        this ObservableCollection<ObservableGroup<TKey, TElement>> source,
        TKey key,
        TElement element)
    {
        Add(source, key, new [] { element });
    }

    /// <summary>
    /// Adds a key-collection <see cref="ObservableGroup{TKey,TElement}"/> item into a target <see cref="ObservableCollection{T}"/>.
    /// </summary>
    /// <typeparam name="TKey">The type of the group key.</typeparam>
    /// <typeparam name="TElement">The type of the elements in the group.</typeparam>
    /// <param name="source">The source <see cref="ObservableCollection{T}"/> instance.</param>
    /// <param name="key">The key to add.</param>
    /// <param name="collection">The collection to add.</param>
    public static void AddGroup<TKey, TElement>(
        this ObservableCollection<ObservableGroup<TKey, TElement>> source,
        TKey key,
        IEnumerable<TElement> collection)
    {
        source.Add(new ObservableGroup<TKey, TElement>(key, collection));
    }
}

It's just a very small syntactic sugar pair of extensions, really, which let you write the following:

Source.Add(key, value);

Just an idea, very ncie job again with the PR! 👏

[vgromfeld]

@Sergio0694 +1 for the extensions.
I haven't mentioned it in the PR description but, I'm also planning to add an helper methods to automatically update the ObservableGroupedCollection<TKey, TValue> content from a LINQ query result (IEnumerable<IGrouping<TKey, TValue>>) updating/adding/removing the groups.

This will go in another PR.

[Sergio0694]

Ah that's great then, yeah having all that part in another PR would make it easier to review I guess, glad you have that planned as well though, this couple PRs look very useful! 😊

[ghost]

This PR has been marked as "needs attention 👋" and awaiting a response from the team.

[Sergio0694]

Just a few more small notes 😄

Thank you for adding the IReadOnlyObservableGroup interface!

Also, just making sure my last comment was lost since the parent thread was already marked as resolved, let me know what you think in this comment! 😊

[michael-hawker]

@vgromfeld this is going to be great addition to the Toolkit, just started looking at this, but I know when I added grouping in XAML Studio, I was surprised there was no default implementation for the IGrouping interface! I also encountered issues with accessibility as the Key's wouldn't have names by default.

For reference to compare against your implementation (just in case we're missing any use-cases), this was my implementation I had planned to contribute back into the Toolkit:

public class Grouping<TKey, TElement> : IGrouping<TKey, TElement>, IEnumerable<TElement>, IEnumerable, ICollectionViewGroup
    {
        public object Group => Key;

        public IObservableVector<object> GroupItems => (IObservableVector<object>)new ObservableCollection<object>((IEnumerable<object>)this);

        public TKey Key { get; private set; }

        private IEnumerable<TElement> Items { get; }

        public Grouping(TKey key, IEnumerable<TElement> items)
        {
            Key = key;
            Items = items;
        }

        public IEnumerator<TElement> GetEnumerator()
        {
            return Items.GetEnumerator();
        }

        IEnumerator IEnumerable.GetEnumerator()
        {
            return Items.GetEnumerator();
        }

        public override string ToString()
        {
            return Key?.ToString();
        }
    }

    public static class EnumerableExtensions
    {
        public static Grouping<TKey, TElement> ToGroup<TKey, TElement>(this IEnumerable<TElement> list, TKey key)
            => new Grouping<TKey, TElement>(key, list);

        public static IEnumerable<Grouping<TKey, TElement>> ToGroup<TKey, TElement>(this IEnumerable<TElement> list, Func<TElement, TKey> keySelector)
            => list.GroupBy(keySelector, (key, items) => new Grouping<TKey, TElement>(key, items));
    }

    public static class GroupingExtensions
    {
        // Converts a System Group to this Group.
        public static Grouping<TKey, TElement> ToGroup<TKey, TElement>(this IGrouping<TKey, TElement> group)
            => new Grouping<TKey, TElement>(group.Key, group.AsEnumerable());
    }

I think your PR covers most of this though (or will by the end), eh?

FYI @Sergio0694

[ghost]

This PR has been marked as "needs attention 👋" and awaiting a response from the team.

[vgromfeld]

@michael-hawker Our grouping implementations are similar (except the extensions methods which are not in this PR 😀)

[michael-hawker]

@vgromfeld don't forget about the docs PR for this as well. :)

[michael-hawker]

Couple of small tweaks for the sample, and needs the doc PR started, but otherwise looking great! Going to test this out more via copy+paste in my stream today.

[ghost]

This PR has been marked as "needs attention 👋" and awaiting a response from the team.

[vgromfeld]

@vgromfeld don't forget about the docs PR for this as well. :)

Done in MicrosoftDocs/WindowsCommunityToolkitDocs#317.

[Sergio0694]

I was thinking, while it's absolutely fine to add this to the main package now in 6.1, but would it make sense to eventually move all these new types to Microsoft.Toolkit.Mvvm when (if) #3229 is merged?

All these new types are primarily meant to be used with the MVVM pattern (they're all collections using the INotifyPropertyChanged interface, etc.), and they're all just using .NET Standard APIs.

Just a note, thought I'd mention this for future reference 😄

[michael-hawker]

@Sergio0694 I think this is probably the right place for them, the main use-case for these is to help with grouping in a ListView which doesn't need to be using an MVVM pattern.

So I think Microsoft.Toolkit.Collections seems like a good spot. Does the MVVM package reference the Microsoft.Toolkit package? As if so, anyone using the MVVM package would get these pulled in that way anyway?

[Sergio0694]

@michael-hawker Nope, as of right now the Microsoft.Toolkit.Mvvm package is completely self-contained. Didn't really need any of the classes from the base Microsoft.Toolkit package so I thought I might as well just leave this new one standalone for good measure.

I was just thinking that, especially in the context of a more "general .NET"-oriented toolkit architecture, these new collection types would've been a good fit for the Mvvm package, since they're specifically meant to be used in MVVM scenarios. Just like the TaskNotify<T> class will be removed from the base toolkit and replaced from functionality from the .Mvvm package, and the Singleton<T> class will be gone as well and replaced by the Ioc class and the other Microsoft.Extensions.DependencyInjection extensions from the .Mvvm package.

As in, my rationale was to leave the base toolkit for basically 100% platform/framework agnostic things, such as general extensions for base types (eg. value types, Type, arrays, etc.), the Guard class for debugging things, and then move the context-dependent types (ie. these observable collections for MVVM scenarios) to a dedicated framework.

This way eg. someone writing a .NET Core console app, which doesn't have any UI framework at all, would just see the types relevent to him when referencing the base package from the toolkit.

Of course, just thinking out loud here! 😊

[michael-hawker]

Thanks @Sergio0694, I think we just want to be careful about putting things in a separate package where they can also be used in non-MVVM scenarios, as then that adds extra dependencies the MVVM package has.

Let's leave this here for now, and see how feedback goes on all these new APIs in upcoming releases.