[Experiment] Initial CompositionCollectionView implementation and basic samples #277
Conversation
… into feature/compositionCollectionView
…tionCollectionView2
There was a problem hiding this comment.
Looks like there's a back-up csproj file committed as well.
Haven't had a chance to try it out, but wanted to provide a few high-level comments first. Thanks for the PR!
@Arlodotexe do we have an easy way to exclude something entirely from Uno at the moment?
labs/CompositionCollectionView/src/CompositionCollectionView.cs
Outdated
Show resolved
Hide resolved
labs/CompositionCollectionView/src/CompositionCollectionView.cs
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| #if !WINAPPSDK | ||
| public abstract partial class Layout<TId, TItem> : ILayout, IDisposable |
There was a problem hiding this comment.
Layout conflicts with the base ItemsRepeater Layout class and is too generic here (which is why you probably ran into immediate issues in WindowsAppSDK land). This should be more like CompositionCollectionLayout or something. Maybe we need a more concise name for the layout itself?
There was a problem hiding this comment.
Sounds good, also I'm flexible about naming in general, if someone suggests something that rolls off the tongue better than CompositionCollectionView I'm happy to rename it
We don't (yet) have a way to disable a component under a specific platform. The expectation so far is that Labs experiments may not be production ready, but once it is, components should work on as many platforms as reasonably possible. So far, we haven't come across any experiment that will never run under a specific platform, but we can (and should) in the future if it becomes a hard blocker. |
I'm afraid this might be the first experiment that falls in that bucket, because it depends heavily on the composition animation API and as far as I can tell Uno doesn't have any plans to add support for those anytime soon. (They mentioned it as a required feature in the Animation for Android epic (unoplatform/uno#5586, which hasn't seen much activity lately and isn't added to any milestone, and it isn't even mentioned at all in the Animation for WASM epic unoplatform/uno#3919) |
|
We definitely need to figure out some story around documenting and calling out supported platforms within Labs, even if we expect stuff to get Uno support later, it may not start out that way. @Arlodotexe something for us to start discussing as we plan for 8.0. @arcadiogarcia want to run XAML Styler with the settings in the repository (there's also a script in the repo root) so that the rest of the build will run? |
|
@arcadiogarcia Do you know why the source project has a shared project in it? This isn't part of our template 🤔
|
|
Note, this is also another project which only works on UWP/WinAppSDK, so we'll need whatever fix we do for the #353 to not generate the wasm single project head here as well. |
I don't remember tbh. Probably I was trying something and forgot to remove it, as far as I can tell is not important, I deleted it. |
|
@arcadiogarcia sorry things keep changing underneath you, there's a lot of moving parts still and we're trying to push forward to be able to split our infrastructure out to reuse. Hopefully things will settle down. Saw there's changes to the global usings file, so there's a conflict now. Most of that seems fine, though we have an open item to refactor that process CommunityToolkit/Tooling-Windows-Submodule#15. Your changes seem fine, though I am a bit weary of including |
Sure no worries! I removed the global |
|
Btw should I expect to see the nuget package in the PR feed already? (https://pkgs.dev.azure.com/dotnet/CommunityToolkit/_packaging/CommunityToolkit-PullRequests/nuget/v3/index.json) I see that the new-experiment build succeeds but I don't see the nuget in the feed, do I have to fix something else for it to get published there? |
That's odd. This isn't present on |
|
Looks like the error messages on the build are all this: Which is weird as that's a different experiment... |
| @@ -0,0 +1,20 @@ | |||
| <!-- | |||
There was a problem hiding this comment.
@Arlodotexe these are remnants of the old system...
Also we're missing the multi-target props, so think that's why we're seeing the Uno warnings/errors, eh?
D:\a\Labs-Windows\Labs-Windows\components\CompositionCollectionView\src\Behaviors\InteractionTrackerGesture\GesturePreviewControl.cs(8,23): error CS0260: Missing partial modifier on declaration of type 'GesturePreviewControl'; another partial declaration of this type exists [D:\a\Labs-Windows\Labs-Windows\components\CompositionCollectionView\src\CommunityToolkit.Labs.WinUI.CompositionCollectionView.csproj]
D:\a\Labs-Windows\Labs-Windows\components\CompositionCollectionView\src\CompositionCollectionView.cs(11,21): error CS0260: Missing partial modifier on declaration of type 'CompositionCollectionView'; another partial declaration of this type exists [D:\a\Labs-Windows\Labs-Windows\components\CompositionCollectionView\src\CommunityToolkit.Labs.WinUI.CompositionCollectionView.csproj]
| #endif | ||
|
|
||
| global using MUXC = Microsoft.UI.Xaml.Controls; | ||
| // Licensed to the .NET Foundation under one or more agreements. |
There was a problem hiding this comment.
Changes here are going to be a problem/conflict with the work we're doing for #405 and #398. Should just tie-in to clean up needed for #321?
| @@ -9,7 +9,6 @@ | |||
| <Copyright>(c) .NET Foundation and Contributors. All rights reserved.</Copyright> | |||
| <PackageProjectUrl>https://github.com/CommunityToolkit/Labs-Windows</PackageProjectUrl> | |||
| <PackageReleaseNotes>https://github.com/CommunityToolkit/Labs-Windows/releases</PackageReleaseNotes> | |||
| <PackageIcon>Icon.png</PackageIcon> | |||
There was a problem hiding this comment.
There's some other non-needed changes here and in the common\test projects
| @@ -0,0 +1,137 @@ | |||
| #nullable disable | |||
There was a problem hiding this comment.
License should still be top thing, does this tie into CS8602? Ideally we should just be good had writing nullable safe code, but probably easier to just do at project level in csproj and track via a todo later.
There was a problem hiding this comment.
Nullability is worth doing a pass on before merging in here. This PR will already be nontrivial to bring up to date with our latest tooling.
| using System.Numerics; | ||
| using Windows.UI; | ||
|
|
||
| namespace Microsoft.Toolkit.Uwp.UI.Animations.ExpressionsFork |
There was a problem hiding this comment.
Do these things already exist in the toolkit? Can we not just reference them from the existing package?
There was a problem hiding this comment.
This experiment needs the changes from my branch at CommunityToolkit/WindowsCommunityToolkit#4559, which we never merged to master, that's why I'm including my own fork that is evolving in parallel. I agree that adding a copy of those classes is not ideal, but there doesn't seem to be any good way to add functionality to those core toolkit classes from an experiment (I can't add the Evaluate() method as an extension method because it relies on private properties of the ExpressionNodes).
| @@ -0,0 +1,17 @@ | |||
| // Licensed to the .NET Foundation under one or more agreements. | |||
There was a problem hiding this comment.
We should just use PolySharp reference for these. Were you using records somewhere?
There was a problem hiding this comment.
PolySharp is included by default on the Uwp head. For component libraries, they need to be included on the project itself, see examples here.
| #endif | ||
| } | ||
|
|
||
| #if !WINAPPSDK |
There was a problem hiding this comment.
Bit confused by these conditionals in the samples, there's no comment, why is this excluded on the WindowsAppSDK?
| @@ -0,0 +1,97 @@ | |||
| // Licensed to the .NET Foundation under one or more agreements. | |||
There was a problem hiding this comment.
This file should be CanvasSample.xaml.cs no?
Co-authored-by: Michael Hawker MSFT (XAML Llama) <[email protected]>
This PR contains the initial implementation of the CompositionCollectionView control proposed in #135. More specifically, it implements:
CommunityToolkit.Labs_.CompositionCollectionView.Sample.2022-09-18.22-29-20.mp4
I'd like to check this in as an initial version of the control so it can be pulled into apps easily and tried by other people. While the public interfaces might undergo changes and I plan to add more behaviors and samples, functionality wise the control is feature complete. I still need to add tests and document each method too.
Other caveats: