Skip to content
This repository has been archived by the owner on Sep 2, 2021. It is now read-only.

Fixes several aero and dark shell issues #443

Closed
wants to merge 13 commits into from
Closed

Conversation

JeffryBooher
Copy link
Contributor

@bchintx bchintx self-assigned this May 15, 2014
@MartinMa
Copy link

Looks great! 👍 That was very fast 😃

{
}

cef_buffered_dc(const cef_buffered_dc&)
Copy link
Contributor

Choose a reason for hiding this comment

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

I know you don't "have" to, but could you add a parameter name, like copy? it'll help make it more obvious that it's the Copy Constructor that you're hiding. You've already done so in your other dark shell class definitions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

doing so causes an unreferenced formal parameter warning which is treated as an error

Copy link
Contributor

Choose a reason for hiding this comment

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

oh? bummer. ok. I guess it's fine then.

@bchintx
Copy link
Contributor

bchintx commented May 15, 2014

@JeffryBooher Done with initial review. Overall, changes look fine. Couple of nit-picky comments, and one bigger concern w/ the RGB choices.

@JeffryBooher
Copy link
Contributor Author

@bchintx I changed the color per @larz0 suggestion. Let me know what you think.

@bchintx
Copy link
Contributor

bchintx commented May 16, 2014

@JeffryBooher Thanks for the misc corrections, and, yes, @larz0 's updated inactive frame color looks better. Still, I have two questions/concerns:

(1) is the 1 pel "white" border around the inactive window an intentional change? IMHO, it's a non-standard Window style, and when it's lit up like that, I keep thinking that that window has focus and is in the middle of a window drag operation or something.

(2) I don't notice this in master, but with your branch, if I click-drag the right window border to resize the window, then I'm seeing a flickering white line across the bottom of the main menu area. It only happens while I'm dragging. As soon as I pause or release the mouse button, the white bar is repainted to the background color correctly. However, so long as I'm drag-resizing, the flickering continues.

@JeffryBooher
Copy link
Contributor Author

&larz0 what do you want to do about true white tuxedo border?

@JeffryBooher
Copy link
Contributor Author

@bchintx I didn't notice the dragging issue but will take a look.

@larz0
Copy link
Member

larz0 commented May 16, 2014

@JeffryBooher let's use #3c3f41 for the 1px border.

@JeffryBooher
Copy link
Contributor Author

@bchintx I'm unable to reproduce the flickering line on Windows 8.1 and my fast machine. I can walk you through debugging it if you have time and want to fix it.

@bchintx
Copy link
Contributor

bchintx commented May 19, 2014

Confirmed that latest changes fix:

  • the menu bar flickering that I reported above when resizing the window frame horizontally
  • the menu visibility issue reported in #6801 when changing the PaddedBorderWidth
  • not seeing any flickering when moving around the window #7222

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants