Skip to content
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

🧪 [Experiment] SizerBase class for GridSplitter + ContentSizer + PropertySizer #102

Merged
merged 18 commits into from
May 13, 2022

Conversation

michael-hawker
Copy link
Member

Approved Discussion #96

Tracking Issue #101

Initial Commit of Experiment

This PR adds:

  • SizerBase an abstract base class to centralize logic around input, visuals, and accessibility.
  • GridSplitter knows how to manipulate a Grid's rows/columns
  • ContentSizer knows how to manipulate the Width/Height of any FrameworkElement
  • PropertySizer knows how to directly manipulate any bound double value
  • Initial docs/samples for these components
  • Tested manually and working on UWP/WinUI 3/WASM with no issues.
  • Small fix for WASM loading files in root of sample projects
  • Update to WinUI 3 version to help test on latest: Can't set ProtectedCursor in DependencyProperty PropertyChangedCallback - Access Violation microsoft/microsoft-ui-xaml#7062 (workaround is in this code, so no crash now here for us)

I'd like to finish before merging:

  • GridSplitter examples

@michael-hawker michael-hawker added enhancement Improvement to an existing feature experiment 🧪 Used to track issues that are experiments (or their linked discussions) partner ⏹ Label for issues that are being depended on by Toolkit partners. labels May 6, 2022
@michael-hawker
Copy link
Member Author

Hmm, looks like I need to update the .NET version somewhere:

"D:\a\Labs-Windows\Labs-Windows\Toolkit.Labs.All.sln" (default target) (1:2) ->
"D:\a\Labs-Windows\Labs-Windows\tests\CommunityToolkit.Labs.UnitTests.WinAppSdk\CommunityToolkit.Labs.UnitTests.WinAppSdk.csproj" (default target) (15:6) ->
(WindowsAppSDKVerifyWinrtRuntimeVersion target) -> 
  C:\Users\runneradmin\.nuget\packages\microsoft.windowsappsdk\1.1.0-preview3\buildTransitive\Microsoft.WindowsAppSDK.targets(34,9): error : This version of WindowsAppSDK requires WinRT.Runtime.dll version 1.6 or greater. [D:\a\Labs-Windows\Labs-Windows\tests\CommunityToolkit.Labs.UnitTests.WinAppSdk\CommunityToolkit.Labs.UnitTests.WinAppSdk.csproj]
C:\Users\runneradmin\.nuget\packages\microsoft.windowsappsdk\1.1.0-preview3\buildTransitive\Microsoft.WindowsAppSDK.targets(34,9): error :     Please update to .NET SDK 6.0.104, 6.0.202, 5.0.213 or 5.0.407 (or later). [D:\a\Labs-Windows\Labs-Windows\tests\CommunityToolkit.Labs.UnitTests.WinAppSdk\CommunityToolkit.Labs.UnitTests.WinAppSdk.csproj]
C:\Users\runneradmin\.nuget\packages\microsoft.windowsappsdk\1.1.0-preview3\buildTransitive\Microsoft.WindowsAppSDK.targets(34,9): error :     Or add a temporary Microsoft.Windows.SDK.NET.Ref reference of version 10.0.*.24 or later. [D:\a\Labs-Windows\Labs-Windows\tests\CommunityToolkit.Labs.UnitTests.WinAppSdk\CommunityToolkit.Labs.UnitTests.WinAppSdk.csproj]
C:\Users\runneradmin\.nuget\packages\microsoft.windowsappsdk\1.1.0-preview3\buildTransitive\Microsoft.WindowsAppSDK.targets(34,9): error :     For example, [D:\a\Labs-Windows\Labs-Windows\tests\CommunityToolkit.Labs.UnitTests.WinAppSdk\CommunityToolkit.Labs.UnitTests.WinAppSdk.csproj]
C:\Users\runneradmin\.nuget\packages\microsoft.windowsappsdk\1.1.0-preview3\buildTransitive\Microsoft.WindowsAppSDK.targets(34,9): error :         <PropertyGroup> [D:\a\Labs-Windows\Labs-Windows\tests\CommunityToolkit.Labs.UnitTests.WinAppSdk\CommunityToolkit.Labs.UnitTests.WinAppSdk.csproj]
C:\Users\runneradmin\.nuget\packages\microsoft.windowsappsdk\1.1.0-preview3\buildTransitive\Microsoft.WindowsAppSDK.targets(34,9): error :             <WindowsSdkPackageVersion>10.0.<Target Windows SDK Build Number>.24</WindowsSdkPackageVersion> [D:\a\Labs-Windows\Labs-Windows\tests\CommunityToolkit.Labs.UnitTests.WinAppSdk\CommunityToolkit.Labs.UnitTests.WinAppSdk.csproj]
C:\Users\runneradmin\.nuget\packages\microsoft.windowsappsdk\1.1.0-preview3\buildTransitive\Microsoft.WindowsAppSDK.targets(34,9): error :         </PropertyGroup> [D:\a\Labs-Windows\Labs-Windows\tests\CommunityToolkit.Labs.UnitTests.WinAppSdk\CommunityToolkit.Labs.UnitTests.WinAppSdk.csproj]
C:\Users\runneradmin\.nuget\packages\microsoft.windowsappsdk\1.1.0-preview3\buildTransitive\Microsoft.WindowsAppSDK.targets(34,9): error :  [D:\a\Labs-Windows\Labs-Windows\tests\CommunityToolkit.Labs.UnitTests.WinAppSdk\CommunityToolkit.Labs.UnitTests.WinAppSdk.csproj]

Will look on Monday

@michael-hawker michael-hawker force-pushed the experiment/sizerbase branch from 183b109 to afa3242 Compare May 10, 2022 17:52
@michael-hawker
Copy link
Member Author

Should have all uno guards in place now. Glad they show up in the CI, that's pretty cool.

Main issue is we're getting some other internal uno error message about __storeBackingField from the DependencyObject generator. Following up with Uno team. Couldn't reproduce in a new Uno project copying our code over.

@michael-hawker michael-hawker force-pushed the experiment/sizerbase branch 2 times, most recently from d716ec6 to 8f236d2 Compare May 11, 2022 21:46
@michael-hawker
Copy link
Member Author

Added ProjectTemplateRns to the <RootNamespace> tags for the source/library projects. that did work around #110 and cause the linux CI to pass. 🎉

Think it's fine for a workaround for now, we just want to validate new experiments don't keep that in the namespaces used in the code files during PRs until the underlying Uno issue is fixed and we can go back to using the same root namespace for everything.

@michael-hawker michael-hawker force-pushed the experiment/sizerbase branch from 9fffd3a to 7dfb6cf Compare May 12, 2022 22:50
@michael-hawker
Copy link
Member Author

Woot! We have a clean build. Think this is all set to review @mrlacey and @Arlodotexe. (Outside of #116 and GridSplitter examples). Let me know if we want that fixed here first or as a separate PR. Would be nice to get a 2nd experiment in main though.

Copy link
Contributor

@mrlacey mrlacey left a comment

Choose a reason for hiding this comment

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

Approving on the basis that this gets a second experiment into the codebase.
However, this will be affected by #116 and #120

@michael-hawker
Copy link
Member Author

Chatted with Arlo, so this isn't modifying the code that is causing #116 and #120 (and just exposing it). Let's merge this and our other open PRs to get all back together on our open work, and then take a look at fixing things forward.

@michael-hawker michael-hawker merged commit 2fec488 into main May 13, 2022
@delete-merged-branch delete-merged-branch bot deleted the experiment/sizerbase branch May 13, 2022 19:46
Martin1994 pushed a commit to Martin1994/Labs-Windows that referenced this pull request Sep 2, 2023
…ent/sizerbase

🧪 [Experiment] SizerBase class for GridSplitter + ContentSizer + PropertySizer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement to an existing feature experiment 🧪 Used to track issues that are experiments (or their linked discussions) partner ⏹ Label for issues that are being depended on by Toolkit partners.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🧪 [Experiment] SizerBase class for GridSplitter + ContentSizer + PropertySizer
2 participants