Added option to preserve the preview window to avoid refreshing when …

[rben01]

…switching away, and added setting to control this behavior.

(Simple fix; vscode.window.createWebviewPanel takes a parameter for this very thing.)

If accepted, it would close #603

[ggrossetie]

It seems like a simple fix but I'm concerned about unexpected side effects. This extension is largely inspired by the Markdown extension which does not use this option: https://github.com/microsoft/vscode/blob/6a4e5cc26b29359472378c2a8951c33f4ea73244/extensions/markdown-language-features/src/preview/preview.ts#L643-L646

Since the Markdown is developed by the VS Code core team, I tend to believe that there's a good reason.

Also, I don't think we should introduce an new option.

[rben01]

https://code.visualstudio.com/api/references/vscode-api#WebviewPanelOptions says

retainContextWhenHidden has a high memory overhead and should only be used if your panel's context cannot be quickly saved and restored.

I don't actually know what the overhead is — is it just that the web view is consuming its normal amount of memory even when not visible, or does enabling this option somehow use even more memory than normal? I added an option because if this is really detrimental, users may want the option of trading restoration speed for memory use.

Saving and restoring scroll position is also an option but would still require a re-render on load, which is slower than just restoring.

I've been using my fork of this extension and haven't noticed any problems — on the contrary I've really been enjoying the quick (nearly zero) loading speed when switching between tabs. Then again I am just one user.

[ggrossetie]

Saving and restoring scroll position is also an option but would still require a re-render on load, which is slower than just restoring.

That's a good point.
In this case, I think we should try to restore scroll position by expanding the setState/getState logic that we currently have in place:

asciidoctor-vscode/preview-src/index.ts

Lines 20 to 28 in 2075fab

	const originalState = vscode.getState()
	const state = {
	...(typeof originalState === 'object' ? originalState : {}),
	...getData<any>('data-state'),
	}
	// Make sure to sync VS Code state here
	vscode.setState(state)

Then, we can add an option (named "Instant Restore"?) to retain the WebView context when hidden that clearly states that this feature has a high memory overhead (quoting the documentation).

What do you think?

[rben01]

Then, we can add an option (named "Instant Restore"?) to retain the WebView context when hidden that clearly states that this feature has a high memory overhead (quoting the documentation).

I think this makes sense. So all that would need to be added is the ability to restore the scroll position when asciidoc.preview.preservePreviewWhenHidden is disabled (since when it's enabled the scroll position is automatically restored). I'll try to work on this this weekend.

[ggrossetie]

That's correct!

You will also need to add a description for this new option and update the README but it's just small details.

[rben01]

@Mogztter I pushed my changes. The only thing I couldn't do was the French and Japanese translations of the setting.

[ggrossetie]

Awesome, thanks @rben01 !
It looks good but I will take a closer look at it tomorrow.

[ggrossetie]

It does not scroll automatically when the webview is restored:

As you can see, I need to click again otherwise it won't scroll.

When asciidoc.preview.preservePreviewWhenHidden is checked, then it's working. It scrolls the preview even when the preview is hidden:

[ggrossetie]

I rebased on master and force-pushed (to fix the conflict)

[rben01]

Actually I think that this behavior was always present in the extension; if I'm not mistaken it always required a scroll to re-sync the preview and the editor. In any case I think I've fixed that issue and have now implemented 100% scroll position restoration. The logic I've implemented tries to respect the most recent user action by saving the corresponding line position and restoring it when the web view is restored. Whichever is the most recent of: 1. scrolling the editor 2. changing the selection in the editor 3. scrolling the preview — will be used to restore the preview's scroll position when it comes back from the background. This happens regardless of whether the new preservePreviewWhenHidden setting is enabled, but of course restoration is faster if it is.

[ggrossetie]

It's working nicely now, thanks!

The only case when it does not restore the scroll position is when:

1. hide the preview
2. scroll (but don't click on the editor)
3. restore the preview

Anyway, that's a small detail and it's already a huge improvement so I'm willing to merge. Let me know if you want to fix this last case or not.

[rben01]

Odd, I'm not seeing that behavior — if I try to reproduce your video I get the intended behavior.

Screen.Recording.2022-07-27.at.11.08.49.AM.mp4

All I can think is that maybe you changed a preview setting without closing and reopening the window? (I think for the the most part the settings are only loaded when the window is created, and are never refreshed.) Anyway, please do merge! I'll see if there's something else that might be causing this discrepancy.

[ggrossetie]

You need to scroll further down when the preview is hidden. In your video you scrolled back to the top.

[rben01]

Hmm, scrolling down also works for me.

[ggrossetie]

Anyway, that's not a big deal. Thanks again for your contribution 🤗