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.

Sign up for GitHub

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

Jump to bottom

ExplorerViewlet triggers two layouts on load #164188

Closed
mjbvz opened this issue Oct 20, 2022 · 2 comments
Closed

ExplorerViewlet triggers two layouts on load #164188

mjbvz opened this issue Oct 20, 2022 · 2 comments
Assignees
@bpasero @lramos15 @sbatten
Labels
debt Code quality issues file-explorer Explorer widget issues insiders-released Patch has been released in VS Code Insiders perf workbench-views Workbench view issues
Milestone
May 2023

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Oct 20, 2022

While profiling the startup of VS Code with the open editors view visible, I noticed that the ExplorerViewletViewsContribution ends up firing two events about updating view visibility:

Screen Shot 2022-10-20 at 2 04 55 PM

Each of these takes ~25ms to complete

  1. registerViews
  2. updateOpenEditorsVisibility

I suspect that the two events can be batched together so that we avoid doing the costly re-layout twice
@mjbvz mjbvz added the perf label Oct 20, 2022
@mjbvz mjbvz changed the title ExplorerViewlet triggers two layouts ExplorerViewlet triggers two layouts on load Oct 20, 2022
@sbatten sbatten added debt Code quality issues workbench-views Workbench view issues labels Oct 21, 2022
@sbatten sbatten added this to the On Deck milestone Oct 21, 2022
@bpasero
Copy link
Member

bpasero commented Apr 29, 2023 •
edited

@isidorn I was looking into this and found the following OpenEditorsVisibleContext context key around open editors:

vscode/src/vs/workbench/contrib/files/common/files.ts

Line 52 in 34cba75

export const OpenEditorsVisibleContext = new RawContextKey<boolean>('openEditorsVisible', false, { type: 'boolean', description: localize('openEditorsVisible', "True when the OPEN EDITORS view is visible.") });

It is false by default but then will in most cases immediately be set to true based on this:

vscode/src/vs/workbench/contrib/files/browser/explorerViewlet.ts

Lines 166 to 168 in 34cba75

private updateOpenEditorsVisibility(): void {
this.openEditorsVisibleContextKey.set(this.workspaceContextService.getWorkbenchState() === WorkbenchState.EMPTY || this.configurationService.getValue('explorer.openEditors.visible') !== 0);
}

Unless a user has explorer.openEditors.visible set to 0, which is not the default.

To fix this issue I think flipping the default would be sufficient. However, I have general questions around this and maybe you recall: why do we even bother to hide the "Open Editors" view from the right click menu conditionally based on this context key? And isn't the name of the context key misleading? I would assume openEditorsVisible is about visibility of the view in the explorer viewlet but not about if I can pick it to show or not.

My 2 cents (without knowing the history of this) is to remove the context key and simply always to show the "Open Editors" view when you select it here but have it hidden by default:

image

We then probably would have to change explorer.openEditors.visible to not allow for 0 as value, but since the default is to hide it anyway, I do not even think users that have this configured would notice.

@isidorn
Copy link
Contributor

isidorn commented May 5, 2023

@bpasero sorry but I do not recall the details. My feeling is that this was done specially for the Open Editors before we had all the nice view visibility mechanisms in VS Code.
Any change here I am fully supportive off. Thus as you suggest let's remove the context key. And let's change explorer.openEditors.visible to not support 0 👏
It is hidden by default so we are good.

fyi @lramos15 who now owns the Open Editors view.

bpasero added a commit that referenced this issue May 5, 2023
@bpasero
ExplorerViewlet triggers two layouts on load (fix #164188)
7778906
@lramos15 lramos15 added the file-explorer Explorer widget issues label May 9, 2023
@lramos15 lramos15 modified the milestones: On Deck, May 2023 May 9, 2023
@lramos15 lramos15 self-assigned this May 9, 2023
@bpasero bpasero closed this as completed in 8ab2e97 May 9, 2023
@VSCodeTriageBot VSCodeTriageBot added the unreleased Patch has not yet been released in VS Code Insiders label May 9, 2023
@bpasero bpasero self-assigned this May 9, 2023
@VSCodeTriageBot VSCodeTriageBot added insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels May 10, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@bpasero bpasero

@lramos15 lramos15

@sbatten sbatten

Labels
debt Code quality issues file-explorer Explorer widget issues insiders-released Patch has been released in VS Code Insiders perf workbench-views Workbench view issues
Projects
None yet
Milestone
May 2023
Development

No branches or pull requests
6 participants
@bpasero @isidorn @lramos15 @sbatten @mjbvz @VSCodeTriageBot