Skip to content

Commit 70e606d

Browse files
bpaserobpasero
authored
Aux window: "Window 1" is a bad label? (fix microsoft#196394) (microsoft#196682)
1 parent e7baee7 commit 70e606d

File tree

11 files changed

+132
-8
lines changed

11 files changed

+132
-8
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,7 @@ export interface IEditorGroupView extends IDisposable, ISerializableView, IEdito
282282
setActive(isActive: boolean): void;
283283

284284
notifyIndexChanged(newIndex: number): void;
285+
notifyLabelChanged(newLabel: string): void;
285286

286287
openEditor(editor: EditorInput, options?: IEditorOptions, internalOptions?: IInternalEditorOpenOptions): Promise<IEditorPane | undefined>;
287288

‎src/vs/workbench/browser/parts/editor/editorGroupView.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
136136
from: IEditorGroupView | ISerializedEditorGroupModel | null,
137137
private readonly editorPartsView: IEditorPartsView,
138138
public readonly groupsView: IEditorGroupsView,
139-
private readonly groupsLabel: string,
139+
private groupsLabel: string,
140140
private _index: number,
141141
@IInstantiationService private readonly instantiationService: IInstantiationService,
142142
@IContextKeyService private readonly contextKeyService: IContextKeyService,
@@ -449,7 +449,7 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
449449
if (this.isEmpty) {
450450
this.element.classList.add('empty');
451451
this.element.tabIndex = 0;
452-
this.element.setAttribute('aria-label', localize('emptyEditorGroup', "{0} (empty)", this.label));
452+
this.element.setAttribute('aria-label', localize('emptyEditorGroup', "{0} (empty)", this.ariaLabel));
453453
}
454454

455455
// Non-Empty Container: revert empty container attributes
@@ -762,6 +762,10 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
762762
}
763763

764764
get ariaLabel(): string {
765+
if (this.groupsLabel) {
766+
return localize('groupAriaLabelLong', "{0}: Editor Group {1}", this.groupsLabel, this._index + 1);
767+
}
768+
765769
return localize('groupAriaLabel', "Editor Group {0}", this._index + 1);
766770
}
767771

@@ -785,6 +789,13 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
785789
}
786790
}
787791

792+
notifyLabelChanged(newLabel: string): void {
793+
if (this.groupsLabel !== newLabel) {
794+
this.groupsLabel = newLabel;
795+
this.model.setLabel(newLabel);
796+
}
797+
}
798+
788799
setActive(isActive: boolean): void {
789800
this.active = isActive;
790801

‎src/vs/workbench/browser/parts/editor/editorPart.ts

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { localize } from 'vs/nls';
76
import { IThemeService } from 'vs/platform/theme/common/themeService';
87
import { Part } from 'vs/workbench/browser/part';
98
import { Dimension, isAncestor, $, EventHelper, addDisposableGenericMouseDownListener, getWindow } from 'vs/base/browser/dom';
@@ -99,6 +98,9 @@ export class EditorPart extends Part implements IEditorPart {
9998
private readonly _onDidChangeGroupIndex = this._register(new Emitter<IEditorGroupView>());
10099
readonly onDidChangeGroupIndex = this._onDidChangeGroupIndex.event;
101100

101+
private readonly _onDidChangeGroupLabel = this._register(new Emitter<IEditorGroupView>());
102+
readonly onDidChangeGroupLabel = this._onDidChangeGroupLabel.event;
103+
102104
private readonly _onDidChangeGroupLocked = this._register(new Emitter<IEditorGroupView>());
103105
readonly onDidChangeGroupLocked = this._onDidChangeGroupLocked.event;
104106

@@ -643,6 +645,9 @@ export class EditorPart extends Part implements IEditorPart {
643645
case GroupModelChangeKind.GROUP_INDEX:
644646
this._onDidChangeGroupIndex.fire(groupView);
645647
break;
648+
case GroupModelChangeKind.GROUP_LABEL:
649+
this._onDidChangeGroupLabel.fire(groupView);
650+
break;
646651
}
647652
}));
648653

@@ -1222,6 +1227,12 @@ export class EditorPart extends Part implements IEditorPart {
12221227
this.getGroups(GroupsOrder.GRID_APPEARANCE).forEach((group, index) => group.notifyIndexChanged(index));
12231228
}
12241229

1230+
notifyGroupsLabelChange(newLabel: string) {
1231+
for (const group of this.groups) {
1232+
group.notifyLabelChanged(newLabel);
1233+
}
1234+
}
1235+
12251236
private get isEmpty(): boolean {
12261237
return this.count === 1 && this._activeGroup.isEmpty;
12271238
}
@@ -1330,13 +1341,14 @@ export class MainEditorPart extends EditorPart {
13301341

13311342
export class AuxiliaryEditorPart extends EditorPart implements IAuxiliaryEditorPart {
13321343

1333-
private static COUNTER = 0;
1344+
private static COUNTER = 1;
13341345

13351346
private readonly _onDidClose = this._register(new Emitter<void>());
13361347
readonly onDidClose = this._onDidClose.event;
13371348

13381349
constructor(
13391350
editorPartsView: IEditorPartsView,
1351+
groupsLabel: string,
13401352
@IInstantiationService instantiationService: IInstantiationService,
13411353
@IThemeService themeService: IThemeService,
13421354
@IConfigurationService configurationService: IConfigurationService,
@@ -1345,7 +1357,7 @@ export class AuxiliaryEditorPart extends EditorPart implements IAuxiliaryEditorP
13451357
@IHostService hostService: IHostService
13461358
) {
13471359
const id = AuxiliaryEditorPart.COUNTER++;
1348-
super(editorPartsView, `workbench.parts.auxiliaryEditor.${id}`, localize('auxiliaryEditorPartLabel', "Window {0}", id + 1), instantiationService, themeService, configurationService, storageService, layoutService, hostService);
1360+
super(editorPartsView, `workbench.parts.auxiliaryEditor.${id}`, groupsLabel, instantiationService, themeService, configurationService, storageService, layoutService, hostService);
13491361
}
13501362

13511363
protected override saveState(): void {

‎src/vs/workbench/browser/parts/editor/editorParts.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6+
import { localize } from 'vs/nls';
67
import { EditorGroupLayout, GroupDirection, GroupOrientation, GroupsArrangement, GroupsOrder, IAuxiliaryEditorPart, IEditorDropTargetDelegate, IEditorGroupsService, IEditorSideGroup, IFindGroupScope, IMergeGroupOptions } from 'vs/workbench/services/editor/common/editorGroupsService';
78
import { Event, Emitter } from 'vs/base/common/event';
89
import { getActiveDocument } from 'vs/base/browser/dom';
@@ -50,7 +51,7 @@ export class EditorParts extends Disposable implements IEditorGroupsService, IEd
5051
partContainer.setAttribute('role', 'main');
5152
auxiliaryWindow.container.appendChild(partContainer);
5253

53-
const editorPart = disposables.add(this.instantiationService.createInstance(AuxiliaryEditorPart, this));
54+
const editorPart = disposables.add(this.instantiationService.createInstance(AuxiliaryEditorPart, this, this.getGroupsLabel(this.parts.size)));
5455
disposables.add(this.registerEditorPart(editorPart));
5556

5657
disposables.add(Event.once(editorPart.onDidClose)(() => disposables.dispose()));
@@ -78,13 +79,28 @@ export class EditorParts extends Disposable implements IEditorGroupsService, IEd
7879
this.parts.add(part);
7980

8081
const disposables = this._register(new DisposableStore());
81-
disposables.add(toDisposable(() => this.parts.delete(part)));
82+
disposables.add(toDisposable(() => this.unregisterEditorPart(part)));
8283

8384
this.registerEditorPartListeners(part, disposables);
8485

8586
return disposables;
8687
}
8788

89+
private unregisterEditorPart(part: EditorPart): void {
90+
this.parts.delete(part);
91+
92+
// Notify all parts about a groups label change
93+
// given it is computed based on the index
94+
95+
Array.from(this.parts).forEach((part, index) => {
96+
if (part === this.mainPart) {
97+
return;
98+
}
99+
100+
part.notifyGroupsLabelChange(this.getGroupsLabel(index));
101+
});
102+
}
103+
88104
private registerEditorPartListeners(part: EditorPart, disposables: DisposableStore): void {
89105
disposables.add(part.onDidFocus(() => {
90106
if (this.parts.size > 1) {
@@ -103,6 +119,10 @@ export class EditorParts extends Disposable implements IEditorGroupsService, IEd
103119
disposables.add(part.onDidChangeGroupLocked(group => this._onDidChangeGroupLocked.fire(group)));
104120
}
105121

122+
private getGroupsLabel(index: number): string {
123+
return localize('groupLabel', "Window {0}", index + 1);
124+
}
125+
106126
//#endregion
107127

108128
//#region Helpers

‎src/vs/workbench/common/editor.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,6 +1073,7 @@ export const enum GroupModelChangeKind {
10731073
/* Group Changes */
10741074
GROUP_ACTIVE,
10751075
GROUP_INDEX,
1076+
GROUP_LABEL,
10761077
GROUP_LOCKED,
10771078

10781079
/* Editor Changes */

‎src/vs/workbench/common/editor/editorGroupModel.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -685,6 +685,14 @@ export class EditorGroupModel extends Disposable implements IEditorGroupModel {
685685
this._onDidModelChange.fire({ kind: GroupModelChangeKind.GROUP_INDEX });
686686
}
687687

688+
setLabel(label: string) {
689+
// We do not really keep the `label` in our model because
690+
// it has no special meaning to us here. But for consistency
691+
// we emit a `onDidModelChange` event so that components can
692+
// react.
693+
this._onDidModelChange.fire({ kind: GroupModelChangeKind.GROUP_LABEL });
694+
}
695+
688696
pin(candidate: EditorInput): EditorInput | undefined {
689697
const res = this.findEditor(candidate);
690698
if (!res) {

‎src/vs/workbench/contrib/files/browser/views/openEditorsView.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ export class OpenEditorsView extends ViewPane {
131131
this.focusActiveEditor();
132132
break;
133133
case GroupModelChangeKind.GROUP_INDEX:
134+
case GroupModelChangeKind.GROUP_LABEL:
134135
if (index >= 0) {
135136
this.list.splice(index, 1, [group]);
136137
}

‎src/vs/workbench/contrib/terminal/browser/terminalActions.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editor
6363
import { Iterable } from 'vs/base/common/iterator';
6464
import { AccessibleViewProviderId, accessibleViewCurrentProviderId, accessibleViewIsShown, accessibleViewOnLastLine } from 'vs/workbench/contrib/accessibility/browser/accessibilityConfiguration';
6565
import { isKeyboardEvent, isMouseEvent, isPointerEvent } from 'vs/base/browser/dom';
66+
import { editorGroupToColumn } from 'vs/workbench/services/editor/common/editorGroupColumn';
6667

6768
export const switchTerminalActionViewItemSeparator = '\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500';
6869
export const switchTerminalShowTabsTitle = localize('showTerminalTabs', "Show Tabs");
@@ -266,7 +267,7 @@ export function registerTerminalActions() {
266267
// called when a terminal is the active editor
267268
const editorGroupsService = accessor.get(IEditorGroupsService);
268269
const instance = await c.service.createTerminal({
269-
location: { viewColumn: editorGroupsService.activeGroup.index }
270+
location: { viewColumn: editorGroupToColumn(editorGroupsService, editorGroupsService.activeGroup) }
270271
});
271272
await instance.focusWhenReady();
272273
}

‎src/vs/workbench/services/editor/test/browser/editorGroupsService.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,57 @@ suite('EditorGroupsService', () => {
316316
groupIndexChangedListener.dispose();
317317
});
318318

319+
test('groups label', async function () {
320+
const [part] = await createPart();
321+
322+
const rootGroup = part.groups[0];
323+
const rightGroup = part.addGroup(rootGroup, GroupDirection.RIGHT);
324+
325+
let partLabelChangedCounter = 0;
326+
const groupIndexChangedListener = part.onDidChangeGroupLabel(() => {
327+
partLabelChangedCounter++;
328+
});
329+
330+
let rootGroupLabelChangeCounter = 0;
331+
const rootGroupLabelChangeListener = rootGroup.onDidModelChange(e => {
332+
if (e.kind === GroupModelChangeKind.GROUP_LABEL) {
333+
rootGroupLabelChangeCounter++;
334+
}
335+
});
336+
337+
let rightGroupLabelChangeCounter = 0;
338+
const rightGroupLabelChangeListener = rightGroup.onDidModelChange(e => {
339+
if (e.kind === GroupModelChangeKind.GROUP_LABEL) {
340+
rightGroupLabelChangeCounter++;
341+
}
342+
});
343+
344+
assert.strictEqual(rootGroup.label, 'Group 1');
345+
assert.strictEqual(rightGroup.label, 'Group 2');
346+
347+
part.notifyGroupsLabelChange('Window 2');
348+
349+
assert.strictEqual(rootGroup.label, 'Window 2: Group 1');
350+
assert.strictEqual(rightGroup.label, 'Window 2: Group 2');
351+
352+
assert.strictEqual(rootGroupLabelChangeCounter, 1);
353+
assert.strictEqual(rightGroupLabelChangeCounter, 1);
354+
assert.strictEqual(partLabelChangedCounter, 2);
355+
356+
part.notifyGroupsLabelChange('Window 3');
357+
358+
assert.strictEqual(rootGroup.label, 'Window 3: Group 1');
359+
assert.strictEqual(rightGroup.label, 'Window 3: Group 2');
360+
361+
assert.strictEqual(rootGroupLabelChangeCounter, 2);
362+
assert.strictEqual(rightGroupLabelChangeCounter, 2);
363+
assert.strictEqual(partLabelChangedCounter, 4);
364+
365+
rootGroupLabelChangeListener.dispose();
366+
rightGroupLabelChangeListener.dispose();
367+
groupIndexChangedListener.dispose();
368+
});
369+
319370
test('copy/merge groups', async () => {
320371
const [part] = await createPart();
321372

‎src/vs/workbench/test/browser/parts/editor/editorGroupModel.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ suite('EditorGroupModel', () => {
101101
locked: number[];
102102
active: number[];
103103
index: number[];
104+
label: number[];
104105
opened: IGroupEditorOpenEvent[];
105106
activated: IGroupEditorChangeEvent[];
106107
closed: IGroupEditorCloseEvent[];
@@ -116,6 +117,7 @@ suite('EditorGroupModel', () => {
116117
const groupEvents: GroupEvents = {
117118
active: [],
118119
index: [],
120+
label: [],
119121
locked: [],
120122
opened: [],
121123
closed: [],
@@ -138,6 +140,9 @@ suite('EditorGroupModel', () => {
138140
} else if (e.kind === GroupModelChangeKind.GROUP_INDEX) {
139141
groupEvents.index.push(group.id);
140142
return;
143+
} else if (e.kind === GroupModelChangeKind.GROUP_LABEL) {
144+
groupEvents.label.push(group.id);
145+
return;
141146
}
142147
if (!e.editor) {
143148
return;
@@ -807,6 +812,17 @@ suite('EditorGroupModel', () => {
807812
assert.strictEqual(events.index.length, 1);
808813
});
809814

815+
test('label', function () {
816+
const group = createEditorGroupModel();
817+
const events = groupListener(group);
818+
819+
assert.strictEqual(events.label.length, 0);
820+
821+
group.setLabel('Window 1');
822+
823+
assert.strictEqual(events.label.length, 1);
824+
});
825+
810826
test('active', function () {
811827
const group = createEditorGroupModel();
812828
const events = groupListener(group);

0 commit comments

Comments
 (0)