[DRAFT] Add LottieIsland and LottieWinRT for frameworkless or native applications #557
Conversation
…ad to build a custom lottie package and copy binaries into output folder manually since VS did not do it correctly. Will update build files to do this copy in the future.
SimpleIslandApp now calls into C# JSON loader
…apped XAML interfaces in new WinRT classes
…pts to build new projects
…isualSourceFrameworkless in order to avoid confusion with existing Microsoft.UI.Xaml.Controls interface
…stead of custom built package
…stom local nuget packages
…#, packages not set up correctly for native yet)
…='x86' /p:Configuration='Release' /target:'SimpleLottieIslandApp'
Lottie-Windows.sln
Outdated
| {C505CD2D-5D26-42EE-8FAA-41BB784821EF}.BETA|Any CPU.ActiveCfg = Debug|Any CPU | ||
| {C505CD2D-5D26-42EE-8FAA-41BB784821EF}.BETA|Any CPU.Build.0 = Debug|Any CPU | ||
| {C505CD2D-5D26-42EE-8FAA-41BB784821EF}.BETA|ARM.ActiveCfg = Debug|Any CPU | ||
| {C505CD2D-5D26-42EE-8FAA-41BB784821EF}.BETA|ARM.Build.0 = Debug|Any CPU |
There was a problem hiding this comment.
What's BETA? Is this a new config? How does this work with Debug and Release?
There was a problem hiding this comment.
Not sure, it was in the solution before I came along, and the BETA entries for the new projects were added automatically.
| @@ -0,0 +1,466 @@ | |||
| // Copyright (c) Microsoft Corporation. | |||
There was a problem hiding this comment.
We'll change this later to use AppWindow and remove a lot of unnecessary code.
…XAML dependency from Lottie-Windows-WinUI3
There was a problem hiding this comment.
Saw some concerns with namespaces and class names, so figured I'd provide some input in that space here before this gets merged/released.
Also, have some potential concerns around breaks/ramifications to the XAML layer in the base LottieVisualSource signature change.
| <RootNamespace>CommunityToolkit.WinAppSDK.LottieIsland</RootNamespace> | ||
| <AssemblyName>CommunityToolkit.WinAppSDK.LottieIsland</AssemblyName> |
There was a problem hiding this comment.
Most other NuGet packages are using the full WindowsAppSDK over abbreviating it. I'd suggest we use the expanded.
| /// An <see cref="IAnimatedVisualSource"/> for a Lottie composition. This allows | ||
| /// a Lottie to be specified as the source for a <see cref="AnimatedVisualPlayer"/>. | ||
| /// </summary> | ||
| public sealed class LottieVisualSourceWinUI : DependencyObject, IDynamicAnimatedVisualSource |
There was a problem hiding this comment.
Having WinUI in the class name seems really odd here as it's already part of the namespace. Do we need an alternate as LottieVisualSource is already used in the base library?
Will they ever both need to be used together though (if they're in different namespaces)?
I'm not completely looped into how this all works, but just trying to provide some general guidance when I saw the namespace/class names come up in the discussion.
| <RootNamespace>CommunityToolkit.WinAppSDK.LottieWinRT</RootNamespace> | ||
| <AssemblyName>CommunityToolkit.WinAppSDK.LottieWinRT</AssemblyName> |
There was a problem hiding this comment.
Having 'WinRT' seems a bit odd for the package/namespace/classes (as most packages/folks still associate WinRT with Win 8 outside of our more recent usage for CppWinRT/CsWinRT), as 'UWP' kind of became an umbrella for everything in the end in-between when we moved to Win 10.
Should this just be CommunityToolkit.WindowsAppSDK.Lottie instead?
Some more details in the PR description on the hierarchy and structure and what each package provides, what it does, and where it'd be used by an end-developer would help here.
|
|
||
| namespace CommunityToolkit.WinAppSDK.LottieWinRT | ||
| { | ||
| public sealed class LottieVisualSourceWinRT |
There was a problem hiding this comment.
Again, odd to have WinRT in the namespace and the class here. Would be good to have some XML comments and better understand which scenarios each of these sources needs to be used in, then might be able to help suggest alternative names.
| using Windows.Storage; | ||
| using Windows.Storage.Streams; | ||
|
|
||
| namespace CommunityToolkit.WinUI.Lottie.Controls |
There was a problem hiding this comment.
Since we're adding new code, would be good to start switching to file-scoped namespaces and reduce indentation.
| /// A class for a Lottie composition. This allows | ||
| /// a Lottie to be specified as source of a <see cref="IAnimatedVisual"/>. | ||
| /// </summary> | ||
| public sealed class LottieVisualSource |
There was a problem hiding this comment.
Why is this different in this case? Won't the dependency properties break if it's not a DependencyObject?
Is this to avoid loading XAML to use elsewhere, but then doesn't that break XAML scenarios?
Does that mean we need a new base class that provides the core functionality and not tied to XAML that then the XAML based pieces wrap here?
No description provided.