Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

"tab.activeBorderTop" color hidden under "editorGroup.border" #174134

Closed
theNestruo opened this issue Feb 11, 2023 · 10 comments
Closed

"tab.activeBorderTop" color hidden under "editorGroup.border" #174134

theNestruo opened this issue Feb 11, 2023 · 10 comments
Assignees
@bpasero @jrieken
Labels
bug Issue identified by VS Code Team member as probable bug confirmed Issue has been confirmed by VS Code Team member insiders-released Patch has been released in VS Code Insiders regression Something that used to work is now broken verified Verification succeeded
Milestone
March 2023

Comments

@theNestruo
Copy link

Type: Bug

Steps to Reproduce:

  1. Configure "tab.activeBorderTop": "#ff0000" and "editorGroup.border": "#454545" (or any other colors)
  2. Split the editor horizontally (up or below)
  3. The "tab.activeBorderTop" is not show in lower editor groups.

Additional info:
It seems that "editorGroup.border" is rendered over the "tab.activeBorderTop", as using a color with transparency lets the "tab.activeBorderTop" to be seen.
A similar bug was previously reported (and closed) as #140685

VS Code version: Code 1.75.1 (441438a, 2023-02-08T21:32:34.589Z)
OS version: Windows_NT x64 10.0.19045
Modes:
Sandboxed: No

System Info
Item Value
CPUs AMD Ryzen 3 2200U with Radeon Vega Mobile Gfx (4 x 2495)
GPU Status 2d_canvas: enabled
canvas_oop_rasterization: disabled_off
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
multiple_raster_threads: enabled_on
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
skia_renderer: enabled_on
video_decode: enabled
video_encode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled
webgpu: disabled_off
Load (avg) undefined
Memory (System) 11.55GB (6.88GB free)
Process Argv
Screen Reader no
VM 0%
@mjbvz mjbvz assigned bpasero and unassigned mjbvz Feb 27, 2023
@bpasero bpasero added bug Issue identified by VS Code Team member as probable bug confirmed Issue has been confirmed by VS Code Team member regression Something that used to work is now broken labels Feb 28, 2023
@bpasero bpasero added this to the March 2023 milestone Feb 28, 2023
@bpasero
Copy link
Member

bpasero commented Feb 28, 2023

@jrieken this is caused by 1c5ea5a. z-index is painful because it cannot be scoped to the component. There are carefully crafted z-index rules for the tabs control as you can see from the comment:

vscode/src/vs/workbench/browser/parts/editor/media/tabstitlecontrol.css

Lines 6 to 40 in e1967e9

/*
################################### z-index explainer ###################################
Tabs have various levels of z-index depending on state, typically:
- scrollbar should be above all
- sticky (compact, shrink) tabs need to be above non-sticky tabs for scroll under effect
including non-sticky tabs-top borders, otherwise these borders would not scroll under
(https://github.com/microsoft/vscode/issues/111641)
- bottom-border needs to be above tabs bottom border to win but also support sticky tabs
(https://github.com/microsoft/vscode/issues/99084) <- this currently cannot be done and
is still broken. putting sticky-tabs above tabs bottom border would not render this
border at all for sticky tabs.
On top of that there is 3 borders with a z-index for a general border below or above tabs
- tabs bottom border
- editor title bottom border (when breadcrumbs are disabled, this border will appear
same as tabs bottom border)
- editor group border
Finally, we show a drop overlay that should always be on top of everything.
The following tabls shows the current stacking order:
[z-index] [kind]
12 drag and drop overlay
11 scrollbar
10 active-tab border-bottom
9 tabs, title border bottom
8 sticky-tab
6 active/dirty-tab border top
5 editor group border / editor group header border
0 tab
##########################################################################################
*/

Can sticky scroll have a different solution that does not require the entire split view to be lifted to 20?

//cc @joaomoreno for split views, not sure this change might have an impact elsewhere

@bpasero
Copy link
Member

bpasero commented Feb 28, 2023

Oh, this is not lifting the entire split view up, just the border. So the impact is probably less visible, but here in this case it causes the tab border to fall below.

@jrieken
Copy link
Member

jrieken commented Feb 28, 2023

Can sticky scroll have a different solution that does not require the entire split view to be lifted to 20?

I am open for suggestions but I didn't find a better way to tackle this

@bpasero
Copy link
Member

bpasero commented Mar 1, 2023

@jrieken @aiday-mar is there a reason the sticky-widget needs a z-index of 11? If we would make that 4, the issue would be solved and I still see the sticky widget floating above.

vscode/src/vs/editor/contrib/stickyScroll/browser/stickyScroll.css

Lines 35 to 40 in 14f639f

.monaco-editor .sticky-widget {
width: 100%;
box-shadow: var(--vscode-scrollbar-shadow) 0 3px 2px -2px;
z-index: 11;
background-color: var(--vscode-editorStickyScroll-background);
}

@aiday-mar
Copy link
Contributor

Hey I dug in the code, and it comes from this: #158730, placing the sticky scroll on top of the zone widget. However this is perhaps not that important, and the z-index can be reduced.

@bpasero
Copy link
Member

bpasero commented Mar 2, 2023

Yes, I would prefer if sticky widget could go below 5, otherwise a lot of other things would have to lift above the new 20 from split view.

@aiday-mar
Copy link
Contributor

Sounds good, there is a PR which should soon be merged

@bpasero
Copy link
Member

bpasero commented Mar 2, 2023

I assume this is the issue:

image

If so I still think we need to fix this somehow. At least so that either the zone is fully above the sticky scroll or fully below, but not intermingled.

@aiday-mar aiday-mar mentioned this issue Mar 2, 2023
Merged
bpasero added a commit that referenced this issue Mar 2, 2023
@bpasero
"tab.activeBorderTop" color hidden under "editorGroup.border" (fix #1…
22a1ef4 
…74134)
@bpasero bpasero closed this as completed in 77c8e2e Mar 2, 2023
@VSCodeTriageBot VSCodeTriageBot added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Mar 2, 2023
@bpasero bpasero added the author-verification-requested Issues potentially verifiable by issue author label Mar 7, 2023
@VSCodeTriageBot
Copy link
Collaborator

This bug has been fixed in the latest release of VS Code Insiders!

@theNestruo, you can help us out by commenting /verified if things are now working as expected.

If things still don't seem right, please ensure you're on version bbc87d8 of Insiders (today's or later - you can use Help: About in the command palette to check), and leave a comment letting us know what isn't working as expected.

Happy Coding!

@theNestruo
Copy link
Author

/verified

Thank you!

@VSCodeTriageBot VSCodeTriageBot removed the author-verification-requested Issues potentially verifiable by issue author label Mar 8, 2023
@VSCodeTriageBot VSCodeTriageBot added the verified Verification succeeded label Mar 8, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Apr 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@bpasero bpasero

@jrieken jrieken

Labels
bug Issue identified by VS Code Team member as probable bug confirmed Issue has been confirmed by VS Code Team member insiders-released Patch has been released in VS Code Insiders regression Something that used to work is now broken verified Verification succeeded
Projects
None yet
Milestone
March 2023
Development

No branches or pull requests
6 participants
@bpasero @jrieken @theNestruo @mjbvz @aiday-mar @VSCodeTriageBot