Allow to configure a top tab border color #49313
Conversation
|
This issue is about highlighting top border of active tab, not changing the default color of bottom one. You need to register a new color, something like |
|
@usernamehw yes but in the comments I am told not to change position: "I would accept a (minimal) PR to get this in. I do not think we can introduce positions but rather we can introduce a new color just for this purpose." Not sure if this means change the default but I assume that is what was meant. nvm I get it now. Will update my PR soon. |
|
|
|
@usernamehw Fixed. There is now an option for either a top border or a bottom border. |
| if (activeTabBorderColor) { | ||
| tabContainer.style.boxShadow = `${activeTabBorderColor} 0 -1px inset`; | ||
| } else if (activeTabBorderColorTop) { | ||
| tabContainer.style.boxShadow = `${activeTabBorderColorTop} 0 2px inset`; |
There was a problem hiding this comment.
I would suggest to use 1px here to keep a similar look as the bottom border.
There was a problem hiding this comment.
Done. Thanks for the feedback.
| @@ -333,8 +333,11 @@ export class TabsTitleControl extends TitleControl { | |||
| // color, the two colors would collide and the tab border never shows up. | |||
| // see https://github.com/Microsoft/vscode/issues/33111 | |||
| const activeTabBorderColor = this.getColor(isGroupActive ? TAB_ACTIVE_BORDER : TAB_UNFOCUSED_ACTIVE_BORDER); | |||
| const activeTabBorderColorTop = this.getColor(isGroupActive ? TAB_ACTIVE_BORDER_TOP : TAB_UNFOCUSED_ACTIVE_BORDER); | |||
There was a problem hiding this comment.
The logic here seems weird. I think you should not fallback to TAB_UNFOCUSED_ACTIVE_BORDER because if that color was set, in the bottom if-else case the color is still not applied. I would suggest to introduce a new color TAB_UNFOCUSED_ACTIVE_BORDER_TOP to do this properly.
There was a problem hiding this comment.
Done. Thanks again for the feedback.
bpasero
changed the title
|
Thanks 👍 |
Changed the color of the TAB_ACTIVE_BORDER as requested in issue #47867