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

Adopt WCO for Windows #147947

Merged
merged 13 commits into from
May 2, 2022
Merged

Adopt WCO for Windows #147947

merged 13 commits into from
May 2, 2022

Conversation

sbatten
Copy link
Member

@sbatten sbatten commented Apr 22, 2022

This PR
fixes #136132
fixes #127449

cc @deepak1556

@sbatten sbatten requested a review from bpasero April 22, 2022 19:18
@sbatten sbatten self-assigned this Apr 22, 2022
@sbatten sbatten added titlebar VS Code main title bar issues windows VS Code on Windows issues windows 11 VS Code on Windows 11 issues labels Apr 22, 2022
Copy link
Member

@bpasero bpasero left a comment

Choose a reason for hiding this comment

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

Pretty cool 👍

I see a bit of flicker when updating the theme, where first the WCO changes and then the workbench. Is there a way to set the WCO transparent so that we do not have to change its color and simply get the color of the title bar without having to update the WCO color?

I also see a border below the WCO when window is maximized:

image

Clicking minimize button and then restore, does not clear the button hover style for me:

image

PS: I did not review he CSS changes or changes in titlebarPart.ts because you are the expert in those by now.

@sbatten
Copy link
Member Author

sbatten commented Apr 25, 2022

@bpasero responded to feedback and made some changes.

Is there a way to set the WCO transparent so that we do not have to change its color and simply get the color of the title bar without having to update the WCO color?

Unfortunately no, a transparent control actually shows a native color (that pale blue from windows aero days)

I also see a border below the WCO when window is maximized

I wasn't able to repro this one. Are you in a VM?

Clicking minimize button and then restore, does not clear the button hover style for me

I can repro this one @deepak1556

@sbatten sbatten requested a review from bpasero April 25, 2022 17:29
@bpasero
Copy link
Member

bpasero commented Apr 26, 2022

Yes I am on a VM and typically have zoom level of 1 in VSCode.

@sbatten sbatten added this to the May 2022 milestone Apr 27, 2022
.monaco-workbench.web .part.titlebar > .titlebar-container,
.monaco-workbench.windows .part.titlebar > .titlebar-container,
.monaco-workbench.linux .part.titlebar > .titlebar-container {
.monaco-workbench.web .part.titlebar>.titlebar-container,
Copy link
Member

Choose a reason for hiding this comment

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

Any reason you changed the style here? We typically have whitespace around >

Copy link
Member Author

Choose a reason for hiding this comment

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

some formatter I have has been doing this on occasion. and I just recently found that I had the setting which only applies this to changed regions

Copy link
Member

Choose a reason for hiding this comment

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

Can we revert that as part of sbatten/outdoor-mosquito maybe? I prefer the old style.

display: flex;
.monaco-workbench.mac .part.titlebar>.window-controls-container {
width: 70px;
height: env(titlebar-area-width, 28px);
Copy link
Member

@bpasero bpasero Dec 19, 2022

Choose a reason for hiding this comment

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

I wonder why this was added? Are we using WCO on macOS?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
titlebar VS Code main title bar issues windows VS Code on Windows issues windows 11 VS Code on Windows 11 issues
Projects
None yet
3 participants