Skip to content
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

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

Open
j4james opened this issue Jan 5, 2022 · 5 comments
Open

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

j4james opened this issue Jan 5, 2022 · 5 comments
Labels
Area-Settings UI Anything specific to the SUI Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Milestone

Comments

@j4james
Copy link
Collaborator

j4james commented Jan 5, 2022

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.

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Jan 5, 2022
@j4james
Copy link
Collaborator Author

j4james commented Jan 5, 2022

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
Copy link
Member

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 zadjii-msft added Area-Settings UI Anything specific to the SUI Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Jan 5, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jan 5, 2022
@zadjii-msft zadjii-msft added this to the Backlog milestone Jan 5, 2022
@zadjii-msft zadjii-msft added the Help Wanted We encourage anyone to jump in on these. label Jan 5, 2022
@zadjii-msft
Copy link
Member

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

@j4james
Copy link
Collaborator Author

j4james commented Jan 5, 2022

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 zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Feb 17, 2022
@zadjii-msft
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings UI Anything specific to the SUI Help Wanted We encourage anyone to jump in on these. Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

2 participants