Fix webview views not updating correctly on scroll #122887
Conversation
|
@sbatten and @joaomoreno Please look at the changes to the split view and view panes to let me know if this approach is ok or if there's a better way to accomplish this. If things look good, I'll go ahead and update the other files in the codebase to pass through the new parameters |
| @@ -237,6 +237,9 @@ export class SplitView<TLayoutContext = undefined> extends Disposable { | |||
| private _onDidSashReset = this._register(new Emitter<number>()); | |||
| readonly onDidSashReset = this._onDidSashReset.event; | |||
|
|
|||
| private _onScroll = this._register(new Emitter<ScrollEvent>()); | |||
There was a problem hiding this comment.
This emitter doesn't need to exist. We can just this.onScroll = this.scrollableElement.onScroll, right?
| @@ -286,7 +287,7 @@ export abstract class Pane extends Disposable implements IView { | |||
| } | |||
|
|
|||
| protected abstract renderHeader(container: HTMLElement): void; | |||
| protected abstract renderBody(container: HTMLElement): void; | |||
| protected abstract renderBody(container: HTMLElement, rootContainer: HTMLElement): void; | |||
There was a problem hiding this comment.
I think this is a hack, but I also can't propose a good solution. Nevertheless, my rule for hacks is: keep them as contained as possible. In that mindset, couldn't we simply navigate up the parent DOM chain in layoutWebview(), find the element we assume to be the root element and use that? The hack would not spread to other files and would also not change component API.
Precedence:
vscode/src/vs/workbench/contrib/scm/browser/scmViewPane.ts
Lines 118 to 120 in 731402d
There was a problem hiding this comment.
Ok, I think that should be possible. As you note, it will require touching less code but could break if the parent dom structure changes (such as if an element's class is renamed)
I originally tried using intersection observers for this but couldn't get them working as I needed
mjbvz
force-pushed
the
webview-relayout-on-scroll
branch
from
097b8e0
to
900ddc2
Compare
Fixes microsoft#122887 This fixes two issues with webview views that are inside view pane containers that are overflowing: When a container with webview views is scrolled, we need to update the positon of the webview If the webview is then scrolled outside of the container, we need to make sure it is clipped so it does not draw on top of other interface elements Both of these are required since the webview is not part of the normal editor dom. Instead it is a top level element that is absolutely positioned in the slot it needs to me
Fixes microsoft#122887 This fixes two issues with webview views that are inside view pane containers that are overflowing: When a container with webview views is scrolled, we need to update the positon of the webview If the webview is then scrolled outside of the container, we need to make sure it is clipped so it does not draw on top of other interface elements Both of these are required since the webview is not part of the normal editor dom. Instead it is a top level element that is absolutely positioned in the slot it needs to me
mjbvz
force-pushed
the
webview-relayout-on-scroll
branch
from
900ddc2
to
c36655a
Compare
|
Merging this so we can get some testing. Let me know if you have any concerns about the implementation |
Fixes #110450
This fixes two issues with webview views that are inside view pane containers that are overflowing:
When a container with webview views is scrolled, we need to update the positon of the webview
If the webview is then scrolled outside of the container, we need to make sure it is clipped so it does not draw on top of other interface elements
Both of these are required since the webview is not part of the normal editor dom. Instead it is a top level element that is absolutely positioned in the slot it needs to me