Port/extensions #12
Merged
Port/extensions #12
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove usings, file-scoped namespace, fix namespace Removed Shadows to be a separate component later (have in stash)
|
Compared with the Uno fork, most code is the same. Think they let Uno001 warnings bubble-up to app so folks can report what they need to Uno if it's something they're trying to use in the Toolkit. Should better understand that for the approach we want to take here as we treat them in errors in the CI. Maybe this is a difference we want to tackle here between Labs and the main Toolkit at the moment? Otherwise, just some small notes for future things to be aware of, otherwise nothing major. Biggest area that'll need some better investigation later is the string localization extensions... Especially between all four types of platforms, there's a lot of nuances here... |
Merged
Realizing that extension conflict is a bit unresolvable at the moment due to dependencies. Should get source package built so we can take as dependency in tooling and circle back? Probably need a flag to disable including, so that these tests use the source versions?
We need to come back and finish porting/fixing these tests once we sort out the namespace conflict by having the old versions in the dependencies of the tooling See: CommunityToolkit/Tooling-Windows-Submodule#39
Bunch of Uno0001 warnings turned errors. Think we turn it off for now? At least in this repo? I think if I put in our directory props, it should work now with the other fix. Thoughts @Arlodotexe @niels9001? |
Yeah.. let's get this in for UWP/WinUI and fix any Uno related thing later (if we can)? |
michael-hawker
force-pushed
the
port/extensions
branch
from
df120fc
to
f566f3e
Compare
michael-hawker
force-pushed
the
port/extensions
branch
from
2f74819
to
17bf62a
Compare
|
Ok, yay! CI passes*... This has the dependencies turned off for the extension code in the tests (or ported via temporary internal helpers). Next phase is to merge this (if no objections), then we can get a package and I can rebuild the tooling to use it with the new shared namespace and devops feed along with a flag for source usage. i.e. have a prop looked for like |
niels9001
approved these changes
Apr 4, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bringing over code for #6
This merges (most) of our extension code from both the
MT.UwpandMT.Uwp.UIpackages for theirExtensionsfolders. Makes a lot more sense in our new world for these to just be in one place to use from UWP or WinUI3 as most usage is for app development anyway and in WindowsAppSDK land the dependency is currently the same as WinUI is bundled in. Don't think non-app UWP developers would need access to theDispatcherQueueextensions? If so, the old packages should be fine. Curious on any thoughts from the past you may have @azchohfi (e.g. UWP APIs in scenarios where WinUI 2 wouldn't also just be referenced)?Think that's it for now?
As mentioned here, not bringing:
These classes don't really work in the WindowsAppSDK and have been disabled:
ProtectedCursorinstead, this was our whole Sizer/Splitter issue before)StringExtensions.GetViewLocalizedmethod as wellAlso, there are a few things known not to work in Uno, so currently disabled in Uno:
But may add to this list, haven't gone through all the Uno001 warnings yet nor compare with Uno branch.