-
Notifications
You must be signed in to change notification settings - Fork 11
[WIP] Port this to macOS/Linux and add support for other XAML variants #51
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
base: dev
Are you sure you want to change the base?
Conversation
michael-hawker
left a comment
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.
Some comments for the approach here to make it more compatible with the direction needed for the project currently. i.e. it'd be great if we could find ways to do this in a way that moves us forward for WinUI 3 and just general clean-up while also just happening to be good for Uno too. Thanks! 🙂
| [TemplatePart(Name = PartAdornerLayer, Type =typeof(AdornerLayer))] | ||
| #if UNO | ||
| [ContentProperty(nameof(Child))] | ||
| #else | ||
| [ContentProperty(Name = nameof(Child))] | ||
| #endif |
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.
This class is going away in #42
| #if UNO | ||
| public class ListViewItemSelectedBehavior : CommunityToolkit.WinUI.Behaviors.BehaviorBase<FrameworkElement> | ||
| #else | ||
| public class ListViewItemSelectedBehavior : BehaviorBase<FrameworkElement> | ||
| #endif |
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.
Could just use the fully qualified name for both cases, right? just have a comment as to why. I guess there's some conflict with a similar named class in Uno?
| #if UNO | ||
| private static void OnTapped(object sender, Microsoft.UI.Xaml.Input.TappedRoutedEventArgs e) | ||
| #else | ||
| private static void OnTapped(object sender, Windows.UI.Xaml.Input.TappedRoutedEventArgs e) | ||
| #endif |
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.
An easier approach for all these may be global namespaces which we'd need to use later eventually to support both UWP + WinUI 3.
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.
All the Pivot stuff probably isn't even used anymore, the original prototype version from like 2017 used to use Pivot before a change to ListBox, and now NavigationView is used... It may be easier to just remove some of these old things that aren't used anymore.
|
@michael-hawker Thanks for the comments. Overall, the port is smooth, till DataGrid stands in the way. I think I will stop for a while and wait for your WinUI 3 tasks to be kicked off. Like you predicted, things like Monaco.Editor and DataGrid might not move forward into the future, so it would take a while to switch to alternatives. But overall the code base is a clean foundation to build upon. |
… handler signatures and conditional compilation directives.
…finitions, event handlers, and styles for better integration.
Fixes
Add simple but necessary conditional compilation to ensure the same source files compile on Uno Platform 6.4.
More outlined in this discussion.
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Uno 6.4 has types in different namespaces, so certain lines won't compile there even with other compiler tricks.
What is the new behavior?
The same files compile on both WinAppSDK and Uno 6.4
PR Checklist
Please check if your PR fulfills the following requirements:
Other information
Since it is still work-in-progress, not ready for full review yet. More commits will follow.