address feedback

3b974dec · isidor · f19ae63f · 3b974dec · 3b974dec · 3b974dec
4 changed file
--- a/src/vs/base/browser/ui/grid/grid.ts
+++ b/src/vs/base/browser/ui/grid/grid.ts
@@ -199,7 +199,7 @@ export class Grid<T extends IView> implements IDisposable {
 	public sashResetSizing: Sizing = Sizing.Distribute;

 	get container(): HTMLElement {
-		return this.gridview.container;
+		return this.gridview.element;
 	}

 	constructor(view: T, options: IGridOptions = {}) {

--- a/src/vs/base/browser/ui/grid/gridview.ts
+++ b/src/vs/base/browser/ui/grid/gridview.ts
@@ -10,7 +10,7 @@ import { Event, anyEvent, Emitter, mapEvent, Relay } from 'vs/base/common/event'
 import { Orientation, Sash } from 'vs/base/browser/ui/sash/sash';
 import { SplitView, IView as ISplitView, Sizing, ISplitViewStyles } from 'vs/base/browser/ui/splitview/splitview';
 import { empty as EmptyDisposable, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
-import { $, append } from 'vs/base/browser/dom';
+import { $ } from 'vs/base/browser/dom';
 import { tail2 as tail } from 'vs/base/common/arrays';
 import { Color } from 'vs/base/common/color';

@@ -418,7 +418,7 @@ function flipNode<T extends Node>(node: T, size: number, orthogonalSize: number)

 export class GridView implements IDisposable {

-	private element: HTMLElement;
+	readonly element: HTMLElement;
 	private styles: IGridViewStyles;

 	private _root: BranchNode;
@@ -483,11 +483,8 @@ export class GridView implements IDisposable {
 		return this.root.maximumHeight;
 	}

-	readonly container: HTMLElement;
-
 	constructor(options: IGridViewOptions = {}) {
-		this.container = $('.');
-		this.element = append(this.container, $('.monaco-grid-view'));
+		this.element = $('.monaco-grid-view');
 		this.styles = options.styles || defaultStyles;
 		this.root = new BranchNode(Orientation.VERTICAL, this.styles);
 	}
@@ -760,7 +757,5 @@ export class GridView implements IDisposable {
 		if (this.element && this.element.parentElement) {
 			this.element.parentElement.removeChild(this.element);
 		}
-
-		this.element = null;
 	}
 }
--- a/src/vs/base/test/browser/ui/grid/grid.test.ts
+++ b/src/vs/base/test/browser/ui/grid/grid.test.ts
@@ -4,20 +4,23 @@
 *--------------------------------------------------------------------------------------------*/

 import * as assert from 'assert';
-import { Direction, Grid, getRelativeLocation, Orientation, SerializableGrid, ISerializableView, IViewDeserializer, GridNode, Sizing, isGridBranchNode, sanitizeGridNodeDescriptor, GridNodeDescriptor, createSerializedGrid } from 'vs/base/browser/ui/grid/grid';
+import { Direction, getRelativeLocation, Orientation, SerializableGrid, ISerializableView, IViewDeserializer, GridNode, Sizing, isGridBranchNode, sanitizeGridNodeDescriptor, GridNodeDescriptor, createSerializedGrid, Grid } from 'vs/base/browser/ui/grid/grid';
 import { TestView, nodesToArrays } from './util';
 import { deepClone } from 'vs/base/common/objects';

-function setupGrid(view: TestView): Grid<TestView> {
-	const grid = new Grid(view);
-	grid.container.style.position = 'absolute';
-	grid.container.style.width = `${800}px`;
-	grid.container.style.height = `${600}px`;
+suite('Grid', function () {
+	let container: HTMLElement;

-	return grid;
-}
+	setup(function () {
+		container = document.createElement('div');
+		container.style.position = 'absolute';
+		container.style.width = `${800}px`;
+		container.style.height = `${600}px`;
+	});

-suite('Grid', function () {
+	teardown(function () {
+		container = null;
+	});

 	test('getRelativeLocation', function () {
 		assert.deepEqual(getRelativeLocation(Orientation.VERTICAL, [0], Direction.Up), [0]);
@@ -53,7 +56,8 @@ suite('Grid', function () {

 	test('empty', function () {
 		const view1 = new TestView(100, Number.MAX_VALUE, 100, Number.MAX_VALUE);
-		const gridview = setupGrid(view1);
+		const gridview = new Grid(view1);
+		container.appendChild(gridview.container);
 		gridview.layout(800, 600);

 		assert.deepEqual(view1.size, [800, 600]);
@@ -61,7 +65,8 @@ suite('Grid', function () {

 	test('two views vertically', function () {
 		const view1 = new TestView(50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupGrid(view1);
+		const grid = new Grid(view1);
+		container.appendChild(grid.container);
 		grid.layout(800, 600);
 		assert.deepEqual(view1.size, [800, 600]);

@@ -73,7 +78,9 @@ suite('Grid', function () {

 	test('two views horizontally', function () {
 		const view1 = new TestView(50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupGrid(view1);
+		const grid = new Grid(view1);
+		container.appendChild(grid.container);
+
 		grid.layout(800, 600);
 		assert.deepEqual(view1.size, [800, 600]);

@@ -85,7 +92,9 @@ suite('Grid', function () {

 	test('simple layout', function () {
 		const view1 = new TestView(50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupGrid(view1);
+		const grid = new Grid(view1);
+		container.appendChild(grid.container);
+
 		grid.layout(800, 600);
 		assert.deepEqual(view1.size, [800, 600]);

@@ -118,7 +127,8 @@ suite('Grid', function () {

 	test('another simple layout with automatic size distribution', function () {
 		const view1 = new TestView(50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupGrid(view1);
+		const grid = new Grid(view1);
+		container.appendChild(grid.container);

 		grid.layout(800, 600);
 		assert.deepEqual(view1.size, [800, 600]);
@@ -161,7 +171,9 @@ suite('Grid', function () {

 	test('another simple layout with split size distribution', function () {
 		const view1 = new TestView(50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupGrid(view1);
+		const grid = new Grid(view1);
+		container.appendChild(grid.container);
+
 		grid.layout(800, 600);
 		assert.deepEqual(view1.size, [800, 600]);

@@ -203,7 +215,8 @@ suite('Grid', function () {

 	test('3/2 layout with split', function () {
 		const view1 = new TestView(50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupGrid(view1);
+		const grid = new Grid(view1);
+		container.appendChild(grid.container);

 		grid.layout(800, 600);
 		assert.deepEqual(view1.size, [800, 600]);
@@ -237,7 +250,8 @@ suite('Grid', function () {

 	test('sizing should be correct after branch demotion #50564', function () {
 		const view1 = new TestView(50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupGrid(view1);
+		const grid = new Grid(view1);
+		container.appendChild(grid.container);

 		grid.layout(800, 600);

@@ -262,7 +276,8 @@ suite('Grid', function () {

 	test('sizing should be correct after branch demotion #50675', function () {
 		const view1 = new TestView(50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupGrid(view1);
+		const grid = new Grid(view1);
+		container.appendChild(grid.container);

 		grid.layout(800, 600);

@@ -287,7 +302,8 @@ suite('Grid', function () {

 	test('getNeighborViews should work on single view layout', function () {
 		const view1 = new TestView(50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupGrid(view1);
+		const grid = new Grid(view1);
+		container.appendChild(grid.container);

 		grid.layout(800, 600);

@@ -304,7 +320,8 @@ suite('Grid', function () {

 	test('getNeighborViews should work on simple layout', function () {
 		const view1 = new TestView(50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupGrid(view1);
+		const grid = new Grid(view1);
+		container.appendChild(grid.container);

 		grid.layout(800, 600);

@@ -347,7 +364,8 @@ suite('Grid', function () {

 	test('getNeighborViews should work on a complex layout', function () {
 		const view1 = new TestView(50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupGrid(view1);
+		const grid = new Grid(view1);
+		container.appendChild(grid.container);

 		grid.layout(800, 600);

@@ -387,7 +405,8 @@ suite('Grid', function () {

 	test('getNeighborViews should work on another simple layout', function () {
 		const view1 = new TestView(50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupGrid(view1);
+		const grid = new Grid(view1);
+		container.appendChild(grid.container);

 		grid.layout(800, 600);

@@ -408,7 +427,8 @@ suite('Grid', function () {

 	test('getNeighborViews should only return immediate neighbors', function () {
 		const view1 = new TestView(50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupGrid(view1);
+		const grid = new Grid(view1);
+		container.appendChild(grid.container);

 		grid.layout(800, 600);

@@ -465,19 +485,25 @@ function nodesToNames(node: GridNode<TestSerializableView>): any {
 	}
 }

-function setupSerializedGrid(view: TestSerializableView): SerializableGrid<TestSerializableView> {
-	const grid = new SerializableGrid(view);
-	grid.container.style.position = 'absolute';
-	grid.container.style.width = `${800}px`;
-	grid.container.style.height = `${600}px`;
+suite('SerializableGrid', function () {

-	return grid;
-}
+	let container: HTMLElement;
+
+	setup(function () {
+		container = document.createElement('div');
+		container.style.position = 'absolute';
+		container.style.width = `${800}px`;
+		container.style.height = `${600}px`;
+	});
+
+	teardown(function () {
+		container = null;
+	});

-suite('SerializableGrid', function () {
 	test('serialize empty', function () {
 		const view1 = new TestSerializableView('view1', 50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupSerializedGrid(view1);
+		const grid = new SerializableGrid(view1);
+		container.appendChild(grid.container);
 		grid.layout(800, 600);

 		assert.deepEqual(grid.serialize(), {
@@ -502,7 +528,8 @@ suite('SerializableGrid', function () {

 	test('serialize simple layout', function () {
 		const view1 = new TestSerializableView('view1', 50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupSerializedGrid(view1);
+		const grid = new SerializableGrid(view1);
+		container.appendChild(grid.container);
 		grid.layout(800, 600);

 		const view2 = new TestSerializableView('view2', 50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
@@ -555,7 +582,8 @@ suite('SerializableGrid', function () {

 	test('deserialize empty', function () {
 		const view1 = new TestSerializableView('view1', 50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupSerializedGrid(view1);
+		const grid = new SerializableGrid(view1);
+		container.appendChild(grid.container);
 		grid.layout(800, 600);

 		const json = grid.serialize();
@@ -570,7 +598,8 @@ suite('SerializableGrid', function () {

 	test('deserialize simple layout', function () {
 		const view1 = new TestSerializableView('view1', 50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupSerializedGrid(view1);
+		const grid = new SerializableGrid(view1);
+		container.appendChild(grid.container);

 		grid.layout(800, 600);

@@ -617,7 +646,8 @@ suite('SerializableGrid', function () {

 	test('deserialize simple layout with scaling', function () {
 		const view1 = new TestSerializableView('view1', 50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupSerializedGrid(view1);
+		const grid = new SerializableGrid(view1);
+		container.appendChild(grid.container);

 		grid.layout(800, 600);

@@ -655,7 +685,8 @@ suite('SerializableGrid', function () {

 	test('deserialize 4 view layout (ben issue #2)', function () {
 		const view1 = new TestSerializableView('view1', 50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupSerializedGrid(view1);
+		const grid = new SerializableGrid(view1);
+		container.appendChild(grid.container);
 		grid.layout(800, 600);

 		const view2 = new TestSerializableView('view2', 50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
@@ -688,7 +719,8 @@ suite('SerializableGrid', function () {

 	test('deserialize 2 view layout (ben issue #3)', function () {
 		const view1 = new TestSerializableView('view1', 50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupSerializedGrid(view1);
+		const grid = new SerializableGrid(view1);
+		container.appendChild(grid.container);

 		grid.layout(800, 600);

@@ -712,7 +744,8 @@ suite('SerializableGrid', function () {

 	test('deserialize simple view layout #50609', function () {
 		const view1 = new TestSerializableView('view1', 50, Number.MAX_VALUE, 50, Number.MAX_VALUE);
-		const grid = setupSerializedGrid(view1);
+		const grid = new SerializableGrid(view1);
+		container.appendChild(grid.container);

 		grid.layout(800, 600);


--- a/src/vs/base/test/browser/ui/grid/gridview.test.ts
+++ b/src/vs/base/test/browser/ui/grid/gridview.test.ts
@@ -4,6 +4,7 @@
 *--------------------------------------------------------------------------------------------*/

 import * as assert from 'assert';
+import { $ } from 'vs/base/browser/dom';
 import { GridView, IView, Sizing } from 'vs/base/browser/ui/grid/gridview';
 import { nodesToArrays, TestView } from './util';

@@ -12,9 +13,12 @@ suite('Gridview', function () {

 	setup(function () {
 		gridview = new GridView();
-		gridview.container.style.position = 'absolute';
-		gridview.container.style.width = `${200}px`;
-		gridview.container.style.height = `${200}px`;
+		const container = $('.container');
+
+		container.style.position = 'absolute';
+		container.style.width = `${200}px`;
+		container.style.height = `${200}px`;
+		container.appendChild(gridview.element);
 	});

 	teardown(function () {