Fade out tabs when sizing is set to shrink (fix #39417) #39829

Dari-K · 2017-12-06T21:24:33Z

Don't know how you feel about using prefixed CSS properties. Also there isn't a way to detect a text overflow just with CSS as far as I know, so I've just masked the end of the parent element of the icon label. This seems to stop the fade from affecting tabs that have short enough names not to overflow, not sure it works 100% of the time though.

bpasero · 2017-12-07T06:54:19Z

@Dari-K looks wonderful! however, I fear that using webkit-mask-image will cause Chrome to no longer use subpixel-AA when rendering tabs. See https://www.html5rocks.com/en/tutorials/internals/antialiasing-101/

bpasero · 2017-12-07T09:42:12Z

Yes, it is a problem, compare the following zoomed tab title.

With your change, Chrome renders with greyscale AA:

Without your change, Chrome renders with subpixel AA:

Dari-K · 2017-12-07T11:57:20Z

Was trying to play with this, but as far as I can tell, the stable release has subpixel-AA there, but the insider build/ a HEAD build doesn't, any ideas @bpasero ?
Stable:

Insider:

This is on a mac if it makes any difference.

bpasero · 2017-12-07T12:06:51Z

@Dari-K seems to depend on more factors. can you check when just one editor is open and no sidebar or panel is visible around the editor? Also make sure your zoom level is 1.

Dari-K · 2017-12-07T12:43:43Z

Ahh yeah, with the sidebar closed it has the sub-pixel antialiasing.
Edit: The sidebar being open doesn't seem to make any difference on stable though, the labels have subpixel AA either way?

bpasero · 2017-12-07T13:00:18Z

@Dari-K yeah looks like we regressed. It is very hard to find the cause for Chrome falling back to greyscale AA. I will have to do some git bisect to find the culprit.

bpasero · 2017-12-07T15:16:29Z

@Dari-K found the issue, see #24957 (comment)

bpasero · 2017-12-12T11:00:58Z

@Dari-K let me know if you are planning to look into a solution that does not require webkit-mask-image.

Dari-K · 2017-12-12T15:09:36Z

@bpasero I did have a look at using pseudo elements to overlay a fade in from transparent to the colour of the tab which gives the same effect, and doesn't cause greyscale AA, but then I have the issue of knowing what colour the tab is, as that depends on the theme, which seems to be applied using JS rather than by style rules? Not sure I can think of another CSS only method for this.

bpasero · 2017-12-13T06:21:09Z

@Dari-K yeah we know the theme colors only from the JS side, but why not apply your change also from the JS side? The tabs are being created from tabsTitleControl.ts and we have code there to react on theme changes and dynamically update the CSS too.

Dari-K · 2017-12-13T10:52:58Z

Ok thanks, I'll have a look later

…xel AA

Dari-K · 2017-12-13T23:29:18Z

So it's using the pseudo elements now, which seems to be working well with most of the built in themes, but there's some issues with the high contrast theme.

The active tab has a border all the way around it, which the label text passes through. This ends up looking rather odd with the fade on the right side of the text. Any ideas about this?

That theme also has no tab.inactiveBackground, are all of the tab colours optional? In which case is there a default colour I should use for each when they aren't present? Other thing is right now I'm only checking active/inactive background and drop background, I guess I now also need to check tab.hoverBackground, and tab.unfocusedHoverBackground.

bpasero

That is a nice solution, and I verified that subpixel AA is still being used.

Yeah, I recently added some new colors to the tabs, mainly the hover border and hover background color. All of the colors for tabs are optional for a theme but some provide a default and some do not. You can see this here.

As for HC theme: I think it is fine in that case to not show a gradient at all. Can we restore the text-overflow: ellipsis just for the HC theme? You can use the CSS selector .hc-black for that purpose.

bpasero · 2017-12-14T06:15:50Z

src/vs/base/browser/ui/iconLabel/iconlabel.css

@@ -34,6 +34,10 @@
 	text-overflow: ellipsis;
 }

+.sizing-shrink > .monaco-icon-label > .monaco-icon-label-description-container {


I suggest to move this into tabstitle.css after line 104 where we have a similar rule.

bpasero · 2017-12-14T06:16:18Z

src/vs/workbench/browser/parts/editor/media/tabstitle.css

@@ -85,6 +85,17 @@
 .monaco-workbench > .part.editor > .content > .one-editor-silo > .container > .title .tabs-container > .tab .tab-label {
 	margin-top: auto;
 	margin-bottom: auto;
+	position: relative;


Please set position: relative only when we are in sizing-shrink mode.

Dari-K · 2017-12-15T11:41:51Z

One other thing is the tab colours sometimes use alpha, which means the fade goes from transparent to a partially transparent colour, so the fade effect is much less noticeable, I guess I can check if the colour is rgba and maybe do something special for that case if you think it's needed.

bpasero · 2017-12-16T15:52:52Z

@Dari-K when I set this via settings:

"workbench.colorCustomizations": {
    "tab.hoverBackground": "#ff0000"
}

I am seeing a weird result:

Do you see this as well?

Dari-K · 2017-12-16T17:57:00Z

Hmm, will take a look soon

Dari-K · 2017-12-16T18:49:24Z

@bpasero ahh ok managed to reproduce it, it doesn't happen unless you're hovering a tab that's part of an unfocused group, I'll investigate a bit more.

bpasero · 2017-12-17T10:07:36Z

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

@@ -950,6 +950,10 @@ registerThemingParticipant((theme: ITheme, collector: ICssStyleCollector) => {
 			.monaco-workbench > .part.editor > .content > .one-editor-silo > .container > .title.active .tabs-container > .tab:hover  {
 				background: ${tabHoverBackground} !important;
 			}
+
+			.monaco-shell:not(.hc-black) .monaco-workbench > .part.editor > .content > .one-editor-silo > .container > .title.active .tabs-container >  .tab.sizing-shrink:hover > .tab-label::after {
+				background: linear-gradient(to left, ${theme.getColor(TAB_HOVER_BACKGROUND)}, transparent);


@Dari-K in many places here you seem to unnecessarily call theme.getColor(TAB_HOVER_BACKGROUND) even though the color was already resolved from the step before.

bpasero · 2017-12-17T10:08:04Z

src/vs/workbench/browser/parts/editor/media/tabstitle.css

 .monaco-workbench > .part.editor > .content > .one-editor-silo > .container > .title .tabs-container > .tab.sizing-fit .monaco-icon-label,
 .monaco-workbench > .part.editor > .content > .one-editor-silo > .container > .title .tabs-container > .tab.sizing-fit .monaco-icon-label > .monaco-icon-label-description-container {
 	overflow: visible; /* fixes https://github.com/Microsoft/vscode/issues/20182 */
 }

+.sizing-shrink > .monaco-icon-label > .monaco-icon-label-description-container {


@Dari-K please prefix more specifically by adding .monaco-workbench > .part.editor > .content > .one-editor-silo > .container > .title .tabs-container > .tab.sizing-shrink

Dari-K · 2018-01-04T17:17:27Z

@bpasero ok everything now works as I expect it to apart from editorGroup.background, which it seems gets overridden by editorGroup.dropBackground when hovering over tabs, but not the editor window.

I need to be able to tell when editorGroup.background is being overridden somehow, but the dragged-over class only seems to be applied to individual tabs, not the whole bar of tabs.

The other problem is when the tab colour is partially transparent and you drag the tab, the background is no longer fixed, so the colour of the pseudo element will be wrong. So I need to hide the pseudo element on the copy of the tab that follows the mouse. Any ideas?

bpasero · 2018-01-05T07:46:57Z

@Dari-K can you try adding classes to both the tab bar and the dragged tab when it is being dragged to remove the styles for that condition? The tab that is being dragged is the actual tab as it appears in the DOM, so any style applied on the tab will show up for the dragged tab as well.

There is already code that flips some CSS classes when dragging begins (either dragging over or the actual tab being dragged), e.g. here.

Dari-K · 2018-01-05T16:49:07Z

@bpasero seems like editorGroup.background gets overridden when you drag over it, but not when you drag out (I thought it was, but it was just being reset by the drag over the main editor body). In which case you can have editorGroup.background overridden when the tab bar isn't currently being dragged over, so adding dragged-over to it still wouldn't help. Seems like I need to add my class based on when the editor background gets set by dragging, which seems to happen in here somewhere.

For the dragged tab do you mean I should add a class specifically for this and not use dragged, which gets added and them immediately removed it seems.

bpasero · 2018-01-06T08:03:01Z

@Dari-K using the dragged class is fine to use and it will not get removed until the drag operation is done. You can give it a try and will notice the behaviour is like that.

Dari-K · 2018-01-16T17:19:55Z

@bpasero I had a look, but as far as I can tell .dragged never gets added to the tab's class, and adding a .dragged style to change background colour has no effect.
Edit: Ah, seems like tab.hoverBackground was overriding the colour I was setting.
Yep, dragged works fine

Now there's just the other problem I mentioned editorGroup.background doesn't reset on drag out, so adding .dragged-over to the tab bar may not help me know what the current colour is.

bpasero · 2018-01-17T06:22:14Z

@Dari-K so the remaining issues are that the shadow looks broken when

hovering over an inactive tab
drop feedback when dragging over tabs

Would it help if you would use the theming API to get the color values of editor.background and editorGroupHeader.tabsBackground? You can get any color from the theme service, e.g. here.

Alternatively, we could disable the shadow effect when:

hovering over an inactive tab
drag and drop is active

bpasero · 2018-01-26T06:42:51Z

@Dari-K should I test your change?

bpasero · 2018-01-26T07:01:48Z

src/vs/base/common/color.ts

@@ -399,6 +399,22 @@ export class Color {
 		return new Color(new RGBA(r, g, b, a));
 	}

+	toRGB(...backgrounds: Color[]): Color {


@Dari-K should this have a better name to reflect what it does? It seems to blend multiple colors into one?

Sure, can change the name, not entirely sure what would fit though, because blend would imply to me that it's a mix of the colours (if I did blend on red and blue I would expect to get purple), but if the foreground colour is opaque, it will be returned unchanged. It's more like a flatten operation. What do you think?

@Dari-K yeah better 👍

bpasero · 2018-01-26T07:03:22Z

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

@@ -1564,6 +1564,7 @@ export class EditorGroupsControl extends Themable implements IEditorGroupsContro
 	private updateFromDropping(element: HTMLElement, isDropping: boolean): void {
 		const groupCount = this.stacks.groups.length;
 		const background = this.getColor(isDropping ? EDITOR_DRAG_AND_DROP_BACKGROUND : groupCount > 0 ? EDITOR_GROUP_BACKGROUND : null);
+		isDropping ? DOM.addClass(element, 'dropping') : DOM.removeClass(element, 'dropping');


@Dari-K isn't this rather "dragged-over"?

Hmm, not exactly. The element that the class will be applied to is the container for all editor panes/tabs. When you drag over the tab list the class is applied (but not to the tab list, to the container mentioned earlier), when you drag over any of the editor panes the class is removed. In either case you are still dragging over the container the class is applied to. If you drag over anything else the class just retains its previous state.

bpasero · 2018-01-26T07:04:23Z

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

+	let adjustedTabBackground: Color;
+	let adjustedTabDragBackground: Color;
+
+	switch (theme.type) {


@Dari-K this seems at the wrong place, can we move it into theme.ts e.g. where the knowledge belongs to?

Dari-K · 2018-01-31T17:42:41Z

@bpasero I moved the workbench background into theme.ts, but doesn't this mean it will now be editable in your user settings? I'd need to know what colour is underneath it as well, which seems to be calculated here.

bpasero · 2018-02-01T05:55:22Z

@Dari-K no, it is not editable as far as I know, I just suggested it to have a better logical place for it. I was not suggesting to introduce areal color, just move the code.

bpasero · 2018-02-05T11:12:11Z

@Dari-K I went ahead and did some 💄 changes on top of yours to be able to merge. I hope I did not break anything. I noticed a couple of null exceptions when dealing with colors: any color can be null (except for workbench background).

Btw I notice that the same color is being used (editorBackgroundColor in belows example) multiple times in some flatten calls, e.g. like so:

adjustedTabBackground = editorGroupHeaderTabsBackground.flatten(editorBackgroundColor, editorGroupBackground, editorBackgroundColor, workbenchBackground);

Dari-K · 2018-02-16T15:32:26Z

@bpasero Hey, yeah the colour does get used multiple times, because it is used multiple times on different elements. So say we have red on blue on red on green. If all the colours are 0.8 alpha then if you don't flatten on red twice you'll get the wrong colour out.

bpasero · 2018-02-17T08:06:15Z

@Dari-K ok. any thoughts on making the shadow be larger and less subtle? E.g. 10px instead of 5px.

Dari-K · 2018-02-26T12:22:09Z

@bpasero sorry for being slow to reply, I don't seem to be getting emails from mentions for some reason. Yeah think increasing it's fine, I made it small to begin with because I wasn't sure how noticeable you wanted it, go for whatever you think looks best.

bpasero · 2018-02-26T13:49:17Z

@Dari-K I changed it to 10px, I think thats good.

Fix microsoft#39417

db2aa90

mjbvz assigned bpasero Dec 6, 2017

bpasero added this to the December 2017 milestone Dec 7, 2017

bpasero modified the milestones: December 2017, On Deck Dec 7, 2017

bpasero mentioned this pull request Dec 8, 2017

Some parts miss ClearType rendering (subpixel anti-aliasing) #24957

Closed

Dari-K and others added 2 commits December 13, 2017 22:36

Use pseudo elements instead of mask-image so that labels retain subpi…

99f0a2b

…xel AA

Merge branch 'master' into tab-fade

d7e6c12

bpasero requested changes Dec 14, 2017

View reviewed changes

bpasero modified the milestones: On Deck, December 2017/January 2018 Dec 14, 2017

Use ellipsis and disable fade for HC theme, and support hover colours

6b5b4d1

bpasero requested changes Dec 17, 2017

View reviewed changes

bpasero changed the title ~~Fix #39417~~ Fade out tabs when sizing is set to shrink (fix #39417) Dec 20, 2017

bpasero modified the milestones: December 2017/January 2018, On Deck Jan 3, 2018

Merge all colours underneath tabs to fix transparency issue

f60ab27

bpasero requested changes Jan 26, 2018

View reviewed changes

Rename color function, move workbench background into themes

ee2f65e

bpasero and others added 6 commits February 5, 2018 11:23

do not allow to change workbench background

28ae1fa

Merge branch 'master' into tab-fade

b4e23c4

💄

d0518b3

fix npes and clean up

2672e8e

fix class names

388ae3d

remove blank line

5a71032

bpasero modified the milestones: On Deck, February 2018 Feb 5, 2018

bpasero merged commit 10e8ce9 into microsoft:master Feb 5, 2018

Dari-K deleted the tab-fade branch February 12, 2018 18:05

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

Fade out tabs when sizing is set to shrink (fix #39417) #39829

Fade out tabs when sizing is set to shrink (fix #39417) #39829

Conversation

Dari-K commented Dec 6, 2017

bpasero commented Dec 7, 2017 • edited Loading

bpasero commented Dec 7, 2017 • edited Loading

Dari-K commented Dec 7, 2017

bpasero commented Dec 7, 2017 • edited Loading

Dari-K commented Dec 7, 2017 • edited Loading

bpasero commented Dec 7, 2017

bpasero commented Dec 7, 2017

bpasero commented Dec 12, 2017

Dari-K commented Dec 12, 2017

bpasero commented Dec 13, 2017 • edited Loading

Dari-K commented Dec 13, 2017

Dari-K commented Dec 13, 2017

bpasero left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dari-K commented Dec 15, 2017

bpasero commented Dec 16, 2017

Dari-K commented Dec 16, 2017

Dari-K commented Dec 16, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dari-K commented Jan 4, 2018

bpasero commented Jan 5, 2018

Dari-K commented Jan 5, 2018

bpasero commented Jan 6, 2018

Dari-K commented Jan 16, 2018 • edited Loading

bpasero commented Jan 17, 2018

bpasero commented Jan 26, 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

Dari-K commented Jan 31, 2018

bpasero commented Feb 1, 2018 • edited Loading

bpasero commented Feb 5, 2018

Dari-K commented Feb 16, 2018

bpasero commented Feb 17, 2018

Dari-K commented Feb 26, 2018

bpasero commented Feb 26, 2018

bpasero commented Dec 7, 2017 •

edited

Loading

bpasero commented Dec 7, 2017 •

edited

Loading

bpasero commented Dec 7, 2017 •

edited

Loading

Dari-K commented Dec 7, 2017 •

edited

Loading

bpasero commented Dec 13, 2017 •

edited

Loading

bpasero left a comment •

edited

Loading

Dari-K commented Jan 16, 2018 •

edited

Loading

bpasero commented Feb 1, 2018 •

edited

Loading