Fix widget in cell editor cutoff by the titlebar.

[rebornix]

This PR attempts fix #93930. Notebook has Monaco editors inside list view which is a scrollable element, thus there are nested position: relative; overflow: hidden; DOM nodes, which in the end leads to the cut off of Hovers, Parameter Hints, etc to be cut off at the boundary of the Notebook Editor.

@sbatten helped me figure out a workaround to fix this issue but it requires setting z-index on title bar. We only want the z-index to be set only when a notebook editor is visible and focused.

TODO:

• Set z-index on title element when notebook is focused
• Change position: fixed for Hover, Parameter Hint, etc.

[rebornix]

@bpasero here I tried to get access to the title bar DOM node in order to change its z-index later on, is there any safer way to do this? Should we expose the DOM node through IEditorGroupView.titlebarDomNode etc?

[bpasero]

I am not a big fan of changing z-index to begin with. I am not a fan of doing this selectively based on the editor showing. Overall, -1. Please think about an alternative if possible.

[rebornix]

The problem we are trying to fix is changing z orders of DOM nodes:

- title
- list
  - editor
     - hover

We want to make sure list and editor are below list but hover is above all of them. Current PR uses z-index to change the z order. It's the right CSS primitive for archiving that but the catch is it requires us to change the z index of title.

The other potential solution is making hover detached in the same way as Context Menu where in Desktop, context menu container is a sibling of grid view so it's always above editors, etc. We can extend the IContextViewService to create a container out of the grid view, and put hover, parameter hint, etc in it.

@bpasero the z-index solution is not great but it keeps this problem inside Notebook wold. The second proposal requires changes to the content widgets inside Monaco so cc @alexdima for his opinions.

[sbatten]

The context view proposal could lend itself to improving some of the issues around context views like centralizing focus handling when the menu steals focus from the editor but we don't want it to. Peng and I have discussed this alternative and thing it can be very nice though it carries some risk in that it touches more area.

[alexdima]

Today, content widgets are rooted in the editor's view dom node. The editor has mouse hit testing code to figure out what is logically hit when moving the mouse pointer in the editor. This is done with a mouse move/down/up listener on the editor view dom node and it can discern, using custom attributes and event.target, where the mouse is, so for example the mouse target can be a content widget an overlay widget, a view zone, etc (see for example references of MouseTargetType.CONTENT_WIDGET). If content widgets are pulled outside of the editor view dom node, then we need to make sure that the mouse target logic of the editor still works.

[rebornix]

@alexdima thanks for the insights ❤️ . I'll try to summarize the challenges we need to tackle if we are going this route:

• Context View service provides a container for detached views for Monaco Editor.
  • 💪 we need to figure out who is going to handle the position of the container or detached view. For example, the position of hover will be relative to a view position in the editor, if the editor itself scrolls, the hover position needs to be updated (let's say we are not dismissing the hover); if the container of the editor scrolls (notebook scenario), the hover should move accordingly as well even though from Monaco Editor's perspective, the relative position of the view position doesn't change at all.
• Hit test in Monaco
• Focus state management in Monaco (as the focus goes out of the editor container)

[rebornix]

@sbatten and me spent another one hour on fixing this issue again as it's somehow broken, we know what we are doing now with the PR and confident with the fix. Firstly we disable transform: translate3d for the list view, thus it won't �trigger the browser to create another layer. Secondly we set fixedOverflowWidgets editor options on every Monaco Editor in the Notebook, which will set overflowing widgets to be positioned fixed.

Now for an editor group, title bar, nested editors, overflowing widgets inside the editors, they are all on the same plane and z-index controls the z ordering. With this in mind, the next time if the hover is cut off again, the way to troubleshoot is

• Locate the hover element, traverse up in the DOM tree, stop when its ancestor
  • has transform, opacity, etc which can create a new layer. The hover element then won't escape this layer. Compare this one with the element which covers the hover.
  • has z-index, then the hover element's z index will be the same as this one. Go back to the first step.

Considering the fix is intuitive and straight forward to troubleshoot when it breaks, I'm going to merge this change to master. We can continue discussion around better solution proposed above later on.

[rebornix]

@bpasero FYI, with #97543 , the notebook editor is rendered directly under workbench DOM node (same as Custom Editor / Webview) and the workaround in this PR is no longer needed. We don't have the z-index manipulation any more.