-
Notifications
You must be signed in to change notification settings - Fork 752
Add a themed dialog #2276
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
Add a themed dialog #2276
Conversation
Theming isn't easy for the system message box and requires using undocumented APIs. To get theming, including a simple themed message box that roughly follows the existing layout. Move the project to the standard SDK which enables using source generators. The existing SDK was a hack for building before WPF and WinForms were officially released on .NET Core. Push the C# version to 12 for collections expressions, default parameters for lambdas, etc. Add CsWin32 for PInvoke source generation. Add PInvokes for getting the modern shell icons for the new dialog. System.Drawing has the new Icons in .NET, but we're still targetting 4.6.2. (Note that some PInvokes have to still be manually defined because we're targetting AnyCPU. Some headers in Windows are defined with nonstandard packing, which makes the SDK generate metadata that is platform specific. For safety they don't generate any of these and they must be manually evaluated. The manual ones I added come from System.Drawing. If we targetted specific architectures this wouldn't be necessary.) If we're able to move to .NET Core in the future we can get more explicit, coherent theming support through WPF or MAUI.
brianrob
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.
LGTM. Thanks @JeremyKuhne! Just to confirm - this does not use any undocumented APIs, right? I think I read from your description that the solution you used to avoid undocumented APIs is to create your own dialog, but just want to make sure.
| <PackageReference Include="Microsoft.IdentityModel.Tokens" GeneratePathProperty="true" /> | ||
| <PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" GeneratePathProperty="true" /> | ||
| <PackageReference Include="Microsoft.Web.WebView2" GeneratePathProperty="true" /> | ||
| <PackageReference Include="Microsoft.Windows.CsWin32"> |
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.
Confirming - do we need any of these artifacts at runtime, or just at compile time?
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 is a development dependency and the PrivateAssets entry controls this.
You might be using a dependency purely as a development harness and might not want to expose that to projects that will consume your package. In this scenario, you can use the PrivateAssets metadata to control this behavior.
The Microsoft.Windows.CsWin32 package takes the information from NativeMethods.txt and genrates the PInvoke data.
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.
Thanks @MagicAndre1981. That was my assumption, but just wanted to be sure as I've not used it before.
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.
Thanks @MagicAndre1981. That was my assumption, but just wanted to be sure as I've not used it before.
after this is merged the currently used DLLimport code at several places should be also moved to generated code by CsWin32
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.
Yep, this just source generates the interop from the Windows published metadata
Yeah, I spun a custom one to avoid undocumented features. |
brianrob
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.
LGTM. Thank you @JeremyKuhne!
Theming isn't easy for the system message box and requires using undocumented APIs. To get theming, including a simple themed message box that roughly follows the existing layout.
Move the project to the standard SDK which enables using source generators. The existing SDK was a hack for building before WPF and WinForms were officially released on .NET Core.
Push the C# version to 12 for collections expressions, default parameters for lambdas, etc.
Add CsWin32 for PInvoke source generation. Add PInvokes for getting the modern shell icons for the new dialog. System.Drawing has the new Icons in .NET, but we're still targetting 4.6.2. (Note that some PInvokes have to still be manually defined because we're targetting AnyCPU. Some headers in Windows are defined with nonstandard packing, which makes the SDK generate metadata that is platform specific. For safety they don't generate any of these and they must be manually evaluated. The manual ones I added come from System.Drawing. If we targetted specific architectures this wouldn't be necessary.)
If we're able to move to .NET Core in the future we can get more explicit, coherent theming support through WPF or MAUI.
Existing:
Themed:
(The light version is not as fully realized, as the theme for that wasn't in the current sources, I did the bare minimum with the intent of following up on the theme sources at some point.)
(The preset dialog doesn't have a real preset string in it, just ran it in the about menu with version text to make sure it was working correctly.)