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

Window control overlay does not dim when modal custom dialog shows #159671

Open
bpasero opened this issue Aug 31, 2022 · 6 comments · May be fixed by #236080
Open

Window control overlay does not dim when modal custom dialog shows #159671

bpasero opened this issue Aug 31, 2022 · 6 comments · May be fixed by #236080
Assignees
Labels
dialogs Issues with native and custom dialogs help wanted Issues identified as good community contribution opportunities titlebar VS Code main title bar issues under-discussion Issue is under discussion for relevance, priority, approach windows VS Code on Windows issues workbench-os-integration Native OS integration issues
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented Aug 31, 2022

When you open a folder on a fresh user data dir, you see a modal workspace trust custom dialog that dims the background but not the new WCO:

image

@bpasero bpasero added windows VS Code on Windows issues workbench-os-integration Native OS integration issues titlebar VS Code main title bar issues labels Aug 31, 2022
@deepak1556
Copy link
Collaborator

Nice catch! colors can for the overlay can be controlled with https://github.com/electron/electron/blob/main/docs/api/browser-window.md#winsettitlebaroverlayoptions-windows.

@deepak1556 deepak1556 removed their assignment Aug 31, 2022
@bpasero bpasero added the dialogs Issues with native and custom dialogs label Aug 31, 2022
@bpasero
Copy link
Member Author

bpasero commented Aug 31, 2022

Yeah, its a bit of a layer break dance, we probably would have to call that method from here:

private async doShow(type: 'none' | 'info' | 'error' | 'question' | 'warning' | 'pending' | undefined, message: string, buttons?: string[], detail?: string, cancelId?: number, checkbox?: ICheckbox, inputs?: IInput[], customOptions?: ICustomDialogOptions): Promise<IDialogResult> {

When a custom dialog opens, but since the layer is wrong we need a specific dialog handler for custom dialogs in electron-sandbox.

Note that there is even a user setting to enable custom dialogs for all dialogs on desktop.

@bpasero
Copy link
Member Author

bpasero commented Sep 1, 2022

Actually it could easily be added where we process dialogs from the desktop app. We know here if a custom or native dialog opens:

let result: IDialogResult | undefined = undefined;

We even have access to the native host service, the only challenge is to find the right color for the WCO. We seem to be adding a large div over the workbench with rgba(0, 0, 0, 0.3) and would have to compute the new title background and foreground color that this brings.

@bpasero bpasero added the help wanted Issues identified as good community contribution opportunities label Sep 1, 2022
@bpasero
Copy link
Member Author

bpasero commented Sep 1, 2022

If someone wants to tackle this, opening for help. Note, when you run out of sources, you have to force enable WCO here:

export function useWindowControlsOverlay(configurationService: IConfigurationService, environmentService: IEnvironmentService): boolean {

And some pseudo code to update the WCO is:

this.nativeHostService.updateWindowControls({
	backgroundColor: withNullAsUndefined(this.getColor(TITLE_BAR_ACTIVE_BACKGROUND, color => color.blend(new Color(new RGBA(0, 0, 0, 0.3))))),
	foregroundColor: withNullAsUndefined(this.getColor(TITLE_BAR_ACTIVE_FOREGROUND, color => color.blend(new Color(new RGBA(0, 0, 0, 0.3))))
});

@sbatten
Copy link
Member

sbatten commented Oct 17, 2022

Just throwing another perspective here: the current non-dimmed controls are an accurate representation of the fact that these buttons are still functional, unlike the rest of the title bar.

@yume-chan
Copy link
Contributor

yume-chan commented Oct 17, 2022

image

Windows Terminal will dim window control buttons when popups are shown (they are still functional). I only know it's now Win32 caption buttons, but not sure whether it's from WinUI or Windows Terminal itself.

Traditionally, when model dialogs are visible, the window behind can't be interacted. A recent exception is web browsers, but that's due to being abused by some malicious websites. I personally don't mind if they can still work when dimmed, but I prefer the dimming to provide a more consistent look, and make users focus more on the model dialog currently shown.

@rzhao271 rzhao271 added the under-discussion Issue is under discussion for relevance, priority, approach label Dec 5, 2022
@rzhao271 rzhao271 added this to the Backlog milestone Dec 6, 2022
@sbatten sbatten assigned benibenj and unassigned sbatten Sep 11, 2024
@benibenj benibenj linked a pull request Dec 13, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialogs Issues with native and custom dialogs help wanted Issues identified as good community contribution opportunities titlebar VS Code main title bar issues under-discussion Issue is under discussion for relevance, priority, approach windows VS Code on Windows issues workbench-os-integration Native OS integration issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants