fix: fix editor progress position when enabled pinnedTabsOnSeparateRow

[harbin1053020115]

To #195313

[gjsjohnmurray]

/assign @benibenj

[benibenj]

Thanks for submitting the bug and creating a PR! I left some comments to your changes

[benibenj]

This probably won't work. Even though the pinnedTabsOnSeparateRow setting might be activated, this doesn't mean that two tab rows are actually visible. If all tabs are pinned or if there is no pinned tab then there will only be one tab row and so the height of the progress bar has to be change in these cases.

We handle the number of tab rows here.

[harbin1053020115]

@benibenj You are right. The first commit won't work. Even pinnedTabsonSeparateRow setting is activated, there will be just one tab row if where is no pinned tabs opened.
And, I found it's not a good way to update tab height dynamic using css variable, we need to conside many scenarios:

1. When open/close/pinned editors, we need to update height according to opened tabs height
2. When moving opened editors in editor title, we need to update height according to opened tabs height
3. When pinnedTabsonSeparateRow/editorTabHeight setting is changed, we need to update height
   So, I have changed the way to realize, I moved the editor progress bar into titleContainer, and make the progress bar at the bottom of titleContainer

[harbin1053020115]

Below is the result of progress bar:

1. normal:
   
2. window.density.editorTabHeight setting changed to compact:
   
3. breadcrumbs.enabled enabled:
   
4. pinnedTabsonSeparateRow enabled:
   

[benibenj]

I think we should still set the progress bar from the top as we did before. Your approach causes it to be placed underneath the breadcrumbs which did not used to be the case, and so we don't want to change that.

This is my recommendation:

We already know the height of the editor title control. It can be accessed in editorTitleControl.ts. Like in your first approach, you can introduce a CSS variable, and overwrite the absolute top position of the progress bar in editorgroupview.css (This is where the progress bar lives). The variable could be called --editor-group-tabs-height as we already have --editor-group-tab-height. I wouldn't call it title height as we do not want to include the breadcrumbs height.

The last thing to do now is update the value when there are changes to the layout, these could be:

• changing window.density.editorTabHeight setting
• changing workbench.editor.pinnedTabsOnSeparateRow setting
• changing workbench.editor.wrapTabs setting
• On layout() (in case of resizing the editor for example) when using wrapped tabs
• When opening, closing and moving an editor
• When sticking and unsticking (known as pinning and unpinning) an editor

The last two are specific to having some settings turned on, however, we can start off by updating the height every time as it is not an expensive operation and future proof. But we might change this in the future.

[harbin1053020115]

I think we should still set the progress bar from the top as we did before. Your approach causes it to be placed underneath the breadcrumbs which did not used to be the case, and so we don't want to change that.

This is my recommendation:

We already know the height of the editor title control. It can be accessed in editorTitleControl.ts. Like in your first approach, you can introduce a CSS variable, and overwrite the absolute top position of the progress bar in editorgroupview.css (This is where the progress bar lives). The variable could be called --editor-group-tabs-height as we already have --editor-group-tab-height. I wouldn't call it title height as we do not want to include the breadcrumbs height.

The last thing to do now is update the value when there are changes to the layout, these could be:

• changing window.density.editorTabHeight setting
• changing workbench.editor.pinnedTabsOnSeparateRow setting
• changing workbench.editor.wrapTabs setting
• On layout() (in case of resizing the editor for example) when using wrapped tabs
• When opening, closing and moving an editor
• When sticking and unsticking (known as pinning and unpinning) an editor

The last two are specific to having some settings turned on, however, we can start off by updating the height every time as it is not an expensive operation and future proof. But we might change this in the future.

Got it. Let me submit a change.

[benibenj]

@harbin1053020115 The change looks great! Thanks very much.
I removed the method you added as we can update the variable in the editorGroupView when we call the layout method.

[harbin1053020115]

@harbin1053020115 The change looks great! Thanks very much. I removed the method you added as we can update the variable in the editorGroupView when we call the layout method.

Great~Moving to layout to reset css variable is a better way.