Centered editor layout #52064

isidorn · 2018-06-15T15:52:55Z

Brings back centered editor layout support to VSCode.
This is work in progress, creating PR now to potentially get early feedback.

Still needs to be done:

double check layout of the grid - seems to overflow to the right
add separator style (due to overflow right separtor is hidden)
background color of left and right empty areas should have the color of the editor

…tor part

bpasero · 2018-06-15T16:23:34Z

@isidorn cool, I guess something is off with the background color?

There is also a bit weirdness when running this when no editor is opened:

bpasero · 2018-06-15T16:25:32Z

Also I would not mind if the centered widget was always used and when centered is off it would just fill the whole space and not allow to drag the sashes and when enabled it toggles. That way we do not need to perform any reparenting when switching modes.

joaomoreno · 2018-06-18T08:57:05Z

@isidorn I would merge master into this, since there was an ugly issue which I just fixed this morning.

isidorn · 2018-06-18T09:57:12Z

I have merged master.
I am now also centering the layout by pushing empty views, not reparenting html elements - so it is more elegent.

Still needs some polish from my first comment, once I address that I plan to merge this in if nobody objects

joaomoreno · 2018-06-18T10:10:18Z

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

@@ -967,7 +985,11 @@ export class EditorPart extends Part implements EditorGroupsServiceImpl, IEditor

 		// Layout Grid
 		try {
-			this.gridWidget.layout(this.dimension.width, this.dimension.height);
+			if (this.centeredViewLayout) {


Isn't there always a centeredViewLayout? This will always be true, it seems.

joaomoreno · 2018-06-18T10:13:11Z

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

 		this.doCreateGridControl();
+		this.centeredViewLayout = new CenteredViewLayout(this.container, {
+			element: this.gridWidget.element,
+			layout: size => this.gridWidget.layout(size, this.dimension ? this.dimension.height : this.gridWidget.maximumHeight),


This looks super weird. There must always be a this.dimension. It's impossible for this view to be asked for layout before doLayout has run.

Also, if you want to remove the weirdness, don't simply use IView from splitview. Use IView from gridview. This one talks about width and height. Simultaneously, CenteredViewLayout.layout would take in both width and height.

joaomoreno · 2018-06-18T10:13:49Z

src/vs/base/browser/ui/centered/centeredViewLayout.ts

+	}
+
+	activate(active: boolean): void {
+		this.active = active;


You probably also need if (this.active === active) { return; } before this.

joaomoreno · 2018-06-18T10:14:34Z

src/vs/base/browser/ui/centered/centeredViewLayout.ts

+			};
+
+			this.splitView.addView(emptyView, Sizing.Distribute, 0);
+			this.splitView.addView(emptyView, Sizing.Distribute);


You need to add two different views... How can that DOM element be put twice in the DOM?

joaomoreno · 2018-06-18T10:15:24Z

src/vs/base/browser/ui/centered/centeredViewLayout.ts

+		this.active = active;
+		if (this.active) {
+			const emptyView = {
+				element: $('.'),


$('.') doesn't mean anything. If you wan a div, use $('div'). Though you probably should use something descriptive like $('.centered-layout-margin').

joaomoreno · 2018-06-18T10:16:35Z

src/vs/base/browser/ui/centered/centeredViewLayout.ts

+			this.splitView.addView(emptyView, Sizing.Distribute);
+		} else {
+			this.splitView.removeView(0);
+			this.splitView.removeView(1);


Why not disposing the splitview altogether and just put the provided view directly as a child of the container?

joaomoreno · 2018-06-18T10:17:06Z

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

@@ -1009,6 +1031,9 @@ export class EditorPart extends Part implements EditorGroupsServiceImpl, IEditor
 		if (this.gridWidget) {
 			this.gridWidget.dispose();
 		}
+		if (this.centeredViewLayout) {
+			this.centeredViewLayout = dispose(this.centeredViewLayout);
+		}


All you need is this.centeredViewLayout.dispose().

isidorn · 2018-06-18T13:28:04Z

@joaomoreno thanks for the review, I have addressed your feedback. There are still some nit-bits to take care of which I am looking into now.

What I find a bit ugly is that for some methods the editorPart goes directly to the gridWidget and for some it goes to the centeredLayout instead which forwards to the gridWidget.

joaomoreno · 2018-06-18T13:31:23Z

src/vs/base/browser/ui/centered/centeredViewLayout.ts

+			element: this.view.element,
+			maximumSize: this.view.maximumWidth,
+			minimumSize: this.view.minimumWidth,
+			onDidChange: Event.None,


You should expose this.view.onDidChange here.

joaomoreno · 2018-06-18T13:32:55Z

src/vs/base/browser/ui/centered/centeredViewLayout.ts

+		this.view = view;
+		if (this.splitView) {
+			this.splitView.removeView(1);
+			this.splitView.addView(this.getView(), Sizing.Distribute, 1);


There is splitview.distributeViewSizes().

joaomoreno · 2018-06-18T13:34:19Z

src/vs/base/browser/ui/centered/centeredViewLayout.ts

-		if (this.active) {
-			const emptyView = {
-				element: $('.'),
+		if (active) {


You should still make a if (active === !!this.splitview) { return; }, so you don't get any NPEs.

isidorn · 2018-06-18T13:44:07Z

@joaomoreno thanks - great feedback, I have addressed that feedback in the latest commit.
There are still some issues with the watermark that I want to address before merging in

isidorn added 2 commits June 15, 2018 17:49

add centeredViewLayout

a86d0e5

editorPart: plugin centeredViewLayout

98e0b5b

isidorn added this to the June 2018 milestone Jun 15, 2018

isidorn assigned joaomoreno and bpasero Jun 15, 2018

update the content class name depending on who is direct chald of edi…

e362c61

…tor part

isidorn added 2 commits June 18, 2018 11:38

merge master

afcd1fc

center layout by adding empty views, do not reparent html elements

151790c

centered layout: respect separator style

01bafa1

joaomoreno requested changes Jun 18, 2018

View reviewed changes

centerd view: address feedback

899aef9

joaomoreno requested changes Jun 18, 2018

View reviewed changes

properly pass onDidChange and react to other feedback

2370f86

isidorn added 4 commits June 18, 2018 16:10

use helper functino to clarify toSplitViewView cast

8f7a642

centeredViewLayout: height 100% to show watermark

4d2c0dd

Merge branch 'master' into isidorn/centeredLayout

906007f

centeredView: store ratios and restore them properly

8527801

isidorn merged commit 6a1ca40 into master Jun 19, 2018

isidorn deleted the isidorn/centeredLayout branch June 19, 2018 08:53

github-actions bot locked and limited conversation to collaborators Mar 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Centered editor layout #52064

Centered editor layout #52064

isidorn commented Jun 15, 2018 •

edited

Loading

bpasero commented Jun 15, 2018

bpasero commented Jun 15, 2018

joaomoreno commented Jun 18, 2018 •

edited

Loading

isidorn commented Jun 18, 2018

joaomoreno Jun 18, 2018

joaomoreno Jun 18, 2018

joaomoreno Jun 18, 2018

joaomoreno Jun 18, 2018

joaomoreno Jun 18, 2018

joaomoreno Jun 18, 2018

joaomoreno Jun 18, 2018

isidorn commented Jun 18, 2018 •

edited

Loading

joaomoreno Jun 18, 2018

joaomoreno Jun 18, 2018

joaomoreno Jun 18, 2018

isidorn commented Jun 18, 2018 •

edited

Loading

Centered editor layout #52064

Centered editor layout #52064

Conversation

isidorn commented Jun 15, 2018 • edited Loading

bpasero commented Jun 15, 2018

bpasero commented Jun 15, 2018

joaomoreno commented Jun 18, 2018 • edited Loading

isidorn commented Jun 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isidorn commented Jun 18, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

isidorn commented Jun 18, 2018 • edited Loading

isidorn commented Jun 15, 2018 •

edited

Loading

joaomoreno commented Jun 18, 2018 •

edited

Loading

isidorn commented Jun 18, 2018 •

edited

Loading

isidorn commented Jun 18, 2018 •

edited

Loading