Hi, @mjbvz - just a quick bump for this PR - was wondering if you might have some time to look into this?

Sorry for the delay

This looks like a pretty hacky/brittle fix. I'm not sure what the proper fix is here but I don't think we can check this in as is

Hi,

Yeah, it's definitely not clean, but we have been running it for a few months without any issues.

There is a slightly less brittle version which moves it once after the animation is complete, but this version "appears" smoother to the user. I would suggest that whilst it isn't pretty, it is better than leaving the code base as-is since if a user ends up in a situation where this bug occurs there is no workaround and no way out of it, other than reloading vscode each time you want to expand or collapse the view.

In terms of the best solution, the cleanest option may be to remove the floating iframes used by webviews and instead position them as child elements of the view. However, I am not sure if there is historically a reason why they are floating instead of being children.

The other option could be to somehow share the animation with the webview before hand, rather than reactively moving it as we do currently. This is something I am happy to look into further, but I would rather know what kind of solutions you'd be interested before investing time in it.

Yes the floating frames are required. They let us avoid having the iframes be re-parented, which destroys the frame contents

@a-stewart @mjbvz Since a simple resize / refresh fixes the position, can it not be triggered automatically once after an expansion/contraction is finished? (200 ms after an expand/contract)

This would not animate smoothly, but at least when finished animating, the position would be corrected abruptly and somehow at least it would be somewhat fixed!

Just wondering... Thanks!

(Sorry not familiar enough yet with vscode's codebase to provide a pull request)

Since a simple resize / refresh fixes the position, can it not be triggered automatically once after an expansion/contraction is finished? (200 ms after an expand/contract)

Yes, that is what this PR did (I then updated it to do the same refresh a few times so it would "appear" to animate).

Rather than doing this (as it currently is):

			for (let i = 40; i <= 240; i += 20) {
				setTimeout(() => {
					this.layoutWebviewOverElement(element, dimension, /*animated*/ false);
				}, i);
			}

We can simply do:

			setTimeout(() => {
				this.layoutWebviewOverElement(element, dimension, /*animated*/ false);
			}, 200);

This will cause a reposition to be triggered 200ms later (after the animation is complete).

I think the cleanest option, may be to see if we can some how pass in the animation information to this message. As at the moment it has to act reactively and try and "catch up" with the underlying div. But if we knew where the div below as aiming at, we could animate it there properly.

Yes I like that second approach better, even if it's still a hack. Can you update the PR to do that instead (but also make to cancel the timeout if the webview view is closed)

@mjbvz Thanks for your consideration and approval of this suggestion! I'll let @a-stewart do the correction as I'm not knowledgable enough in vscode's code base to do so myself!

Thanks again! :)

Updated as suggested.

This will result in there being a single "correction" made after the animation. So the underlying view and headers will move smoothly to the new position with the webview remaining where it is, and then once the animation is complete, the webview will jump in one step to the new location of the view.

Is there something I need to do for Hygiene and Layering?

Sorry for the delay - I have pulled in main to my local branch and run format document to hopefully fix the hygene issue.