search

microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!
MIT License
95.39k stars 8.3k forks source link

Theme changes don't work correctly after opening the About box #12093

Open j4james opened 2 years ago

j4james commented 2 years ago

Windows Terminal version

1.12.3472.0

Windows build number

10.0.19041.1415

Other Software

No response

Steps to reproduce

  1. Start Windows Terminal (don't just use an existing session).
  2. Open the Settings UI, and go to the Appearance section.
  3. Set the Theme to Light, and click Save.
  4. Without closing the settings, open the About box, and close it again.
  5. Now switch the Theme to Dark, and click Save.

Expected Behavior

The UI colors should change to the dark theme.

Actual Behavior

Most of the UI remains using the light theme, but some elements appear to be updated in such a way that they become invisible.

image

This breaks in a similar way when switching from dark to light, and it's also not only the About box that triggers it - I think any popup dialog will do (e.g. the "Do you want to close all tabs?" warning has the same effect).

If you open the About box again, it temporarily "fixes" the problem (i.e. it refreshes the colors correctly), but that only last until you try and change the theme again.

j4james commented 2 years ago

Btw, if this is something that's fixed in Windows 11, feel free to close it. I only noticed while testing the PR I was going to submit for #1230, but if there are going to be a whole lot of these issues specific to Windows 10, I'm happy to just ditch that PR and accept that Windows 10 won't handle themes very well.

zadjii-msft commented 2 years ago

holy butts that's bad, definitely not fixed in Windows 11. I suspect our hack for "walk up the UI tree and change the resources manually" doesn't totally work great with the SUI. The place to investigate this would be AppLogic::ShowDialog. This is also firmly in BODGY territory, and considering there is a workaround ("just reopen the about dialog"), then this isn't the biggest issue.

zadjii-msft commented 2 years ago

We may want to just run the themeingLambda whenever we reload the settings. IDK if that would work or not though.

j4james commented 2 years ago

Yeah, I was looking at the AppLogic::ShowDialog implementation, and I tried running something similar to the themeingLambda thing in the IslandWindow::OnApplicationThemeChanged handler, but I couldn't get it to work. I thought at the time that my attempted fix for #1230 was to blame, and it was becoming a lot more effort than I had anticipated for a "quick fix", so I gave up on it pretty quickly. If it's not going to be fixed in Windows 11, though, I may take another stab at it when I have more time.

zadjii-msft commented 1 year ago

Heck, this still isn't fixed in 1.17, even after all the other changes to theming.