Skip to content

Commit c479457

Browse files
sandy081Copilot
andauthored
sessions: use toolbar separator in editor header (#329387)
* sessions: use toolbar separator in editor header Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * sessions: hide separator without layout actions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent d68e68f commit c479457

4 files changed

Lines changed: 15 additions & 23 deletions

File tree

.github/skills/sessions/SKILL.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ You **must** run these checks before declaring work complete:
238238

239239
- **Single-pane created-session default is Editor-only (Changes editor, detail closed) — the detail is not force-opened by editor activation**: a Changes/file editor becoming active must NOT auto-reveal the docked detail (aux bar). `SinglePaneDetailPanelStrategy._syncForcedDetailTarget` reveals a hidden detail ONLY when it was temporarily hidden by a full-width editor (`_hiddenByFullWidthEditor`), never when it is hidden by the per-session default or an explicit user hide; when the detail is visible it still switches the container (Changes/Files) to match the active editor. **Exception — opening the empty Files placeholder (`EmptyFileEditorInput`) reveals the Files detail** (its content, the Files tree, lives in the aux bar). This is a dedicated `onDidActiveEditorChange` listener in the strategy that reveals the aux bar when the placeholder *becomes the active editor* — NOT reactive logic inside the detail autorun (which re-reads `auxBarVisibleObs`, so it would re-reveal the instant the user hides the detail — bug: "can't hide the details view in the empty file editor at all"). Keying on **active editor** (not `onWillOpenEditor`) is deliberate: the managed auto-ensured Files tab is opened **inactive** as a background tab (`fileTabOptions`), so it never becomes active and never reveals — preserving the Editor-only default — while the `+` Files action and selecting the Files tab both make it active and reveal. Do NOT reveal from the `NewFileTabAction` instead: that misses tab-selection and other activation paths (tried and rejected — "does not work"). The listener is guarded by `isVisible(EDITOR_PART)` (don't reveal the detail alone while the whole side pane is closed, e.g. Scenario C reload) and `!ctx.isRestoringSessionLayout` (a restore-driven activation must not reveal). Because hiding the aux bar fires `onDidChangePartVisibility`, not `onDidActiveEditorChange`, the user's hide sticks while the placeholder stays active. Do NOT reintroduce a `DetailPanelTarget.FilesReveal` in the autorun or an `isEditorPartAutoVisibilitySuppressed()` layout-service API — the active-editor listener needs neither. The reopen default is workbench-layout policy via `_defaultSidePaneState` (`SinglePaneWorkbench` = editor-only); the controller only filters it against available content. When changing this, update the `[single-pane] reveals the Files detail when the empty Files placeholder becomes active` and `[Scenario C]` tests together.
240240
- **Header overflow belongs before trailing layout controls, not promoted into the inline action strip**: keep secondary menu items in the toolbar's `secondary` group so they remain under `...`. Render trailing layout actions from a separate header menu after that toolbar, with a separator between the two action hosts; do not make secondary actions primary just to change the overflow button's position.
241+
- **A toolbar-host separator must follow rendered actions, not menu presence**: a configured layout menu can have zero visible items after context-key evaluation. Enable the secondary toolbar's trailing separator from the layout toolbar's actual item count and rebuild it when that menu changes, or the header can retain an orphan separator.
241242
- **Hide Editor and Toggle Details share visibility, but not enablement**: when an editor supports docked details, keep both actions visible so the trailing layout group stays stable. Gate Hide Editor with `AuxiliaryBarVisibleContext` as a `precondition`, so hiding details disables the action instead of removing it.
242243
- **Code Review switches between right-inline and first-overflow placement**: while `MainEditorAreaVisibleContext` is true, contribute **Run Code Review** to the right header menu in the first primary group (`0_codeReview`). While false, contribute it to `secondary/1_codeReview`, so it is first in `...` with a separator after it; keep both placements mutually exclusive and retain identical applicability gates.
243244
- **Bridge extension editor-title actions into the right header with standard semantics**: `EditorTitleMenuBridgeContribution` mirrors extension-owned `MenuId.EditorTitle` commands and submenus into `Menus.SessionsEditorHeaderSecondary`. Map `navigation` to an inline extension group and every other group to `secondary/extension/<original-group>` so non-navigation actions stay in overflow; forward the active editor's original URI as the menu argument.

src/vs/sessions/LAYOUT.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ The entire third-pane redesign is gated behind the experimental setting `session
278278

279279
- The auxiliary bar is removed from the workbench grid and **docked inside the editor part** (absolutely positioned on the right, below the editor tab strip); the grid's top-right row becomes `Sessions | Editor`, and the editor part spans the editor + detail-panel width.
280280
- The editor group's **tab strip and header-hosted breadcrumbs span the full width**, while the editor content is inset on the right by the detail-panel width via the concrete `EditorPart.setContentRightInset(px)` method (`EditorPart`/`EditorGroupView`; not on the `IEditorPart` interface; `0` = no-op for all other layouts). The detail panel is always docked on the right, so no left margin is needed.
281-
- A **full-width header** sits below the tab bar, spanning the editor content and docked detail panel. `SinglePaneMainEditorPart.getGroupViewOptions()` enables it with `showHeader` and supplies `Menus.SessionsEditorHeaderPrimary`, `Menus.SessionsEditorHeaderSecondary`, and `Menus.SessionsEditorHeaderLayout`. `EditorHeaderControl` owns the header DOM, evaluates those menus, renders their toolbars, and exposes its fixed visible height to `EditorTitleControl`; the title control includes that height in its layout. The header directly contains breadcrumbs followed by one actions container. That actions container owns the primary and secondary action hosts, followed by a separator and the layout-action host for **Toggle Details** and **Hide Editor**. Menu items own their active-editor `when` clauses. `SessionChangesEditor.scopedInstantiationService` only supplies its editor-scoped context for evaluating those clauses; its presence does not control whether the header is created.
281+
- A **full-width header** sits below the tab bar, spanning the editor content and docked detail panel. `SinglePaneMainEditorPart.getGroupViewOptions()` enables it with `showHeader` and supplies `Menus.SessionsEditorHeaderPrimary`, `Menus.SessionsEditorHeaderSecondary`, and `Menus.SessionsEditorHeaderLayout`. `EditorHeaderControl` owns the header DOM, evaluates those menus, renders their toolbars, and exposes its fixed visible height to `EditorTitleControl`; the title control includes that height in its layout. The header directly contains breadcrumbs followed by one actions container. That actions container owns the primary and secondary action hosts, followed by the layout-action host for **Toggle Details** and **Hide Editor**. When the layout toolbar has visible items, the secondary toolbar uses the standard trailing-separator action to divide its actions (including `...`) from those layout actions; layout menu changes rebuild the paired toolbars so an empty layout toolbar never leaves an orphan separator. The layout host supplies the same far-side action gap that an internal separator receives from a single toolbar. The header must not create or style a separate separator element. Menu items own their active-editor `when` clauses. `SessionChangesEditor.scopedInstantiationService` only supplies its editor-scoped context for evaluating those clauses; its presence does not control whether the header is created.
282282
- Text-file breadcrumbs reuse that **same fixed-height header row**. When `IEditorGroupViewOptions.showHeader` is enabled, `EditorTitleControl` creates `BreadcrumbsControl` directly in the header; otherwise it keeps the standard below-tabs placement in the title container. Header padding defines the shared left anchor for breadcrumbs and primary actions, so either starts at the same inset when the other is absent. Header-hosted breadcrumbs lay out at their actual flexed width, accounting for the header padding and sibling actions instead of using the full editor-group width. While the editor area is visible, the empty Files placeholder exposes the active session's first mounted working directory as its resource, so the row shows that Files view root; the breadcrumb model retains an exact workspace-root resource even when ordinary single-root file breadcrumbs omit that root. Detail-only layouts keep the breadcrumb hidden. This is a single-root fallback: multi-root sessions should eventually show a workspace-level breadcrumb that identifies the workspace and exposes all roots instead of presenting the first folder as the whole workspace. Editors without breadcrumbs or applicable menu actions hide the row and report zero header height.
283283
- A vertical **sash** on the left edge of the docked panel resizes it (`DockedAuxiliaryBarController` in `browser/dockedAuxiliaryBarController.ts` owns `layout()` / `_ensureSash()`, created/driven by `SinglePaneMainEditorPart`). The preferred first-open width is 300px; explicit user resizes persist via the part-sizes snapshot. While the panel is visible it clamps to `[220px, editorWidth - 300px]`; dragging the raw sash width down to ~0 hides the docked detail panel, leaving the editor content visible. Temporary width growth from collapsing the sessions list is restored before persistence and must not become the user's detail width.
284284
- Collapsing the sessions list transfers the freed sidebar width to the editor grid node when the editor content is **visible**, and to the **detail panel** (`_dockedAuxiliaryBarWidth`, with the editor node kept equal to it) when the editor content is **hidden** (detail-only). Reopening the sessions list restores the pre-collapse editor-node width / detail width. Keeping the hidden-editor node equal to the detail width ensures the width-based reveal-sync never mistakes a wide detail-only node for a revealed editor.

src/vs/sessions/contrib/editor/browser/media/editorHeader.css

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616

1717
.agent-sessions-workbench.dock-detail-panel .part.editor .editor-group-header-actions {
1818
display: grid;
19-
grid-template-columns: minmax(0, 1fr) auto auto auto;
19+
grid-template-columns: minmax(0, 1fr) auto auto;
2020
align-items: center;
2121
min-width: 0;
2222
flex: 1 1 auto;
@@ -45,19 +45,11 @@
4545

4646
.agent-sessions-workbench.dock-detail-panel .part.editor .editor-group-header-layout-actions {
4747
display: flex;
48-
grid-column: 4;
48+
grid-column: 3;
4949
align-items: center;
5050
flex-shrink: 0;
5151
}
5252

53-
.agent-sessions-workbench.dock-detail-panel .part.editor .editor-group-header-actions-separator {
54-
grid-column: 3;
55-
align-self: stretch;
56-
width: var(--vscode-strokeThickness);
57-
margin: 0 var(--vscode-spacing-size40, 4px);
58-
background-color: var(--vscode-editorGroup-border);
59-
}
60-
6153
.agent-sessions-workbench.dock-detail-panel .part.editor .editor-group-header-secondary-actions > .monaco-toolbar,
6254
.agent-sessions-workbench.dock-detail-panel .part.editor .editor-group-header-secondary-actions > .monaco-toolbar > .monaco-action-bar,
6355
.agent-sessions-workbench.dock-detail-panel .part.editor .editor-group-header-secondary-actions > .monaco-toolbar > .monaco-action-bar > .actions-container {

src/vs/workbench/browser/parts/editor/editorHeaderControl.ts

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ export class EditorHeaderControl extends Disposable {
2020
private readonly actionsContainer: HTMLElement | undefined;
2121
private readonly primaryActionsContainer: HTMLElement | undefined;
2222
private readonly secondaryActionsContainer: HTMLElement | undefined;
23-
private readonly layoutActionsSeparator: HTMLElement | undefined;
2423
private readonly layoutActionsContainer: HTMLElement | undefined;
2524
private primaryActionsToolbar: MenuWorkbenchToolBar | undefined;
2625
private secondaryActionsToolbar: MenuWorkbenchToolBar | undefined;
@@ -55,8 +54,6 @@ export class EditorHeaderControl extends Disposable {
5554
this.actionsContainer = $('.editor-group-header-actions');
5655
this.primaryActionsContainer = append(this.actionsContainer, $('.editor-group-header-primary-actions'));
5756
this.secondaryActionsContainer = append(this.actionsContainer, $('.editor-group-header-secondary-actions'));
58-
this.layoutActionsSeparator = append(this.actionsContainer, $('.editor-group-header-actions-separator'));
59-
this.layoutActionsSeparator.setAttribute('aria-hidden', 'true');
6057
this.layoutActionsContainer = append(this.actionsContainer, $('.editor-group-header-layout-actions'));
6158
this._register(toDisposable(() => this.headerContainer?.remove()));
6259
this._register(this.groupView.onDidActiveEditorChange(() => this.renderActions(true)));
@@ -111,7 +108,7 @@ export class EditorHeaderControl extends Disposable {
111108
}
112109

113110
private updateVisibility(relayout: boolean): void {
114-
if (!this.headerContainer || !this.actionsContainer || !this.primaryActionsContainer || !this.secondaryActionsContainer || !this.layoutActionsContainer || !this.layoutActionsSeparator) {
111+
if (!this.headerContainer || !this.actionsContainer || !this.primaryActionsContainer || !this.secondaryActionsContainer || !this.layoutActionsContainer) {
115112
if (relayout) {
116113
this.groupView.relayout();
117114
}
@@ -125,10 +122,10 @@ export class EditorHeaderControl extends Disposable {
125122
this.secondaryActionsContainer.style.display = hasSecondaryActions ? '' : 'none';
126123
this.secondaryActionsContainer.style.marginLeft = hasPrimaryActions ? 'var(--vscode-spacing-size80, 8px)' : '';
127124
this.layoutActionsContainer.style.display = hasLayoutActions ? '' : 'none';
128-
this.layoutActionsSeparator.style.display = hasSecondaryActions && hasLayoutActions ? '' : 'none';
125+
this.layoutActionsContainer.style.marginLeft = hasSecondaryActions && hasLayoutActions ? 'var(--vscode-spacing-size40, 4px)' : '';
129126
this.actionsContainer.style.display = hasMenuActions ? '' : 'none';
130127
this.actionsContainer.style.flex = this.breadcrumbsVisible ? '0 1 auto' : '1 1 auto';
131-
this.actionsContainer.style.gridTemplateColumns = this.breadcrumbsVisible ? 'auto auto auto auto' : 'minmax(0, 1fr) auto auto auto';
128+
this.actionsContainer.style.gridTemplateColumns = this.breadcrumbsVisible ? 'auto auto auto' : 'minmax(0, 1fr) auto auto';
132129
this.visible = this.breadcrumbsVisible || hasMenuActions;
133130
this.headerContainer.style.display = this.visible ? '' : 'none';
134131
if (relayout) {
@@ -158,8 +155,9 @@ export class EditorHeaderControl extends Disposable {
158155

159156
const store = new DisposableStore();
160157
const scopedInstantiationService = this.groupView.activeEditorPane?.scopedInstantiationService ?? this.instantiationService;
161-
const createToolbarOptions = (ariaLabel: string) => ({
158+
const createToolbarOptions = (ariaLabel: string, trailingSeparator = false) => ({
162159
ariaLabel,
160+
trailingSeparator,
163161
menuOptions: {
164162
arg: EditorResourceAccessor.getOriginalUri(this.groupView.activeEditor, { supportSideBySide: SideBySideEditor.PRIMARY }),
165163
shouldForwardArgs: true,
@@ -171,13 +169,14 @@ export class EditorHeaderControl extends Disposable {
171169
this.primaryActionsToolbar = store.add(scopedInstantiationService.createInstance(MenuWorkbenchToolBar, this.primaryActionsContainer, headerPrimaryMenuId, createToolbarOptions(localize('ariaLabelEditorHeaderPrimaryActions', "Editor primary actions"))));
172170
store.add(this.primaryActionsToolbar.onDidChangeMenuItems(() => this.updateVisibility(true)));
173171
}
174-
if (headerSecondaryMenuId) {
175-
this.secondaryActionsToolbar = store.add(scopedInstantiationService.createInstance(MenuWorkbenchToolBar, this.secondaryActionsContainer, headerSecondaryMenuId, createToolbarOptions(localize('ariaLabelEditorHeaderActions', "Editor actions"))));
176-
store.add(this.secondaryActionsToolbar.onDidChangeMenuItems(() => this.updateVisibility(true)));
177-
}
178172
if (headerLayoutMenuId) {
179173
this.layoutActionsToolbar = store.add(scopedInstantiationService.createInstance(MenuWorkbenchToolBar, this.layoutActionsContainer, headerLayoutMenuId, createToolbarOptions(localize('ariaLabelEditorHeaderLayoutActions', "Editor layout actions"))));
180-
store.add(this.layoutActionsToolbar.onDidChangeMenuItems(() => this.updateVisibility(true)));
174+
store.add(this.layoutActionsToolbar.onDidChangeMenuItems(() => this.renderActions(true)));
175+
}
176+
if (headerSecondaryMenuId) {
177+
const hasLayoutActions = (this.layoutActionsToolbar?.getItemsLength() ?? 0) > 0;
178+
this.secondaryActionsToolbar = store.add(scopedInstantiationService.createInstance(MenuWorkbenchToolBar, this.secondaryActionsContainer, headerSecondaryMenuId, createToolbarOptions(localize('ariaLabelEditorHeaderActions', "Editor actions"), hasLayoutActions)));
179+
store.add(this.secondaryActionsToolbar.onDidChangeMenuItems(() => this.updateVisibility(true)));
181180
}
182181
this.headerActions.value = store;
183182
this.updateVisibility(relayout);

0 commit comments

Comments
 (0)