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

Make sure caption controls "dim out" when window loses NC focus #10401

Closed
wants to merge 3 commits into from

Conversation

AnuthaDev
Copy link
Contributor

Smol Code

Summary of the Pull Request

Make the caption controls "dim out" when window loses focus

References

https://docs.microsoft.com/en-us/windows/win32/winmsg/wm-ncactivate
#5881
#3025
#7015

PR Checklist

  • Applies to Epic: Fix remaining issues with non-client drawing #1625
  • CLA signed. If not, go over here and sign the CLA
  • Tests added/passed
  • Documentation updated. If checked, please file a pull request on our docs repo and link it here: #xxx
  • Schema updated.
  • I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

Added a handler for WM_NCACTIVATE to propagate the focus change to the Titlebar Control.

The unfocused color is set to ForegroundBaseLow

Validation Steps Performed

None

(Potato™ PC, no compile)

@AnuthaDev
Copy link
Contributor Author

Phone: 1, Code-Format Checker: 0

@AnuthaDev
Copy link
Contributor Author

BTW, I would love some guidance on how to compile just the minimum projects necessary for testing UI changes (preferably using VS)

@AnuthaDev
Copy link
Contributor Author

अरे कोई रिव्यू कर दो

Comment on lines +10 to +11
WindowVisualStateIconified,
WindowVisualStateFocused,
Copy link
Member

Choose a reason for hiding this comment

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

I'm still pretty worried about including these in the list of window states. I think they need to be a separate enum. Here's why:

The window is always aware of its current visual state, right...

What happens if you maximize it, then un-focus it, and then focus it again?

The visual state, according to the window, is no longer "maximized." It's "focused." It totally forgot that it was maximized!

We need to track the focus state separately from whether the window is normal, maximized or iconified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I thought I had fixed that🤔, nvm, doing it asap

@ghost ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jun 28, 2021
@zadjii-msft zadjii-msft added the Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something label Jul 7, 2021
@zadjii-msft
Copy link
Member

BTW, I would love some guidance on how to compile just the minimum projects necessary for testing UI changes (preferably using VS)

To test these changes, you should be building and running "CascadiaPackage" in VS. That's the project that's the complete Terminal package.

@ghost ghost added the No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. label Jul 14, 2021
@ghost
Copy link

ghost commented Jul 14, 2021

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants