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

Fix for secondary window's not respecting app's theme. #1881

Merged
merged 4 commits into from
Nov 14, 2023

Conversation

bbonaby
Copy link
Contributor

@bbonaby bbonaby commented Nov 12, 2023

Summary of the pull request

  1. Fixes issue where the secondary windows do not respect the app's theme when its set in the Dev Home settings page. Without this fix the secondary windows continue to keep the Windows system theme, and change when the system theme is changed. For example if Dev Home's theme was set to light theme in the app, but the system theme is dark theme, the secondary window will be in dark theme. Now with this fix, the secondary windows respect the theme set by Dev Home, whether that be light, dark or windows default.
  2. Fixes secondary window's titlebar flickering between the Windows default titlebar and our custom titlebar when the window activates.
  3. Fixes issue where secondary window is not using a background, and thus appearing to be transparent.

here is a video of how the secondary window works with theme changes after this change.

secondary.window.theme.change.fix.mp4

References and relevant issues

Notice in this video at the end, the last time I close and relaunch the Dev Drive window, the minimize, maximize and close buttons, look like they have a light theme foreground. I checked the Dev Home app itself and it looks like an issue with all the windows of the app currently in Dev builds. So, I opened this issue #1882

Detailed description of the pull request / Additional comments

Validation steps performed

tested this locally.

PR checklist

  • Closes #xxx
  • Tests added/passed
  • Documentation updated

@AmelBawa-msft AmelBawa-msft added the Needs-Second Pull request that needs another approval label Nov 14, 2023
@krschau
Copy link
Contributor

krschau commented Nov 14, 2023

Just curious, would UseAppTheme ever be false (besides in the destructor?)

DevDriveWindowContainer.Activate();

// Setting this before the window activates prevents the window from showing up on the screen,
// then moving abruptly to the center.
Copy link
Contributor

@dhoehna dhoehna Nov 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: Consider adding to the comment explaining what needs to be called first.

@AmelBawa-msft
Copy link
Contributor

Just curious, would UseAppTheme ever be false (besides in the destructor?)

@krschau, UseAppTheme is a property I added when creating the SecondaryWindow and is configurable from a custom XAML window. Although it may be uncommon to use it, it simply allows us to create a SecondaryWindow that is theme-independent from the application, and instead follows the system theme.

MyWindow.xaml

<SecondaryWindow
    UseAppTheme="False"
   ....

More details on usage here: SecondaryWindow docs

@EricJohnson327 EricJohnson327 merged commit 33e2f23 into main Nov 14, 2023
4 checks passed
@EricJohnson327 EricJohnson327 deleted the user/bbonaby/fix-for-secondary-window branch November 14, 2023 23:18
@krschau krschau removed the Needs-Second Pull request that needs another approval label Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants