search

CommunityToolkit / Microsoft.Toolkit.Win32

ARCHIVE - This repository contained XAML Islands wrapper controls and tooling for XAML Islands with WinUI 2, see readme for more info about XAML Islands with WinUI 3 and the WindowsAppSDK.
https://aka.ms/windowsappsdk
Other
383 stars 89 forks source link

XAML Island doesn't respond to Windows app themes changes (Dark, Light) at run-time #228

Closed Palatis closed 4 years ago

Palatis commented 4 years ago

Describe the bug

I have no idea if this issue should go here (or to Microsoft.Toolkit.Win32), but since the source code is here, I post it here.

I'm using XamlIsland in WPF to host my UWP custom control. However it occasionally crash with InvalidOperationException says something like "Cannot create new view because main window is not created." (the actual message is "無法建立新的檢視，因為尚未建立主視窗", this is just a translation.) with a stacktrace:

   at Windows.ApplicationModel.Core.CoreApplication.get_MainView()
   at Microsoft.Toolkit.Uwp.UI.Helpers.ThemeListener.Settings_ColorValuesChanged(UISettings sender, Object args) in D:\a\1\s\Microsoft.Toolkit.Uwp.UI\Helpers\ThemeListener.cs:line 81
   at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__139_1(Object state)
   at System.Threading.QueueUserWorkItemCallbackDefaultContext.Execute()
   at System.Threading.ThreadPoolWorkQueue.Dispatch()
   at System.Threading._ThreadPoolWaitCallback.PerformWaitCallback()

I haven't switch the theme, tho... I just leave it there running for several minute and it crashes.

Steps to Reproduce

Steps to reproduce the behavior:

  1. follow the WPF XamlIsland tutorial on Microsoft website
  2. run the program and wait

Expected behavior

no problem

Screenshots

probably don't need one...

Environment

NuGet Package(s):

<ItemGroup>
  <PackageReference Include="Microsoft.Toolkit.UI.XamlHost" Version="6.0.0" />
  <PackageReference Include="Microsoft.Toolkit.Wpf.UI.XamlHost" Version="6.0.0" />
</ItemGroup>

Package Version(s): 6.0.0

Windows 10 Build Number:

App min and target version:

Device form factor:

Visual Studio

Additional context

currently no

michael-hawker commented 4 years ago

Thanks @Palatis for the report, moving it to the Win32 for initial triage as it's occurring with XAML Islands, though see now that the stack does have a toolkit component. Not sure where that reference is from.

@marb2000 @ocalvo @azchohfi thoughts?

ncarrillo commented 4 years ago

I'm also running into this. I don't think we can call into CoreView in the context of a XAML Island.

For me, this was occurring because RadialGauge has a static ThemeResource field which sets up the event subscription to ColorValuesChanged.

I think the effect of this is that any XAML Island application using the Windows Community Toolkit will crash on a color change.

I'll leave the fix up to you all, but I do have a workaround that I came up with to get my own app to stop crashing :-):

I run this in my app's main page Loaded event:

`

        var radialGaugeType = typeof(RadialGauge);
        var themeListener = radialGaugeType
            .GetField("ThemeListener", BindingFlags.NonPublic | BindingFlags.Static)
            .GetValue(null) as ThemeListener;
        themeListener.Dispose();

`

Calling Dispose unregisters the event subscription to ColorValuesChanged.

It won't help anyone who wishes to use RadialGauge or MarkdownTextBlock directly though.

marb2000 commented 4 years ago

Doing the repro steps and I can't repro the bug...

Palatis commented 4 years ago

Doing the repro steps and I can't repro the bug...

maybe try switch system theme in the control panel? this is really theme related. it crashes on me everytime i switch theme.

sometimes my Windows 10 switch theme from dark to light and immediately back to dark randomly... i dunno why... but it does......

michael-hawker commented 4 years ago

@azchohfi can you look at the ThemeListener helper here in the toolkit when you have time? Is there a different model we should be using that's more XAML Islands friendly due to it's use of CoreWindow?

marb2000 commented 4 years ago

In UWP, the Application's RequestedTheme only changes in the constructor. So no sure what this helper is listening...

Palatis commented 4 years ago

@marb2000 so what actually happens if the user switch theme from the control panel? my other UWP apps actually switch theme (like from light to dark or vise versa).

marb2000 commented 4 years ago

Gotcha! I get the scenario. now. You are listening the Window activation event to figure out whether the Theme has changed and then raise the event, am I right?

marb2000 commented 4 years ago

@Palatis can you attach/link a repro project where we can see the "Cannot create new view because main window is not created." exception, please?

Palatis commented 4 years ago

Well you really just have to use the XamlIsland tutorial project and switch system theme... Because that is what I did when I get the issue...

marb2000 commented 4 years ago

OK. Let me change the title of the bug so it can be addressed properly . Bug description: XAML Islands doesn't respond Windows app themes changes (Dark, Light) dynamically, only at app start.

Repro:

  1. Run a XAML Islands app, for instance the WPF XamlIsland tutorial.
  2. Switch the Color of the default app mode in Windows's settings

Expected: The content of the XAML Island changes. Observed: The context of the XAML Island doesn't change.

ncarrillo commented 4 years ago

This is crashing my XAML Islands application referencing the Windows Community Toolkit when changing the accent color on line 81 of ThemeListener.cs due to RunAsync:

CoreApplication.MainView?.CoreWindow?.Dispatcher?.RunAsync

Shouldn't this be using a DispatcherQueue instead in a XAML Island context? That's what I've been using elsewhere to run code on the UI Thread.

marb2000 commented 4 years ago

@nc4rrillo Yes, using DispatcherQueue is the recommendation for UWP and XAML Islands given it works everywhere. If this doesn;t fix your bug, please, open a new bug within the repo steps, adding a project that fails is really helpful. Thanks!

About the bug description: Unfortunately, we can't fix this bug in the OS, but we can do it in WinUI 3 😊

ncarrillo commented 4 years ago

@marb2000 thank you! The fix for this will have to come specifically from WCT in this instance, though I’d be happy to offer up a PR.

cc: @michael-hawker

michael-hawker commented 4 years ago

@nc4rrillo Thanks! I don't know if we have another issue tracking this specifically in the WCT repo, this issue may have started there, but I moved here originally due to the XAML Islands context, but there is indeed an issue in the toolkit. If you don't mind filing an issue there for tracking with the specific details you've called out from ThemeListener, that'd be great for tracking!

I know my colleague @azchohfi has also been looking into these scenarios across the toolkit as well, so just coordinate with him in case he's already started something; otherwise, always happy for another hand and a PR!