diff --git a/src/vs/sessions/browser/menus.ts b/src/vs/sessions/browser/menus.ts index 1e7f84f1f6cac..d056772a88415 100644 --- a/src/vs/sessions/browser/menus.ts +++ b/src/vs/sessions/browser/menus.ts @@ -52,7 +52,6 @@ export const Menus = { SessionConversations: new MenuId('SessionsSessionConversations'), SessionChatTab: new MenuId('SessionsSessionChatTab'), SessionsEditorHeaderPrimary: new MenuId('SessionsEditorHeaderPrimary'), - SessionsEditorHeaderSecondary: new MenuId('SessionsEditorHeaderSecondary'), SessionsEditorHeaderLayout: new MenuId('SessionsEditorHeaderLayout'), SessionsEditorTitle: new MenuId('SessionsEditorTitle'), SessionsEditorTabsBarContext: new MenuId('SessionsEditorTabsBarContext'), diff --git a/src/vs/sessions/browser/parts/media/editorPart.css b/src/vs/sessions/browser/parts/media/editorPart.css index 5356230069f1f..1d11337194c6a 100644 --- a/src/vs/sessions/browser/parts/media/editorPart.css +++ b/src/vs/sessions/browser/parts/media/editorPart.css @@ -27,6 +27,10 @@ padding-right: 0; } +.agent-sessions-workbench .part.editor > .content .editor-group-container > .title > .title-actions { + padding: 0 0 0 var(--vscode-spacing-size40); +} + .agent-sessions-workbench.dock-detail-panel .part.editor > .content .editor-group-container > .title:not(.tabs) > .label-container > .single-tab.title-label { padding-left: var(--vscode-spacing-size80); } @@ -35,16 +39,25 @@ display: none; } -.agent-sessions-workbench.dock-detail-panel .part.editor > .content .editor-group-container > .title:not(.tabs) > .tabs-bar-add-tab { +.agent-sessions-workbench.dock-detail-panel .part.editor > .content .editor-group-container > .title:not(.tabs) > .label-container > .tabs-bar-add-tab { display: flex; align-items: center; flex: 0 0 auto; height: var(--editor-group-tab-height); } -.agent-sessions-workbench.dock-detail-panel .part.editor > .content .editor-group-container > .title:not(.tabs) > .tabs-bar-add-tab .action-label:not(.separator) { - width: 22px; - height: 22px; +.agent-sessions-workbench.dock-detail-panel .part.editor > .content .editor-group-container > .title:not(.tabs) > .label-container > .tabs-bar-add-tab::after { + content: ''; + width: var(--vscode-strokeThickness); + height: var(--vscode-spacing-size160); + margin: 0 var(--vscode-spacing-size40); + background-color: var(--vscode-titleBar-activeForeground); + opacity: 0.3; +} + +.agent-sessions-workbench.dock-detail-panel .part.editor > .content .editor-group-container > .title:not(.tabs) > .label-container > .tabs-bar-add-tab .action-label:not(.separator) { + width: var(--vscode-codiconFontSize); + height: var(--vscode-codiconFontSize); margin: 0 var(--vscode-spacing-size40); display: flex; align-items: center; @@ -53,12 +66,12 @@ color: var(--chat-tab-inactive-foreground, currentColor); } -.agent-sessions-workbench.dock-detail-panel .part.editor > .content .editor-group-container > .title:not(.tabs) > .tabs-bar-add-tab .action-label:not(.separator):not(.disabled):hover { +.agent-sessions-workbench.dock-detail-panel .part.editor > .content .editor-group-container > .title:not(.tabs) > .label-container > .tabs-bar-add-tab .action-label:not(.separator):not(.disabled):hover { background-color: var(--vscode-toolbar-hoverBackground); color: var(--chat-tab-active-foreground, currentColor); } -.agent-sessions-workbench.dock-detail-panel .part.editor > .content .editor-group-container > .title:not(.tabs) > .tabs-bar-add-tab .action-label:not(.separator):focus-visible { +.agent-sessions-workbench.dock-detail-panel .part.editor > .content .editor-group-container > .title:not(.tabs) > .label-container > .tabs-bar-add-tab .action-label:not(.separator):focus-visible { outline: var(--vscode-strokeThickness) solid var(--vscode-focusBorder); outline-offset: calc(-1 * var(--vscode-strokeThickness)); } diff --git a/src/vs/sessions/browser/parts/singlePaneEditorPart.ts b/src/vs/sessions/browser/parts/singlePaneEditorPart.ts index 316cd5198dfe5..3fda9ddb79e7d 100644 --- a/src/vs/sessions/browser/parts/singlePaneEditorPart.ts +++ b/src/vs/sessions/browser/parts/singlePaneEditorPart.ts @@ -27,10 +27,9 @@ import { SinglePaneAuxiliaryBarPart } from './singlePaneAuxiliaryBarPart.js'; * the editor part share one instance) and the {@link DockedAuxiliaryBarController} * that docks and sizes the auxiliary bar inside the editor part. The full-width * header itself is rendered by the editor group from the group's configured header - * menus ({@link Menus.SessionsEditorHeaderPrimary} / {@link Menus.SessionsEditorHeaderSecondary}, - * supplied via {@link getGroupViewOptions}) and also hosts breadcrumbs in that row - * for text file editors. The part only reacts to the header's height to reposition - * the docked auxiliary bar. + * menus, supplied via {@link getGroupViewOptions}, and also hosts breadcrumbs in + * that row for text file editors. The part only reacts to the header's height to + * reposition the docked auxiliary bar. */ export class SinglePaneMainEditorPart extends MainEditorPart { @@ -42,7 +41,6 @@ export class SinglePaneMainEditorPart extends MainEditorPart { return { menuIds: { headerPrimary: Menus.SessionsEditorHeaderPrimary, - headerSecondary: Menus.SessionsEditorHeaderSecondary, headerLayout: Menus.SessionsEditorHeaderLayout, editorActions: Menus.SessionsEditorTitle, tabsBarContext: Menus.SessionsEditorTabsBarContext, diff --git a/src/vs/sessions/contrib/changes/browser/changesViewActions.ts b/src/vs/sessions/contrib/changes/browser/changesViewActions.ts index d00c461da5819..35ad8683c9775 100644 --- a/src/vs/sessions/contrib/changes/browser/changesViewActions.ts +++ b/src/vs/sessions/contrib/changes/browser/changesViewActions.ts @@ -208,10 +208,8 @@ class SetChangesListViewModeAction extends Action2 { icon: Codicon.listFlat, f1: false, menu: { - // Always in the overflow ("…") of the right header, whether the editor - // area is visible or collapsed (as long as the changes list is shown). - id: Menus.SessionsEditorHeaderSecondary, - group: 'secondary/2_viewMode', + id: Menus.SessionsEditorTitle, + group: '2_viewMode', order: 20, when: ContextKeyExpr.and( singlePaneDiffEditorTitle, @@ -239,10 +237,8 @@ class SetChangesTreeViewModeAction extends Action2 { icon: Codicon.listTree, f1: false, menu: { - // Always in the overflow ("…") of the right header, whether the editor - // area is visible or collapsed (as long as the changes list is shown). - id: Menus.SessionsEditorHeaderSecondary, - group: 'secondary/2_viewMode', + id: Menus.SessionsEditorTitle, + group: '2_viewMode', order: 20, when: ContextKeyExpr.and( singlePaneDiffEditorTitle, @@ -270,7 +266,7 @@ class CollapseAllSessionChangesDiffsAction extends Action2 { icon: Codicon.collapseAll, f1: false, menu: { - id: Menus.SessionsEditorHeaderSecondary, + id: Menus.SessionsEditorTitle, group: '1_diff', order: 10, when: ContextKeyExpr.and( @@ -300,7 +296,7 @@ class ExpandAllSessionChangesDiffsAction extends Action2 { icon: Codicon.expandAll, f1: false, menu: { - id: Menus.SessionsEditorHeaderSecondary, + id: Menus.SessionsEditorTitle, group: '1_diff', order: 10, when: ContextKeyExpr.and( @@ -329,20 +325,13 @@ registerAction2(ExpandAllSessionChangesDiffsAction); // The action changes the preferred layout. Side by side still falls back to inline // when the editor is narrow, so the label must not promise an immediate layout. -MenuRegistry.appendMenuItem(Menus.SessionsEditorHeaderSecondary, { +MenuRegistry.appendMenuItem(Menus.SessionsEditorTitle, { command: { id: TOGGLE_DIFF_SIDE_BY_SIDE, - title: localize('preferSideBySideDiff', "Prefer Side by Side Diff"), - tooltip: localize('preferSideBySideDiff.tooltip', "Uses inline layout when space is limited unless screen reader optimized mode is enabled."), + title: localize('alwaysShowInlineDiff', "Always Show Inline Diff"), + tooltip: localize('alwaysShowInlineDiff.tooltip', "Always uses inline layout."), icon: Codicon.diffSidebyside, - toggled: { - condition: ContextKeyExpr.or( - ContextKeyExpr.and(singlePaneChangesEditorActive, SessionsDiffRenderSideBySideContext), - ContextKeyExpr.and(singlePaneFileDiffEditorActive, SessionsDiffRenderSideBySideContext) - )!, - title: localize('preferInlineDiff', "Prefer Inline Diff"), - tooltip: localize('preferInlineDiff.tooltip', "Always uses inline layout."), - }, + toggled: SessionsDiffRenderSideBySideContext.negate(), }, group: '1_diff', order: 20, diff --git a/src/vs/sessions/contrib/changes/browser/sessionsChangesAccessibilityHelp.ts b/src/vs/sessions/contrib/changes/browser/sessionsChangesAccessibilityHelp.ts index 6d6c973ae4a84..35dfd2a1a07f0 100644 --- a/src/vs/sessions/contrib/changes/browser/sessionsChangesAccessibilityHelp.ts +++ b/src/vs/sessions/contrib/changes/browser/sessionsChangesAccessibilityHelp.ts @@ -10,6 +10,7 @@ import { IAccessibleViewImplementation } from '../../../../platform/accessibilit import { IViewsService } from '../../../../workbench/services/views/common/viewsService.js'; import { AccessibilityVerbositySettingId } from '../../../../workbench/contrib/accessibility/browser/accessibilityConfiguration.js'; import { FocusedViewContext } from '../../../../workbench/common/contextkeys.js'; +import { IAgentWorkbenchLayoutService } from '../../../browser/workbench.js'; import { CHANGES_VIEW_ID } from '../common/changes.js'; import { ChangesViewPane } from './changesView.js'; @@ -26,6 +27,7 @@ export class SessionsChangesAccessibilityHelp implements IAccessibleViewImplemen getProvider(accessor: ServicesAccessor) { const viewsService = accessor.get(IViewsService); + const layoutService = accessor.get(IAgentWorkbenchLayoutService); const content: string[] = []; content.push(localize('sessionsChanges.overview', "You are in the Changes view. It shows the files changed by the current session as a tree, followed by two collapsible sections: Other Files and Checks.")); @@ -35,7 +37,9 @@ export class SessionsChangesAccessibilityHelp implements IAccessibleViewImplemen content.push(localize('sessionsChanges.checks', "The Checks section lists the continuous integration checks for the session's pull request. Its header is a button: press Enter or Space to collapse or expand it{0}.", '')); content.push(localize('sessionsChanges.viewMode', "The Changes view can show files as a tree or a flat list. Use the view's toolbar actions to switch between Tree and List modes.")); content.push(localize('sessionsChanges.operations', "When available, the toolbar also provides actions to commit, merge, sync, or create a pull request. Use Tab and Shift+Tab to move between the file list and toolbar actions.")); - content.push(localize('sessionsChanges.diffView', "File diffs can prefer side-by-side or inline layout. Unless screen reader optimized mode is enabled, side-by-side diffs automatically use inline layout when space is limited. Use the Toggle Preferred Diff View command to switch the preference{0}.", '')); + content.push(layoutService.isSinglePaneLayoutEnabled + ? localize('sessionsChanges.diffView.singlePane', "File diffs can prefer side-by-side or inline layout. Unless screen reader optimized mode is enabled, side-by-side diffs automatically use inline layout when space is limited. Use Always Show Inline Diff in the editor title bar's More Actions menu, or use the Toggle Preferred Diff View command to switch the preference{0}.", '') + : localize('sessionsChanges.diffView.classic', "File diffs can use side-by-side or inline layout. Use Inline View in the editor title area's More Actions menu, or use the Toggle Inline View command to switch the layout{0}.", '')); return new AccessibleContentProvider( AccessibleViewProviderId.SessionsChanges, diff --git a/src/vs/sessions/contrib/changes/test/browser/changesViewActions.test.ts b/src/vs/sessions/contrib/changes/test/browser/changesViewActions.test.ts index 833b374f7d3e5..140b5785588e9 100644 --- a/src/vs/sessions/contrib/changes/test/browser/changesViewActions.test.ts +++ b/src/vs/sessions/contrib/changes/test/browser/changesViewActions.test.ts @@ -18,13 +18,16 @@ import { TestInstantiationService } from '../../../../../platform/instantiation/ import { EditorContextKeys } from '../../../../../editor/common/editorContextKeys.js'; import { SessionsDiffRenderSideBySideContext } from '../../../editor/common/diffEditorOptionsService.js'; import { ActiveEditorContext, AuxiliaryBarVisibleContext, IsAuxiliaryWindowContext, IsSessionsWindowContext, IsTopRightEditorGroupContext, MainEditorAreaVisibleContext, TextCompareEditorActiveContext } from '../../../../../workbench/common/contextkeys.js'; +import { IViewsService } from '../../../../../workbench/services/views/common/viewsService.js'; import { Menus } from '../../../../browser/menus.js'; +import { IAgentWorkbenchLayoutService } from '../../../../browser/workbench.js'; import { ISessionsService } from '../../../../services/sessions/browser/sessionsService.js'; import { IActiveSession } from '../../../../services/sessions/common/sessionsManagement.js'; import { ChangesContextKeys, ChangesViewMode } from '../../common/changes.js'; import { IsPhoneLayoutContext, SessionHasChangesContext, SessionHasWorkspaceContext, SessionIsCreatedContext, SinglePaneDiffEditorInputActiveContext, SinglePaneLayoutEnabledContext } from '../../../../common/contextkeys.js'; import { SessionChangesEditor } from '../../browser/sessionChangesEditor.js'; import { CHANGES_HEADER_ACTIONS_ID } from '../../browser/changesView.js'; +import { SessionsChangesAccessibilityHelp } from '../../browser/sessionsChangesAccessibilityHelp.js'; import '../../browser/changesViewActions.js'; suite('Changes View Actions', () => { @@ -102,12 +105,12 @@ suite('Changes View Actions', () => { ]); }); - test('collapse all diffs is contributed to the single-pane editor header (right)', () => { - const item = MenuRegistry.getMenuItems(Menus.SessionsEditorHeaderSecondary) + test('collapse all diffs is contributed to the editor title bar overflow menu', () => { + const item = MenuRegistry.getMenuItems(Menus.SessionsEditorTitle) .filter(isIMenuItem) .find(item => item.command.id === 'workbench.action.agentSessions.collapseAllDiffs'); - assert.ok(item, 'expected collapse all diffs action on the single-pane editor header menu'); + assert.ok(item, 'expected collapse all diffs action in the editor title bar overflow menu'); const when = item.when?.serialize() ?? ''; assert.deepStrictEqual({ group: item.group, @@ -128,12 +131,12 @@ suite('Changes View Actions', () => { }); }); - test('expand all diffs is contributed to the single-pane editor header (right)', () => { - const item = MenuRegistry.getMenuItems(Menus.SessionsEditorHeaderSecondary) + test('expand all diffs is contributed to the editor title bar overflow menu', () => { + const item = MenuRegistry.getMenuItems(Menus.SessionsEditorTitle) .filter(isIMenuItem) .find(item => item.command.id === 'workbench.action.agentSessions.expandAllDiffs'); - assert.ok(item, 'expected expand all diffs action on the single-pane editor header menu'); + assert.ok(item, 'expected expand all diffs action in the editor title bar overflow menu'); const when = item.when?.serialize() ?? ''; assert.deepStrictEqual({ group: item.group, @@ -156,15 +159,15 @@ suite('Changes View Actions', () => { }); }); - test('preferred diff view is contributed to multi-file and single-file diff editor headers with toggle state', () => { - const item = MenuRegistry.getMenuItems(Menus.SessionsEditorHeaderSecondary) + test('always show inline diff is contributed to the editor title bar overflow menu for multi-file and single-file diffs', () => { + const item = MenuRegistry.getMenuItems(Menus.SessionsEditorTitle) .filter(isIMenuItem) .find(item => item.command.id === 'toggle.diff.renderSideBySide'); - assert.ok(item, 'expected the toggle inline view action on the single-pane editor header menu'); + assert.ok(item, 'expected the preferred diff view action in the editor title bar overflow menu'); const when = item.when?.serialize() ?? ''; const toggled = item.command.toggled; - const toggledInfo = isICommandActionToggleInfo(toggled) ? toggled : undefined; + const toggledCondition = isICommandActionToggleInfo(toggled) ? toggled.condition : toggled; const nonTextDiffContext = new Context(1, null); nonTextDiffContext.setValue(IsSessionsWindowContext.key, true); nonTextDiffContext.setValue(SinglePaneDiffEditorInputActiveContext.key, true); @@ -172,6 +175,10 @@ suite('Changes View Actions', () => { nonTextDiffContext.setValue(IsAuxiliaryWindowContext.key, false); nonTextDiffContext.setValue(IsTopRightEditorGroupContext.key, true); nonTextDiffContext.setValue(MainEditorAreaVisibleContext.key, true); + const toggleContext = new Context(1, null); + toggleContext.setValue(SessionsDiffRenderSideBySideContext.key, true); + const toggledWhenSideBySide = toggledCondition?.evaluate(toggleContext); + toggleContext.setValue(SessionsDiffRenderSideBySideContext.key, false); assert.deepStrictEqual({ id: item.command.id, title: typeof item.command.title === 'string' ? item.command.title : item.command.title.value, @@ -179,9 +186,9 @@ suite('Changes View Actions', () => { order: item.order, icon: ThemeIcon.isThemeIcon(item.command.icon) ? item.command.icon.id : undefined, tooltip: typeof item.command.tooltip === 'string' ? item.command.tooltip : item.command.tooltip?.value, - toggledTitle: toggledInfo?.title, - toggledTooltip: toggledInfo?.tooltip, - toggledOnSharedPreference: toggledInfo?.condition.serialize().includes(SessionsDiffRenderSideBySideContext.key), + hasStateSpecificTitle: isICommandActionToggleInfo(toggled), + toggledWhenSideBySide, + toggledWhenInline: toggledCondition?.evaluate(toggleContext), hasSessionsWindowGate: when.includes(IsSessionsWindowContext.key), hasActiveEditorGate: when.includes(ActiveEditorContext.key) && when.includes(SessionChangesEditor.ID), hasTextCompareEditorGate: when.includes(TextCompareEditorActiveContext.key), @@ -190,14 +197,14 @@ suite('Changes View Actions', () => { matchesNonTextDiffContext: item.when?.evaluate(nonTextDiffContext) ?? false, }, { id: 'toggle.diff.renderSideBySide', - title: 'Prefer Side by Side Diff', + title: 'Always Show Inline Diff', group: '1_diff', order: 20, icon: Codicon.diffSidebyside.id, - tooltip: 'Uses inline layout when space is limited unless screen reader optimized mode is enabled.', - toggledTitle: 'Prefer Inline Diff', - toggledTooltip: 'Always uses inline layout.', - toggledOnSharedPreference: true, + tooltip: 'Always uses inline layout.', + hasStateSpecificTitle: false, + toggledWhenSideBySide: false, + toggledWhenInline: true, hasSessionsWindowGate: true, hasActiveEditorGate: true, hasTextCompareEditorGate: true, @@ -235,9 +242,29 @@ suite('Changes View Actions', () => { }); }); + function getChangesAccessibilityHelp(singlePane: boolean): string { + const instantiationService = new TestInstantiationService(); + instantiationService.stub(IViewsService, new class extends mock() { }); + instantiationService.stub(IAgentWorkbenchLayoutService, new class extends mock() { + override readonly isSinglePaneLayoutEnabled = singlePane; + }); + const provider = new SessionsChangesAccessibilityHelp().getProvider(instantiationService); + + const content = provider.provideContent(); + provider.dispose(); + return content; + } + + test('Changes accessibility help describes the single-pane diff action', () => { + assert.strictEqual(getChangesAccessibilityHelp(true).includes('Use Always Show Inline Diff in the editor title bar\'s More Actions menu'), true); + }); + + test('Changes accessibility help describes the classic diff action', () => { + assert.strictEqual(getChangesAccessibilityHelp(false).includes('Use Inline View in the editor title area\'s More Actions menu'), true); + }); - test('view mode toggles include non-text single-file diff editor headers', () => { - const items = MenuRegistry.getMenuItems(Menus.SessionsEditorHeaderSecondary) + test('view mode toggles are contributed to the editor title bar overflow for non-text single-file diffs', () => { + const items = MenuRegistry.getMenuItems(Menus.SessionsEditorTitle) .filter(isIMenuItem) .filter(item => item.command.id === 'workbench.action.agentSessions.setChangesListViewMode' || item.command.id === 'workbench.action.agentSessions.setChangesTreeViewMode'); @@ -265,6 +292,7 @@ suite('Changes View Actions', () => { hasDiffEditorInputGate: when.includes(SinglePaneDiffEditorInputActiveContext.key), hasSinglePaneConfigGate: when.includes(SinglePaneLayoutEnabledContext.key), hasAuxBarVisibleGate: when.includes(AuxiliaryBarVisibleContext.key), + hasEditorAreaVisibleGate: when.includes(MainEditorAreaVisibleContext.key), hasViewModeGate: when.includes(ChangesContextKeys.ViewMode.key), matchesSingleFileDiffContext: item.when?.evaluate(context) ?? false, }; @@ -273,7 +301,7 @@ suite('Changes View Actions', () => { assert.deepStrictEqual(actual, [{ id: 'workbench.action.agentSessions.setChangesListViewMode', title: 'View as List', - group: 'secondary/2_viewMode', + group: '2_viewMode', order: 20, icon: Codicon.listFlat.id, hasSessionsWindowGate: true, @@ -281,12 +309,13 @@ suite('Changes View Actions', () => { hasDiffEditorInputGate: true, hasSinglePaneConfigGate: true, hasAuxBarVisibleGate: true, + hasEditorAreaVisibleGate: false, hasViewModeGate: true, matchesSingleFileDiffContext: true, }, { id: 'workbench.action.agentSessions.setChangesTreeViewMode', title: 'View as Tree', - group: 'secondary/2_viewMode', + group: '2_viewMode', order: 20, icon: Codicon.listTree.id, hasSessionsWindowGate: true, @@ -294,6 +323,7 @@ suite('Changes View Actions', () => { hasDiffEditorInputGate: true, hasSinglePaneConfigGate: true, hasAuxBarVisibleGate: true, + hasEditorAreaVisibleGate: false, hasViewModeGate: true, matchesSingleFileDiffContext: true, }]); diff --git a/src/vs/sessions/contrib/codeReview/browser/codeReview.contributions.ts b/src/vs/sessions/contrib/codeReview/browser/codeReview.contributions.ts index 64beb06764d6f..db89b36aced01 100644 --- a/src/vs/sessions/contrib/codeReview/browser/codeReview.contributions.ts +++ b/src/vs/sessions/contrib/codeReview/browser/codeReview.contributions.ts @@ -10,7 +10,7 @@ import { Action2, MenuId, registerAction2 } from '../../../../platform/actions/c import { ContextKeyExpr } from '../../../../platform/contextkey/common/contextkey.js'; import { InstantiationType, registerSingleton } from '../../../../platform/instantiation/common/extensions.js'; import { ServicesAccessor } from '../../../../platform/instantiation/common/instantiation.js'; -import { ActiveEditorContext, IsAuxiliaryWindowContext, IsSessionsWindowContext, IsTopRightEditorGroupContext, MainEditorAreaVisibleContext } from '../../../../workbench/common/contextkeys.js'; +import { ActiveEditorContext, IsAuxiliaryWindowContext, IsSessionsWindowContext, IsTopRightEditorGroupContext } from '../../../../workbench/common/contextkeys.js'; import { IsPhoneLayoutContext, SessionHasChangesContext, SessionIsCreatedContext, SessionWorkspaceIsVirtualContext, SessionProviderIdContext, SinglePaneLayoutEnabledContext } from '../../../common/contextkeys.js'; import { ChatContextKeys } from '../../../../workbench/contrib/chat/common/actions/chatContextKeys.js'; import { CHAT_CATEGORY } from '../../../../workbench/contrib/chat/browser/actions/chatActions.js'; @@ -29,9 +29,8 @@ const CODE_REVIEW_QUERY = '/code-review'; const singlePaneDetailPanel = SinglePaneLayoutEnabledContext; -// Code review is shown next to the diff-stats action in the single-pane Changes -// editor header, so it is only contributed to the classic changes button bar -// when single-pane is off. +// Code review is shown in the single-pane editor title bar, so it is only +// contributed to the classic changes button bar when single-pane is off. const codeReviewChangesToolbarWhen = ContextKeyExpr.and( IsSessionsWindowContext, SessionWorkspaceIsVirtualContext.toNegated(), @@ -63,7 +62,7 @@ class RunSessionCodeReviewAction extends Action2 { tooltip: localize('sessions.runCodeReview.tooltip', "Run Code Review"), category: CHAT_CATEGORY, icon: Codicon.codeReview, - precondition: ChatContextKeys.hasAgentSessionChanges, + precondition: ContextKeyExpr.or(ChatContextKeys.hasAgentSessionChanges, SessionHasChangesContext), menu: [ { id: MenuId.AgentsChangesToolbar, @@ -72,16 +71,10 @@ class RunSessionCodeReviewAction extends Action2 { when: codeReviewChangesToolbarWhen, }, { - id: Menus.SessionsEditorHeaderSecondary, - group: '0_codeReview', - order: 10, - when: ContextKeyExpr.and(singlePaneCodeReviewWhen, MainEditorAreaVisibleContext), - }, - { - id: Menus.SessionsEditorHeaderSecondary, - group: 'secondary/1_codeReview', + id: Menus.SessionsEditorTitle, + group: 'navigation', order: 10, - when: ContextKeyExpr.and(singlePaneCodeReviewWhen, MainEditorAreaVisibleContext.toNegated()), + when: singlePaneCodeReviewWhen, }, ], }); diff --git a/src/vs/sessions/contrib/codeReview/test/browser/codeReviewService.test.ts b/src/vs/sessions/contrib/codeReview/test/browser/codeReviewService.test.ts index 716ce961cedd5..1be7dd5d46c0e 100644 --- a/src/vs/sessions/contrib/codeReview/test/browser/codeReviewService.test.ts +++ b/src/vs/sessions/contrib/codeReview/test/browser/codeReviewService.test.ts @@ -9,6 +9,7 @@ import { URI } from '../../../../../base/common/uri.js'; import { IObservable, constObservable, derived, observableValue } from '../../../../../base/common/observable.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; import { isIMenuItem, MenuId, MenuRegistry } from '../../../../../platform/actions/common/actions.js'; +import { Context } from '../../../../../platform/contextkey/browser/contextKeyService.js'; import { TestInstantiationService } from '../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; import { CommandsRegistry } from '../../../../../platform/commands/common/commands.js'; import { ServicesAccessor } from '../../../../../platform/instantiation/common/instantiation.js'; @@ -30,6 +31,7 @@ import { ICodeReviewService, CodeReviewService, PRReviewStateKind } from '../../ import { ISessionsService } from '../../../../services/sessions/browser/sessionsService.js'; import { IActiveSession, ISendRequestOptions, ISessionsChangeEvent, ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; import { IChatWidgetService } from '../../../../../workbench/contrib/chat/browser/chat.js'; +import { ChatContextKeys } from '../../../../../workbench/contrib/chat/common/actions/chatContextKeys.js'; import { ISessionChangesService } from '../../../changes/browser/sessionChangesService.js'; import '../../browser/codeReview.contributions.js'; @@ -311,50 +313,37 @@ suite('Code Review Contributions', () => { const store = ensureNoDisposablesAreLeakedInTestSuite(); - test('Run Code Review is right-inline when visible and first in overflow when collapsed', () => { - const primaryItem = MenuRegistry.getMenuItems(Menus.SessionsEditorHeaderPrimary) + test('Run Code Review is contributed to the editor title bar', () => { + const titleItem = MenuRegistry.getMenuItems(Menus.SessionsEditorTitle) .filter(isIMenuItem) .find(item => item.command.id === 'sessions.codeReview.run'); - const rightItems = MenuRegistry.getMenuItems(Menus.SessionsEditorHeaderSecondary) - .filter(isIMenuItem) - .filter(item => item.command.id === 'sessions.codeReview.run'); - const inlineItem = rightItems.find(item => item.group === '0_codeReview'); - const overflowItem = rightItems.find(item => item.group === 'secondary/1_codeReview'); - - assert.strictEqual(primaryItem, undefined, 'Run Code Review should not render inline in the primary header'); - assert.ok(inlineItem, 'expected Run Code Review inline on the right while the editor is visible'); - assert.ok(overflowItem, 'expected Run Code Review in overflow while the editor is collapsed'); - const inlineWhen = inlineItem.when?.serialize() ?? ''; - const overflowWhen = overflowItem.when?.serialize() ?? ''; + + assert.ok(titleItem, 'expected Run Code Review in the editor title bar'); + const when = titleItem.when?.serialize() ?? ''; + const enablementContext = new Context(1, null); + enablementContext.setValue(ChatContextKeys.hasAgentSessionChanges.key, false); + enablementContext.setValue(SessionHasChangesContext.key, true); + const enabledFromSessionChanges = titleItem.command.precondition?.evaluate(enablementContext); + enablementContext.setValue(ChatContextKeys.hasAgentSessionChanges.key, true); + enablementContext.setValue(SessionHasChangesContext.key, false); assert.deepStrictEqual({ - inline: { - group: inlineItem.group, - order: inlineItem.order, - editorAreaGate: inlineWhen.includes(MainEditorAreaVisibleContext.key), - }, - overflow: { - group: overflowItem.group, - order: overflowItem.order, - editorAreaGate: overflowWhen.includes(`!${MainEditorAreaVisibleContext.key}`), - }, - hasSessionsWindowGate: inlineWhen.includes(IsSessionsWindowContext.key), - hasActiveEditorGate: inlineWhen.includes(ActiveEditorContext.key) && inlineWhen.includes(SessionChangesEditorInput.EDITOR_ID), - hasSinglePaneLayoutGate: inlineWhen.includes(SinglePaneLayoutEnabledContext.key), - hasAuxiliaryWindowGate: inlineWhen.includes(IsAuxiliaryWindowContext.key), - hasTopRightEditorGroupGate: inlineWhen.includes(IsTopRightEditorGroupContext.key), - hasChangesGate: inlineWhen.includes(SessionHasChangesContext.key), - hasCreatedGate: inlineWhen.includes(SessionIsCreatedContext.key), + group: titleItem.group, + order: titleItem.order, + enabledFromSessionChanges, + enabledFromChatChanges: titleItem.command.precondition?.evaluate(enablementContext), + hasSessionsWindowGate: when.includes(IsSessionsWindowContext.key), + hasActiveEditorGate: when.includes(ActiveEditorContext.key) && when.includes(SessionChangesEditorInput.EDITOR_ID), + hasSinglePaneLayoutGate: when.includes(SinglePaneLayoutEnabledContext.key), + hasAuxiliaryWindowGate: when.includes(IsAuxiliaryWindowContext.key), + hasTopRightEditorGroupGate: when.includes(IsTopRightEditorGroupContext.key), + hasChangesGate: when.includes(SessionHasChangesContext.key), + hasCreatedGate: when.includes(SessionIsCreatedContext.key), + hasEditorAreaVisibleGate: when.includes(MainEditorAreaVisibleContext.key), }, { - inline: { - group: '0_codeReview', - order: 10, - editorAreaGate: true, - }, - overflow: { - group: 'secondary/1_codeReview', - order: 10, - editorAreaGate: true, - }, + group: 'navigation', + order: 10, + enabledFromSessionChanges: true, + enabledFromChatChanges: true, hasSessionsWindowGate: true, hasActiveEditorGate: true, hasSinglePaneLayoutGate: true, @@ -362,6 +351,7 @@ suite('Code Review Contributions', () => { hasTopRightEditorGroupGate: true, hasChangesGate: true, hasCreatedGate: true, + hasEditorAreaVisibleGate: false, }); }); diff --git a/src/vs/sessions/contrib/editor/browser/editor.contribution.ts b/src/vs/sessions/contrib/editor/browser/editor.contribution.ts index a6ddc9d85b070..3ba19c76f29c2 100644 --- a/src/vs/sessions/contrib/editor/browser/editor.contribution.ts +++ b/src/vs/sessions/contrib/editor/browser/editor.contribution.ts @@ -448,7 +448,7 @@ class AddFileAsContextAction extends Action2 { f1: true, precondition, menu: [{ - id: Menus.SessionsEditorHeaderSecondary, + id: Menus.SessionsEditorTitle, group: 'navigation', order: 100000, when: ContextKeyExpr.and(precondition, singlePaneDetailPanel) @@ -483,7 +483,7 @@ class AddFileAsContextAction extends Action2 { registerAction2(AddFileAsContextAction); /** - * Mirrors extension-contributed `editor/title` items into {@link Menus.SessionsEditorHeaderSecondary} + * Mirrors extension-contributed `editor/title` items into {@link Menus.SessionsEditorTitle} * so they are not lost in the single-pane layout. See `LAYOUT.md` for details. */ export class EditorTitleMenuBridgeContribution extends Disposable implements IWorkbenchContribution { @@ -524,10 +524,7 @@ export class EditorTitleMenuBridgeContribution extends Disposable implements IWo ? !!item.command.source : item.submenu.id.startsWith(EditorTitleMenuBridgeContribution._extensionSubmenuPrefix); if (isExtensionItem) { - const group = item.group === 'navigation' - ? 'extension/navigation' - : `secondary/extension/${item.group ?? 'other'}`; - this._mirrored.add(MenuRegistry.appendMenuItem(Menus.SessionsEditorHeaderSecondary, { ...item, group })); + this._mirrored.add(MenuRegistry.appendMenuItem(Menus.SessionsEditorTitle, item)); } } } diff --git a/src/vs/sessions/contrib/editor/test/browser/editorHeader.fixture.ts b/src/vs/sessions/contrib/editor/test/browser/editorHeader.fixture.ts index 3e19976e8899c..721aca0ecdce4 100644 --- a/src/vs/sessions/contrib/editor/test/browser/editorHeader.fixture.ts +++ b/src/vs/sessions/contrib/editor/test/browser/editorHeader.fixture.ts @@ -5,6 +5,7 @@ import '../../browser/media/editorBreadcrumbs.css'; import '../../browser/media/editorHeader.css'; +import '../../../../browser/parts/media/editorPart.css'; import { Codicon } from '../../../../../base/common/codicons.js'; import { localize2 } from '../../../../../nls.js'; import { MenuId, MenuRegistry } from '../../../../../platform/actions/common/actions.js'; diff --git a/src/vs/sessions/test/browser/editorTitleMenuBridge.test.ts b/src/vs/sessions/test/browser/editorTitleMenuBridge.test.ts index 35418e6ec94e1..19563d2eed961 100644 --- a/src/vs/sessions/test/browser/editorTitleMenuBridge.test.ts +++ b/src/vs/sessions/test/browser/editorTitleMenuBridge.test.ts @@ -24,19 +24,19 @@ suite('Sessions - Editor Title Menu Bridge', () => { return { isSinglePaneLayoutEnabled: singlePane } as IAgentWorkbenchLayoutService; } - function sessionsEditorHeaderCommands(): { id: string; group: string | undefined }[] { - return MenuRegistry.getMenuItems(Menus.SessionsEditorHeaderSecondary) + function sessionsEditorTitleCommands(): { id: string; group: string | undefined }[] { + return MenuRegistry.getMenuItems(Menus.SessionsEditorTitle) .filter(isIMenuItem) .map(item => ({ id: item.command.id, group: item.group })); } - function sessionsEditorHeaderSubmenus(): { id: string; group: string | undefined }[] { - return MenuRegistry.getMenuItems(Menus.SessionsEditorHeaderSecondary) + function sessionsEditorTitleSubmenus(): { id: string; group: string | undefined }[] { + return MenuRegistry.getMenuItems(Menus.SessionsEditorTitle) .filter(isISubmenuItem) .map(item => ({ id: item.submenu.id, group: item.group })); } - test('mirrors only extension-contributed editor/title items into the right editor header menu', () => { + test('mirrors only extension-contributed editor/title items into the Sessions editor title menu', () => { const local = store.add(new DisposableStore()); local.add(MenuRegistry.appendMenuItem(MenuId.EditorTitle, { @@ -50,34 +50,34 @@ suite('Sessions - Editor Title Menu Bridge', () => { store.add(new EditorTitleMenuBridgeContribution(createLayoutService(true))); - const mirrored = sessionsEditorHeaderCommands(); + const mirrored = sessionsEditorTitleCommands(); assert.deepStrictEqual( mirrored.find(item => item.id === 'test.ext.editorTitleAction'), - { id: 'test.ext.editorTitleAction', group: 'extension/navigation' }, + { id: 'test.ext.editorTitleAction', group: 'navigation' }, ); assert.ok(!mirrored.some(item => item.id === 'test.core.editorTitleAction'), 'core action should not be bridged'); local.dispose(); }); - test('keeps the right editor header menu in sync as extensions register/unregister', async () => { + test('keeps the Sessions editor title menu in sync as extensions register/unregister', async () => { store.add(new EditorTitleMenuBridgeContribution(createLayoutService(true))); - assert.ok(!sessionsEditorHeaderCommands().some(item => item.id === 'test.ext.dynamic'), 'not present before registration'); + assert.ok(!sessionsEditorTitleCommands().some(item => item.id === 'test.ext.dynamic'), 'not present before registration'); const registration = MenuRegistry.appendMenuItem(MenuId.EditorTitle, { command: { id: 'test.ext.dynamic', title: 'Dynamic Extension Action', source: { id: 'pub.ext', title: 'My Extension' } }, group: 'navigation' }); await Promise.resolve(); - assert.ok(sessionsEditorHeaderCommands().some(item => item.id === 'test.ext.dynamic'), 'present after registration'); + assert.ok(sessionsEditorTitleCommands().some(item => item.id === 'test.ext.dynamic'), 'present after registration'); registration.dispose(); await Promise.resolve(); - assert.ok(!sessionsEditorHeaderCommands().some(item => item.id === 'test.ext.dynamic'), 'removed after unregistration'); + assert.ok(!sessionsEditorTitleCommands().some(item => item.id === 'test.ext.dynamic'), 'removed after unregistration'); }); - test('mirrors only extension-contributed submenus into the right editor header menu', () => { + test('mirrors only extension-contributed submenus into the Sessions editor title menu', () => { const local = store.add(new DisposableStore()); // Extension submenus are registered with an `api:` menu id; core submenus are not. @@ -90,10 +90,10 @@ suite('Sessions - Editor Title Menu Bridge', () => { store.add(new EditorTitleMenuBridgeContribution(createLayoutService(true))); - const mirrored = sessionsEditorHeaderSubmenus(); + const mirrored = sessionsEditorTitleSubmenus(); assert.deepStrictEqual( mirrored.find(item => item.id === 'api:test.ext.submenu'), - { id: 'api:test.ext.submenu', group: 'secondary/extension/1_extension' }, + { id: 'api:test.ext.submenu', group: '1_extension' }, ); assert.ok(!mirrored.some(item => item.id === 'test.core.submenu'), 'core submenu should not be bridged'); @@ -109,7 +109,7 @@ suite('Sessions - Editor Title Menu Bridge', () => { store.add(new EditorTitleMenuBridgeContribution(createLayoutService(false))); - assert.ok(!sessionsEditorHeaderCommands().some(item => item.id === 'test.ext.disabledLayout'), 'nothing bridged when disabled'); + assert.ok(!sessionsEditorTitleCommands().some(item => item.id === 'test.ext.disabledLayout'), 'nothing bridged when disabled'); local.dispose(); }); diff --git a/src/vs/sessions/test/browser/layoutActions.test.ts b/src/vs/sessions/test/browser/layoutActions.test.ts index 2abd52d29d9c4..e1d3771971210 100644 --- a/src/vs/sessions/test/browser/layoutActions.test.ts +++ b/src/vs/sessions/test/browser/layoutActions.test.ts @@ -153,9 +153,9 @@ suite('Sessions - Layout Actions', () => { assert.ok(!headerIds.includes('workbench.action.agentSessions.hideMainEditorPart')); assert.ok(!headerIds.includes('workbench.action.agentSessions.showMainEditorPart')); - // Add File as Context stays a right-header action, not a layout action. - const headerSecondaryIds = MenuRegistry.getMenuItems(Menus.SessionsEditorHeaderSecondary).filter(isIMenuItem).map(item => item.command.id); - assert.ok(headerSecondaryIds.includes('workbench.action.agentSessions.addFileAsContext')); + // Add File as Context stays an editor action, not a group-header layout action. + const editorTitleIds = MenuRegistry.getMenuItems(Menus.SessionsEditorTitle).filter(isIMenuItem).map(item => item.command.id); + assert.ok(editorTitleIds.includes('workbench.action.agentSessions.addFileAsContext')); assert.ok(!layoutItems.some(item => item.command.id === 'workbench.action.agentSessions.addFileAsContext')); }); diff --git a/src/vs/sessions/test/browser/workbench.test.ts b/src/vs/sessions/test/browser/workbench.test.ts index 1d86c3ad5d255..92b63fc32989b 100644 --- a/src/vs/sessions/test/browser/workbench.test.ts +++ b/src/vs/sessions/test/browser/workbench.test.ts @@ -1604,7 +1604,7 @@ suite('Sessions - Workbench', () => { }, { showHeader: true, headerPrimary: Menus.SessionsEditorHeaderPrimary, - headerSecondary: Menus.SessionsEditorHeaderSecondary, + headerSecondary: undefined, headerLayout: Menus.SessionsEditorHeaderLayout, }); }); diff --git a/src/vs/workbench/browser/parts/editor/editorTabsControl.ts b/src/vs/workbench/browser/parts/editor/editorTabsControl.ts index a290e4cc817f0..c5487ffa306e7 100644 --- a/src/vs/workbench/browser/parts/editor/editorTabsControl.ts +++ b/src/vs/workbench/browser/parts/editor/editorTabsControl.ts @@ -119,9 +119,6 @@ export abstract class EditorTabsControl extends Themable implements IEditorTabsC private readonly editorActionsDisposables = this._register(new DisposableStore()); /** Whether the editor-actions toolbar currently has any actions (drives the layout-actions separator). */ private editorActionsToolbarHasActions = false; - private editorActionsToolbarHasTrailingSeparator = false; - private addTabControlHasActions = false; - private addTabControlHasTrailingSeparator = false; protected editorLayoutActionsSeparator: HTMLElement | undefined; protected editorLayoutActionsToolbarContainer: HTMLElement | undefined; @@ -204,11 +201,10 @@ export abstract class EditorTabsControl extends Themable implements IEditorTabsC return this.groupsView.partOptions.editorActionsLocation === 'default' && this.groupsView.partOptions.showTabs !== 'none'; } - protected createEditorActionsToolBar(parent: HTMLElement, classes: string[], trailingSeparator = false): void { + protected createEditorActionsToolBar(parent: HTMLElement, classes: string[]): void { this.editorActionsToolbarContainer = $('div'); this.editorActionsToolbarContainer.classList.add(...classes); parent.appendChild(this.editorActionsToolbarContainer); - this.editorActionsToolbarHasTrailingSeparator = trailingSeparator; this.handleEditorActionToolBarVisibility(this.editorActionsToolbarContainer); @@ -221,10 +217,9 @@ export abstract class EditorTabsControl extends Themable implements IEditorTabsC this.handleEditorLayoutActionsToolBarVisibility(this.editorLayoutActionsToolbarContainer); } - protected createAddTabControl(parent: HTMLElement, menuId: MenuId, before?: HTMLElement, trailingSeparator = false): HTMLElement { + protected createAddTabControl(parent: HTMLElement, menuId: MenuId, before?: HTMLElement): HTMLElement { const container = $('.tabs-bar-add-tab'); parent.insertBefore(container, before ?? null); - this.addTabControlHasTrailingSeparator = trailingSeparator; const menu = this._register(this.menuService.createMenu(menuId, this.contextKeyService)); const getActions = () => getFlatActionBarActions(menu.getActions({ shouldForwardArgs: true })); @@ -240,15 +235,12 @@ export abstract class EditorTabsControl extends Themable implements IEditorTabsC })); const toolbar = this._register(this.instantiationService.createInstance(WorkbenchToolBar, container, { ariaLabel: localize('ariaLabelAddTab', "Add Tab"), - trailingSeparator, actionViewItemProvider: action => action === addTabAction ? dropdown : undefined })); toolbar.setActions([addTabAction]); const updateVisibility = () => { - this.addTabControlHasActions = getActions().length > 0; - container.classList.toggle('hidden', !this.addTabControlHasActions); - this.updateEditorLayoutActionsSeparator(); + container.classList.toggle('hidden', getActions().length === 0); }; updateVisibility(); this._register(menu.onDidChange(updateVisibility)); @@ -260,9 +252,7 @@ export abstract class EditorTabsControl extends Themable implements IEditorTabsC const hasLayoutActions = (this.editorLayoutActionsToolbar?.getItemsLength() ?? 0) > 0; if (this.editorLayoutActionsSeparator) { setVisibility(hasLayoutActions - && !this.editorActionsToolbarHasTrailingSeparator - && !this.addTabControlHasTrailingSeparator - && (this.editorActionsToolbarHasActions || this.addTabControlHasActions), this.editorLayoutActionsSeparator); + && this.editorActionsToolbarHasActions, this.editorLayoutActionsSeparator); } } @@ -327,7 +317,6 @@ export abstract class EditorTabsControl extends Themable implements IEditorTabsC telemetrySource: 'editorPart', resetMenu: editorActionsMenuId, overflowBehavior: { maxItems: 9, exempted: EDITOR_CORE_NAVIGATION_COMMANDS }, - trailingSeparator: this.editorActionsToolbarHasTrailingSeparator, highlightToggledItems: true })); diff --git a/src/vs/workbench/browser/parts/editor/multiEditorTabsControl.ts b/src/vs/workbench/browser/parts/editor/multiEditorTabsControl.ts index f180d9ea76027..9b68bee9ebf94 100644 --- a/src/vs/workbench/browser/parts/editor/multiEditorTabsControl.ts +++ b/src/vs/workbench/browser/parts/editor/multiEditorTabsControl.ts @@ -258,7 +258,7 @@ export class MultiEditorTabsControl extends EditorTabsControl { } // Create Editor Toolbar - this.createEditorActionsToolBar(this.tabsAndActionsContainer, ['editor-actions'], !!this.menuIds?.tabsBarAddTab); + this.createEditorActionsToolBar(this.tabsAndActionsContainer, ['editor-actions']); // Set tabs control visibility this.updateTabsControlVisibility(); diff --git a/src/vs/workbench/browser/parts/editor/singleEditorTabsControl.ts b/src/vs/workbench/browser/parts/editor/singleEditorTabsControl.ts index 01a3638f77e62..0e9c593237298 100644 --- a/src/vs/workbench/browser/parts/editor/singleEditorTabsControl.ts +++ b/src/vs/workbench/browser/parts/editor/singleEditorTabsControl.ts @@ -67,13 +67,13 @@ export class SingleEditorTabsControl extends EditorTabsControl { titleContainer.classList.toggle('breadcrumbs', Boolean(this.breadcrumbsControl)); this._register(toDisposable(() => titleContainer.classList.remove('breadcrumbs'))); // important to remove because the container is a shared dom node - // Create editor actions toolbar - this.createEditorActionsToolBar(titleContainer, ['title-actions']); - if (this.menuIds?.tabsBarAddTab) { - this.createAddTabControl(titleContainer, this.menuIds.tabsBarAddTab, this.editorLayoutActionsSeparator, true); + this.createAddTabControl(labelContainer, this.menuIds.tabsBarAddTab); } + // Create editor actions toolbar + this.createEditorActionsToolBar(titleContainer, ['title-actions']); + return titleContainer; }