Merge pull request #334864 from microsoft/mrleemurray/fix-sidebar-width-reset-on-toggle

Fix primary side bar width reset when toggled with the side bar on the right

Author: mrleemurray

src/vs/base/browser/ui/splitview/splitview.ts

@@ -1017,7 +1017,13 @@ export class SplitView<TLayoutContext = undefined, TView extends IView<TLayoutCo
 			this.layoutViews();
 		} else {
 			item.size = size;
-			this.relayout([index], undefined);
+
+			// Absorb the change by layout priority, like `resizeView` does, so a neighbour with a size of its own is not squeezed (see #334167)
+			const indexes = range(this.viewItems.length).filter(i => i !== index);
+			const lowPriorityIndexes = [...indexes.filter(i => this.viewItems[i].priority === LayoutPriority.Low), index];
+			const highPriorityIndexes = indexes.filter(i => this.viewItems[i].priority === LayoutPriority.High);
+
+			this.relayout(lowPriorityIndexes, highPriorityIndexes);
 		}
 	}
 

src/vs/base/test/browser/ui/splitview/splitview.test.ts

@@ -36,6 +36,9 @@ class TestView implements IView<number> {
 	private readonly _onDidFocus = new Emitter<void>();
 	readonly onDidFocus = this._onDidFocus.event;
 
+	private readonly _onDidSetVisible = new Emitter<boolean>();
+	readonly onDidSetVisible = this._onDidSetVisible.event;
+
 	constructor(
 		private _minimumSize: number,
 		private _maximumSize: number,
@@ -44,6 +47,17 @@ class TestView implements IView<number> {
 		assert(_minimumSize <= _maximumSize, 'splitview view minimum size must be <= maximum size');
 	}
 
+	/** Change both size constraints at once, as a view with a fixed width does when that width changes. */
+	setSizeConstraints(minimumSize: number, maximumSize: number): void {
+		this._minimumSize = minimumSize;
+		this._maximumSize = maximumSize;
+		this._onDidChange.fire(undefined);
+	}
+
+	setVisible(visible: boolean): void {
+		this._onDidSetVisible.fire(visible);
+	}
+
 	layout(size: number, _offset: number, orthogonalSize: number | undefined): void {
 		this._size = size;
 		this._orthogonalSize = orthogonalSize;
@@ -59,6 +73,7 @@ class TestView implements IView<number> {
 		this._onDidGetElement.dispose();
 		this._onDidLayout.dispose();
 		this._onDidFocus.dispose();
+		this._onDidSetVisible.dispose();
 	}
 }
 
@@ -459,6 +474,32 @@ suite('Splitview', () => {
 		assert.deepStrictEqual([view1.size, view2.size, view3.size], [20, 160, 20]);
 	});
 
+	test('view changing its size constraints while a sibling is toggled does not squeeze that sibling (#334167)', () => {
+
+		// Mirrors the workbench with the side bar on the right: the activity bar reports its new fixed width from within the visibility change of the side bar
+		const editor = store.add(new TestView(20, Number.POSITIVE_INFINITY, LayoutPriority.High));
+		const sideBar = store.add(new TestView(20, Number.POSITIVE_INFINITY, LayoutPriority.Low));
+		const activityBar = store.add(new TestView(10, 10));
+		const splitview = store.add(new SplitView(container, { proportionalLayout: false }));
+		splitview.layout(200);
+
+		splitview.addView(editor, 120);
+		splitview.addView(sideBar, 70);
+		splitview.addView(activityBar, 10);
+
+		// `addView` distributes in index order too, so restore the side bar to a known starting point
+		splitview.resizeView(1, 70);
+		assert.deepStrictEqual([editor.size, sideBar.size, activityBar.size], [120, 70, 10]);
+
+		store.add(sideBar.onDidSetVisible(visible => activityBar.setSizeConstraints(visible ? 10 : 12, visible ? 10 : 12)));
+
+		splitview.setViewVisible(1, false);
+		assert.deepStrictEqual([editor.size, sideBar.size, activityBar.size], [188, 0, 12]);
+
+		splitview.setViewVisible(1, true);
+		assert.deepStrictEqual([editor.size, sideBar.size, activityBar.size], [120, 70, 10]);
+	});
+
 	test('context propagates to views', () => {
 		const view1 = store.add(new TestView(20, Number.POSITIVE_INFINITY));
 		const view2 = store.add(new TestView(20, Number.POSITIVE_INFINITY));