Adds support for moving the tab close button to the left #17863

reaktivo · 2016-12-27T23:18:41Z

I've added support for moving the tab close button to the left of the tab. I've implemented it in the simplest way possible, by adding a new setting called tabCloseButtonOnLeft. But I'm open to opinions regarding the possibility to refactor the showTabCloseButton and tabCloseButtonOnLeft into a single setting specifying if it's hidden or on which side should it be displayed.

Fixes #15833

msftclas · 2016-12-27T23:18:44Z

Hi @reaktivo, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

reaktivo · 2016-12-28T08:38:14Z

I've signed the CLA.

… the left

msftclas · 2017-01-02T10:47:34Z

Hi @reaktivo, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

bpasero · 2017-01-04T11:22:11Z

@reaktivo good stuff, elegant CSS solution 👍 . Now that I see both workbench.editor.showTabCloseButton and workbench.editor.tabCloseButtonOnLeft I suggest to merge both into one with an enum, like:
workbench.editor.tabCloseButton: 'left' | 'right' | 'off'

I would still support workbench.editor.showTabCloseButton by treating it as workbench.editor.tabCloseButton: 'off' for backwards compatibility.

bpasero · 2017-01-13T11:49:28Z

@reaktivo are you still working on this?

Also uses the tab close button option to position the dirty indicator

…left

reaktivo · 2017-01-15T22:11:21Z

@bpasero Yes sure, hadn't had the time to update the PR. I just did and also included a few changes to also move the dirty file indicator when the Hide Close button setting is set to true.

…port-tab-close-button-on-left

bpasero

@reaktivo thanks, I gave some feedback. overall I suggest to remove the workbench.editor.showTabCloseButton setting entirely. We can release note this change for our users.

bpasero · 2017-01-16T07:57:33Z

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

 	private updateTabOptions(tabOptions: ITabOptions, refresh?: boolean): void {
-		const showTabCloseButton = this.tabOptions ? this.tabOptions.showTabCloseButton : false;
+		const tabCloseButton = this.tabOptions ? this.tabOptions.tabCloseButton : 'right';
+		const showTabCloseButton =  this.tabOptions ? this.tabOptions.showTabCloseButton : false;


Just delete support for showTabCloseButton option since it is now supported via your tabCloseButton setting.

bpasero · 2017-01-16T07:57:46Z

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


 				// Refresh title when icons change
-				else if (showingIcons !== this.tabOptions.showIcons || showTabCloseButton !== this.tabOptions.showTabCloseButton) {
+				else if (showingIcons !== this.tabOptions.showIcons || showTabCloseButton !== this.tabOptions.showTabCloseButton || tabCloseButton !== this.tabOptions.tabCloseButton) {


...same here

bpasero · 2017-01-16T07:57:57Z

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

 				showIcons: editorConfig.showIcons,
 				showTabs: editorConfig.showTabs,
-				showTabCloseButton: editorConfig.showTabCloseButton
+				showTabCloseButton: editorConfig.showTabCloseButton,


Just delete support for showTabCloseButton option since it is now supported via your tabCloseButton setting.

bpasero · 2017-01-16T07:59:36Z

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

 		tabContainer.setAttribute('role', 'presentation'); // cannot use role "tab" here due to https://github.com/Microsoft/vscode/issues/8659
 		DOM.addClass(tabContainer, 'tab monaco-editor-background');

+		if (!this.tabOptions.showTabCloseButton || this.tabOptions.tabCloseButton === 'off') {


Please move this into the doUpdate method above where you will notice that there already is an update for no-close-button class from the other option (that should be deleted). We need to have it there because otherwise tabs will not update live from a config change.

bpasero · 2017-01-16T07:59:57Z

src/vs/workbench/common/editor.ts

 		editor: {
 			showTabs: boolean;
 			showTabCloseButton: boolean;
+			tabCloseButton: 'left' | 'right' | 'off';


remove showTabCloseButton

bpasero · 2017-01-16T08:00:10Z

src/vs/workbench/electron-browser/main.contribution.ts

+			'type': 'string',
+			'enum': ['left', 'right', 'off'],
+			'default': 'right',
+			'description': nls.localize('editorTabCloseButton', "Controls the position of the editor's tabs close buttons.")


This should mention what "off" does.

bpasero · 2017-01-16T08:00:32Z

src/vs/workbench/services/group/common/groupService.ts

 export interface ITabOptions {
 	showTabs?: boolean;
 	showTabCloseButton?: boolean;
+	tabCloseButton?: string;


Use string enum here ('left' | 'right' | 'off')

reaktivo · 2017-01-16T12:30:46Z

@bpasero Ok, will update today. We'll loose the (questionably necessary) ability to move the dirty indicator to the left when the close button is disabled though, that's fine right?

bpasero · 2017-01-16T13:38:26Z

@reaktivo I am not sure I understand, here is how I expect this to work:

left: dirty indicator shows on the left hand side
right: dirty indicator shows on the right hand side
off: dirty indicator shows on the right hand side

This would basically keep todays behaviour of how the dirty indicator is showing but adds on top to show it on the left hand side if enabled.

…ions

reaktivo · 2017-01-17T09:50:25Z

@bpasero I've updated the PR with the requested changes, the builds are failing with non-specified errors, can you retrigger the builds?

msftclas · 2017-01-17T09:52:29Z

Hi @reaktivo, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

reaktivo · 2017-01-17T12:14:58Z

@bpasero I rerun the build and it's passing now.

bpasero · 2017-01-17T13:14:57Z

@reaktivo thanks, checking it later today.

bpasero · 2017-01-17T15:31:51Z

@reaktivo ok we are getting there! one thing I would like to have cleaned up is the CSS classes applied to a tab depending on the options. When I look into the CSS I for example see this rule:

.tab.no-close-button.close-position-left

I think this rule should not be possible since we just have 1 setting now that controls everything. Can we not have rules like these:

.tab (default)
.tab.close-button-off
.tab.close-button-left

?

reaktivo · 2017-01-18T09:27:49Z

@bpasero Updated

bpasero · 2017-01-18T10:10:08Z

@reaktivo awesome 👍

billiegoose · 2018-09-01T22:52:35Z

This makes me so happy! Thank you @reaktivo ❤️

msftclas added the cla-required label Dec 27, 2016

reaktivo changed the title ~~Fixes #15833 Adds support for moving the tab close button to the left~~ Adds support for moving the tab close button to the left Dec 27, 2016

Fixes microsoft#15833 Adds support for moving the tab close button to…

b604568

… the left

reaktivo force-pushed the feature-15833-support-tab-close-button-on-left branch from db61f80 to b604568 Compare December 28, 2016 14:46

dbaeumer assigned bpasero Dec 29, 2016

bpasero closed this Jan 2, 2017

bpasero reopened this Jan 2, 2017

msftclas added the cla-already-signed label Jan 2, 2017

bpasero removed the cla-required label Jan 2, 2017

Updates to handle tab close button options: left | right | off

0cd9cda

Also uses the tab close button option to position the dirty indicator

reaktivo force-pushed the feature-15833-support-tab-close-button-on-left branch from 5cce9f2 to 0cd9cda Compare January 15, 2017 21:24

Merge branch 'master' into feature-15833-support-tab-close-button-on-…

0eb737d

…left

Merge remote-tracking branch 'upstream/master' into feature-15833-sup…

2167bc4

…port-tab-close-button-on-left

bpasero requested changes Jan 16, 2017

View reviewed changes

Removes the showTabCloseButton setting in favor of tabCloseButton opt…

520aaf7

…ions

reaktivo closed this Jan 17, 2017

reaktivo reopened this Jan 17, 2017

msftclas added the cla-already-signed label Jan 17, 2017

reaktivo force-pushed the feature-15833-support-tab-close-button-on-left branch from e724c7f to 660457a Compare January 17, 2017 22:45

Simplified selection of tab close button css styles

3eaac7f

reaktivo force-pushed the feature-15833-support-tab-close-button-on-left branch from 660457a to 3eaac7f Compare January 17, 2017 22:51

bpasero merged commit 836afa4 into microsoft:master Jan 18, 2017

bpasero added this to the January 2017 milestone Jan 18, 2017

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

Adds support for moving the tab close button to the left #17863

Adds support for moving the tab close button to the left #17863

Uh oh!

Conversation

reaktivo commented Dec 27, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

msftclas commented Dec 27, 2016

Uh oh!

reaktivo commented Dec 28, 2016

Uh oh!

msftclas commented Jan 2, 2017

Uh oh!

bpasero commented Jan 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bpasero commented Jan 13, 2017

Uh oh!

reaktivo commented Jan 15, 2017

Uh oh!

bpasero left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bpasero Jan 16, 2017

Choose a reason for hiding this comment

Uh oh!

bpasero Jan 16, 2017

Choose a reason for hiding this comment

Uh oh!

bpasero Jan 16, 2017

Choose a reason for hiding this comment

Uh oh!

bpasero Jan 16, 2017

Choose a reason for hiding this comment

Uh oh!

bpasero Jan 16, 2017

Choose a reason for hiding this comment

Uh oh!

bpasero Jan 16, 2017

Choose a reason for hiding this comment

Uh oh!

bpasero Jan 16, 2017

Choose a reason for hiding this comment

Uh oh!

reaktivo commented Jan 16, 2017

Uh oh!

bpasero commented Jan 16, 2017

Uh oh!

reaktivo commented Jan 17, 2017

Uh oh!

msftclas commented Jan 17, 2017

Uh oh!

reaktivo commented Jan 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bpasero commented Jan 17, 2017

Uh oh!

bpasero commented Jan 17, 2017

Uh oh!

reaktivo commented Jan 18, 2017

Uh oh!

bpasero commented Jan 18, 2017

Uh oh!

billiegoose commented Sep 1, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

reaktivo commented Dec 27, 2016 •

edited

Loading

bpasero commented Jan 4, 2017 •

edited

Loading

bpasero left a comment •

edited

Loading

reaktivo commented Jan 17, 2017 •

edited

Loading