Add cross-platform UI based on Avalonia #336
Conversation
mjcheetham
changed the title
mjcheetham
requested review from
derrickstolee,
jeffhostetler,
dscho,
vtbassmatt and
ldennington
|
Nice! I'm 👍 from a product perspective, no code review provided though 🤣 |
mjcheetham
added
enhancement
There was a problem hiding this comment.
On the whole, this was a lovely PR to review - the code was super clean and fun to follow.
However, I do have one recommendation. This was a bit of a long review, especially for someone like me who doesn't do a ton of UI work. Something I try to do when I'm working on big changes like this is submit them gradually in smaller, more digestible PRs. I know it's a bit of a pain to do, but from my experience it makes people more willing to look at my code and allows them to examine it more thoroughly and catch things they may have glossed over otherwise.
Other than that, awesome work! I think the new UIs are quite beautiful.
src/shared/Atlassian.Bitbucket.UI/ViewModels/CredentialsViewModel.cs
Outdated
Show resolved
Hide resolved
src/shared/Microsoft.Git.CredentialManager.UI/ViewModels/WindowViewModel.cs
Show resolved
Hide resolved
src/shared/Microsoft.Git.CredentialManager/Authentication/AuthenticationBase.cs
Outdated
Show resolved
Hide resolved
Ahh sorry! I should perhaps have called out in the main PR comment that the best way to review here is commit-by-commit.
|
Thanks, that's good to know! Will do that moving forward. |
Push the `GetApplicationPath` method down to the ApplicationBase type so its logic can be shared across other applications.
Extract the `OpenDefaultBrowser` method to a helper/util class so it can be used by other components.
Allow UI helper processes to be terminated by cancelling a CancelationToken that has been passes to `InvokeHelperAsync`.
Add a new project to host all the shared components for Avalonia-based UI helpers.
Add a styled dialog window control for use by UI helper applications built on Avalonia.
Add a new application class for helper applications to surface commands and respond over standard output.
Add a dispatcher type for use with Avalonia. This dispatcher is different from the built-in Avalonia `Dispatcher` type. On macOS the underlying platform requires that the main NSRunLoop be run on the **entry** thread of the process (that is, thread 0). This is unlike on Windows where all that is required is an "STA" thread. Because of this we need to "reserve" thread 0 (the main entry thread) for running the Avalonia infrastructure, and kick off a new thread to run the rest of the "normal" .NET application logic (such as command-line handling). The dispatcher allows queuing of work to be run on the initialised thread, until the loop is terminated. We can use this dispatcher and model to start the Avalonia app/runloops at a defered time, after main app startup. The Avalonia `Dispatcher` type is then used to push more work on to that inner message pump/runloop after the Avalonia application has started.
Add a component that uses our Dispatcher, and the Avalonia Dispatcher types to correctly initialise, show, and close windows and views in the Avalonia UI framework, ad-hoc.
Add a new Avalonia UI project for a GitHub UI helper.
Add the main "credentials" prompt for the Avalonia-based GitHub helper UI.
Add new Avalonia-based "two-factor" auth code prompt dialog for GitHub.
Pass the `--sms` option to the UI helper when prompting for a 2FA code sent via SMS for GitHub.
Add a new UI helper for Bitbucket using Avalonia.
There was a problem hiding this comment.
This is quite a big project!
I can't speak to most of the Avalonia/UX boilerplate, but I tried to find some interesting tidbits to comment on.
Overall, I find the commits to be well organized and comments are placed to help our future selves recall what the reasons were for the complicated choices. In particular, the "thread 0" dance you need to do was very interesting.
src/shared/Microsoft.Git.CredentialManager.UI/Controls/DialogWindow.axaml.cs
Show resolved
Hide resolved
|
Not all of the Avalonia UI parts have licenses attached so I cannot use this version. |
Can you elaborate, @BenAigan? All of the code in this repository is covered by the MIT License, even though we do not add license headers on every file. |
|
The package that is holding up my onboarding is https://www.nuget.org/packages/Avalonia.Angle.Windows.Natives/2.1.0.2020091801/ where you can see no license is mentioned, unfortunately the license needs to be specified for us to use it. |
The native UI feature from issue git-ecosystem#136 was completed in git-ecosystem#336 by https://github.com/mjcheetham .
The native UI feature from issue git-ecosystem/git-credential-manager#136 was completed in git-ecosystem/git-credential-manager#336 by https://github.com/mjcheetham .
Introduce a new set of UI helpers based on the Avalonia UI framework for GitHub and Bitbucket!
At the moment the WPF/Windows-only UI helpers continue to exist and be used on Windows. The new helpers are only used on macOS and Linux.
The design has changed slightly in most cases. The GitHub main prompt has been updated the most and hides other auth modes behind a tabbed control (rather than showing them all on one screen as we do on Windows).
(Note to reviewers: this is a large PR.. it's best to review commit by commit!)
GitHub
(WPF-based)
Bitbucket
(WPF-based)
macOS "About" Dialog
Fixes: #136