-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Uno Platform Compatibility updates #3651
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
Conversation
Thanks jeromelaban 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 🙌 |
e33b079
to
79cd819
Compare
79cd819
to
44e06fe
Compare
This change enables Uno Platform compatibility forward compatibility.
44e06fe
to
01acee9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. FYI @Sergio0694 for the new Animations stuff and @RosarioPulella for the new Controls split.
@jeromelaban any reason this was opened as a draft in particular? |
It was until I got the build passing :) It now is. Thanks for the review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 😄
Great to see some work towards unifying the codebase with Uno Platform! 🎉
Question: do we want to hold this off until #3639 is merged, since that completely rewrites the Microsoft.Toolkit.Uwp.UI.Animations
package and hence would require the same changes applied to it too, or do we just want to merge this in the meantime and then manually do the same tweaks in there? 🤔
We're happy to contribute and make the toolkit even greater 🙂 This PR mainly reduces the number of partials to be merged, and it is expected that it'll be needed to add some more over time when Uno will make more steps toward being mergeable. There is no need to consider adding partials blindly at this point, as far as the current master is concerned. |
@Sergio0694 think we can just merge this now? We'll have a few things to sort out but merge conflicts should hopefully be pretty self-explanatory? |
Thanks @RosarioPulella for the update, I indeed missed this one ;) |
@jeromelaban, I noticed this |
@RosarioPulella it's not a tag I can manage, it was added automatically by the msftbot. This is PR is ready from my side. |
Hello @RosarioPulella! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
PR Type
What kind of change does this PR introduce?
What is the new behavior?
This PR adds a set of forward compatibilty changes to ease the cross-compilation of the community toolkit to Uno Platform targets.
TargetFrameworks
instead ofTargetFramework
to allow for late modification of the platform listPR Checklist
Please check if your PR fulfills the following requirements:
Other information