Skip to content

Conversation

@sandy081
Copy link
Member

No description provided.

Copilot AI review requested due to automatic review settings January 22, 2026 22:00
@sandy081 sandy081 enabled auto-merge (squash) January 22, 2026 22:00
@sandy081 sandy081 self-assigned this Jan 22, 2026
@vs-code-engineering
Copy link

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@bpasero

Matched files:

  • src/vs/workbench/browser/layout.ts

@vs-code-engineering vs-code-engineering bot added this to the January 2026 milestone Jan 22, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates workbench layout behavior so that when the secondary sidebar (auxiliary bar) is configured to open maximized, the UI can collapse the otherwise-empty editor area when editors are closed and other parts are hidden.

Changes:

  • Adds a restoreMaximizedAuxiliaryBar helper that re-maximizes the auxiliary bar when there are no visible editors and both sidebar and panel are hidden.
  • Hooks the restore logic into editor visibility changes and general part-visibility change events.
  • Introduces an auxiliaryBarOpensMaximized() helper to detect when the auxiliary bar should open maximized based on configuration.

}

private auxiliaryBarOpensMaximized(): boolean {
return this.configurationService.getValue(WorkbenchLayoutSettings.AUXILIARYBAR_DEFAULT_VISIBILITY) === 'maximized';
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

auxiliaryBarOpensMaximized() only checks for workbench.secondarySideBar.defaultVisibility === 'maximized', but the setting also supports 'maximizedInWorkspace' (and there is also workbench.secondarySideBar.restoreMaximized). As written, the new restore path won’t trigger for 'maximizedInWorkspace' in non-empty workspaces, which looks inconsistent with applyOverrides() (layout.ts:2971-2981). Consider aligning the condition with the existing override logic, including WorkbenchState handling.

Suggested change
return this.configurationService.getValue(WorkbenchLayoutSettings.AUXILIARYBAR_DEFAULT_VISIBILITY) === 'maximized';
const visibility = this.configurationService.getValue<string>(WorkbenchLayoutSettings.AUXILIARYBAR_DEFAULT_VISIBILITY);
if (visibility === 'maximized') {
return true;
}
if (visibility === 'maximizedInWorkspace') {
const workbenchState = this.contextService.getWorkbenchState();
if (workbenchState === WorkbenchState.FOLDER || workbenchState === WorkbenchState.WORKSPACE) {
return true;
}
}
return false;

Copilot uses AI. Check for mistakes.
}));
this._register(this.editorGroupService.mainPart.onDidActivateGroup(showEditorIfHidden));

// Restore maximized auxiliary bar when sidebar or panel visibility changes
Copy link

Copilot AI Jan 22, 2026

Choose a reason for hiding this comment

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

The comment says this listener is for “sidebar or panel visibility changes”, but onDidChangePartVisibility is a void event fired for any part visibility change (see layout.ts:159-1630). Either update the comment to match reality, or gate the restore logic so it only runs when sidebar/panel hidden state actually changed (e.g., track previous values).

Suggested change
// Restore maximized auxiliary bar when sidebar or panel visibility changes
// Restore maximized auxiliary bar when any part visibility changes

Copilot uses AI. Check for mistakes.
@sandy081 sandy081 merged commit 108524f into main Jan 22, 2026
27 of 28 checks passed
@sandy081 sandy081 deleted the sandy081/principal-catshark branch January 22, 2026 22:20
eleanorjboyd pushed a commit to eleanorjboyd/vscode that referenced this pull request Jan 23, 2026
@bpasero
Copy link
Member

bpasero commented Jan 23, 2026

@sandy081 thanks for jumping onto this, I wanted to look into it myself. Seeing your change makes me wonder if it uses the right condition for whether to restore the 2nd sidebar as maximised (see also the Copilot comments): rather than probing on the setting, wouldn't it make more sense to restore maximised state simply if the 2nd sidebar was maximised before? In other words:

  • lets say, 2nd sidebar is maximised (e.g. the user did this, or it opened maximised after startup), we store this state in the runtime layout info
  • an editor, or sidebar or panel opens, so 2nd sidebar restores, but not because the user wanted to restore
  • now when editor becomes empty, or sidebar/panel hides, we should go back to maximised mode

Arguably the same applies for the panel, but I would say we can leave todays behaviour there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants