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

[PTRun]Bring back acrylic and proper fix to title bar accent showing #33458

Merged
merged 5 commits into from
Jun 21, 2024

Conversation

jaimecbernardo
Copy link
Collaborator

Summary of the Pull Request

Recent UI changes in PowerToys Run made users unhappy with the looks of PowerToys Run. And those changes didn't properly fix the issues they were trying to solve.

This reverts #33046 and #32118
It brings back Acrylic backdrop.
It removes the opaque search bar background that was disliked by users.
It includes the fix in wpf ui for the title bar accent showing through: https://github.com/lepoco/wpfui/pull/1122/files#diff-196b404f4db09632665ef546da6c8e57302b2f3e3d082eb4b5c295ae3482d94a
If fixes PowerToys Run becoming opaque after a theme change too.

It also fixes the Wox Plugin DWM Attributes enum, since the values were wrong. Used this as a base to update the values: https://groups.google.com/g/golang-codereviews/c/C06XsvvpNPo?pli=1

PR Checklist

Validation Steps Performed

Set up the accent color to show in windows title bar:
image

After that, verified the tile bar didn't appear in PowerToys Run, as specified in #30206

Also changed theme a couple of times to verify it still looked good with some transparency.

WindowCornerPreference = 33,
BorderColor = 34,
CaptionColor = 35,
TextColor = 36,
VisibleFrameBorderThickness,
Copy link
Collaborator

Choose a reason for hiding this comment

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

number is missing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right! Thank you! Fixed!

Comment on lines +507 to +508
Cloak = 13,
Cloaked = 14,
Copy link
Collaborator

@htcfreek htcfreek Jun 20, 2024

Choose a reason for hiding this comment

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

Do we use a different enum in ww plugin? otherwise please verify that no dummy windows for uwp apps shown in results.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ww plugin just puts 12 directly 😆

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For the cloak ones, the number is the same :) It starts with 1 and goes up is the default behavior, so the number will be kept.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was trying to use CaptionColor and that's when I saw this was wrong.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ww plugin just puts 12 directly

No. See Components\Window.cs:L306

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I know :) Cloak and Cloaked get the same value due to the default +1 logic in enums. This just makes it explicit since not every value follows that logic

Copy link
Collaborator

@htcfreek htcfreek Jun 20, 2024

Choose a reason for hiding this comment

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

My mistake. Thought the counter changes +1 for the first enum item. But the first two have a very similar name. 🙃

BorderColor = 34,
CaptionColor = 35,
TextColor = 36,
VisibleFrameBorderThickness = 37,
Last,
Copy link
Collaborator

Choose a reason for hiding this comment

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

=38?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let's leave that one without a number, so it's always +1 then the previous one. I think that's there just so we have a "stop" if we want to go in a loop 🤔 That was something that was usual to do in C code.

Copy link
Collaborator

@stefansjfw stefansjfw left a comment

Choose a reason for hiding this comment

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

works great! nice work!

@jaimecbernardo jaimecbernardo merged commit 9509d7c into main Jun 21, 2024
15 checks passed
jaimecbernardo added a commit that referenced this pull request Jun 25, 2024
After #33458 , that fix
crashes on Windows 10, where the caption color attribute is not
supported.

This PR disables the fix on Windows 10, since it's not even needed there
actually.
@crutkas crutkas deleted the dev/jaime/fix-ptrun-mica-issues branch July 22, 2024 20:44
@jaimecbernardo jaimecbernardo added this to the PowerToys 0.82 milestone Jul 23, 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.

3 participants