From 61bd0a8ea10b319c30215ae44b5ca85826565634 Mon Sep 17 00:00:00 2001 From: gjsjohnmurray Date: Mon, 2 Dec 2019 22:48:56 +0000 Subject: [PATCH 1/6] #27498 restore extension editor webview scroll positions --- .../extensions/browser/extensionEditor.ts | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/contrib/extensions/browser/extensionEditor.ts b/src/vs/workbench/contrib/extensions/browser/extensionEditor.ts index f302a97cfc70a..661135a2e325f 100644 --- a/src/vs/workbench/contrib/extensions/browser/extensionEditor.ts +++ b/src/vs/workbench/contrib/extensions/browser/extensionEditor.ts @@ -172,6 +172,12 @@ export class ExtensionEditor extends BaseEditor { private extensionChangelog: Cache | null; private extensionManifest: Cache | null; + // Some action bar items use a webview whose vertical scroll position we track in this array (Readme=0, Changelog=1) + private initialScrollProgress: number[] = []; + + // Spot when an ExtensionEditor instance gets reused for a different extension, in which case the vertical scroll positions must be zeroed + private currentIdentifier: string = ''; + private layoutParticipants: ILayoutParticipant[] = []; private readonly contentDisposables = this._register(new DisposableStore()); private readonly transientDisposables = this._register(new DisposableStore()); @@ -322,6 +328,11 @@ export class ExtensionEditor extends BaseEditor { this.editorLoadComplete = false; const extension = input.extension; + if (this.currentIdentifier !== extension.identifier.id) { + this.initialScrollProgress = [0, 0]; + this.currentIdentifier = extension.identifier.id; + } + this.transientDisposables.clear(); this.extensionReadme = new Cache(() => createCancelablePromise(token => extension.getReadme(token))); @@ -573,19 +584,25 @@ export class ExtensionEditor extends BaseEditor { return Promise.resolve(null); } - private async openMarkdown(cacheResult: CacheResult, noContentCopy: string, template: IExtensionEditorTemplate): Promise { + private async openMarkdown(cacheResult: CacheResult, noContentCopy: string, template: IExtensionEditorTemplate, webviewIndex: number): Promise { try { const body = await this.renderMarkdown(cacheResult, template); const webviewElement = this.contentDisposables.add(this.webviewService.createWebviewEditorOverlay('extensionEditor', { enableFindWidget: true, + tryRestoreScrollPosition: true, }, {})); - webviewElement.claim(this); + webviewElement.initialScrollProgress = this.initialScrollProgress[webviewIndex]; + webviewElement.layoutWebviewOverElement(template.content); webviewElement.html = body; + webviewElement.claim(this); this.contentDisposables.add(webviewElement.onDidFocus(() => this.fireOnDidFocus())); + + this.contentDisposables.add(webviewElement.onDidScroll(() => this.initialScrollProgress[webviewIndex] = webviewElement.initialScrollProgress)); + const removeLayoutParticipant = arrays.insert(this.layoutParticipants, { layout: () => { webviewElement.layoutWebviewOverElement(template.content); @@ -824,11 +841,11 @@ export class ExtensionEditor extends BaseEditor { } private openReadme(template: IExtensionEditorTemplate): Promise { - return this.openMarkdown(this.extensionReadme!.get(), localize('noReadme', "No README available."), template); + return this.openMarkdown(this.extensionReadme!.get(), localize('noReadme', "No README available."), template, 0); } private openChangelog(template: IExtensionEditorTemplate): Promise { - return this.openMarkdown(this.extensionChangelog!.get(), localize('noChangelog', "No Changelog available."), template); + return this.openMarkdown(this.extensionChangelog!.get(), localize('noChangelog', "No Changelog available."), template, 1); } private openContributions(template: IExtensionEditorTemplate): Promise { From 98b02966f4b37843132c58c9895c9e2d35c65613 Mon Sep 17 00:00:00 2001 From: gjsjohnmurray Date: Sun, 8 Dec 2019 19:41:35 +0000 Subject: [PATCH 2/6] Delay setInitialScrollPosition so iframe contents have time to load fully (we hope) --- src/vs/workbench/contrib/webview/browser/pre/main.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/webview/browser/pre/main.js b/src/vs/workbench/contrib/webview/browser/pre/main.js index 138707c9a9baa..291405138acfb 100644 --- a/src/vs/workbench/contrib/webview/browser/pre/main.js +++ b/src/vs/workbench/contrib/webview/browser/pre/main.js @@ -453,7 +453,12 @@ if (contentDocument && contentDocument.body) { // Workaround for https://github.com/Microsoft/vscode/issues/12865 // check new scrollY and reset if neccessary - setInitialScrollPosition(contentDocument.body, contentWindow); + + // Delay this in the hope that all of the iframe content will have loaded by the time we set + // the desired initial scroll position. But how can we be sure how long is long enough? + setTimeout(() => { + setInitialScrollPosition(contentDocument.body, contentWindow); + }, 1000); } const newFrame = getPendingFrame(); From 18266848ee8586f50ac64e5061b9d4f56b250ccf Mon Sep 17 00:00:00 2001 From: gjsjohnmurray Date: Fri, 22 May 2020 16:05:54 +0100 Subject: [PATCH 3/6] Some changes requested by @mjbvz --- .../extensions/browser/extensionEditor.ts | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/vs/workbench/contrib/extensions/browser/extensionEditor.ts b/src/vs/workbench/contrib/extensions/browser/extensionEditor.ts index 586d4a0289326..2c8e9fc6fe1be 100644 --- a/src/vs/workbench/contrib/extensions/browser/extensionEditor.ts +++ b/src/vs/workbench/contrib/extensions/browser/extensionEditor.ts @@ -164,6 +164,11 @@ interface IExtensionEditorTemplate { header: HTMLElement; } +const enum WebviewIndex { + Readme, + Changelog +} + export class ExtensionEditor extends BaseEditor { static readonly ID: string = 'workbench.editor.extension'; @@ -174,8 +179,8 @@ export class ExtensionEditor extends BaseEditor { private extensionChangelog: Cache | null; private extensionManifest: Cache | null; - // Some action bar items use a webview whose vertical scroll position we track in this array (Readme=0, Changelog=1) - private initialScrollProgress: number[] = []; + // Some action bar items use a webview whose vertical scroll position we track in this map + private initialScrollProgress: Map = new Map(); // Spot when an ExtensionEditor instance gets reused for a different extension, in which case the vertical scroll positions must be zeroed private currentIdentifier: string = ''; @@ -334,7 +339,7 @@ export class ExtensionEditor extends BaseEditor { const extension = input.extension; if (this.currentIdentifier !== extension.identifier.id) { - this.initialScrollProgress = [0, 0]; + this.initialScrollProgress.clear(); this.currentIdentifier = extension.identifier.id; } @@ -593,7 +598,7 @@ export class ExtensionEditor extends BaseEditor { return Promise.resolve(null); } - private async openMarkdown(cacheResult: CacheResult, noContentCopy: string, template: IExtensionEditorTemplate, webviewIndex: number): Promise { + private async openMarkdown(cacheResult: CacheResult, noContentCopy: string, template: IExtensionEditorTemplate, webviewIndex: WebviewIndex): Promise { try { const body = await this.renderMarkdown(cacheResult, template); @@ -602,7 +607,7 @@ export class ExtensionEditor extends BaseEditor { tryRestoreScrollPosition: true, }, {}, undefined)); - webview.initialScrollProgress = this.initialScrollProgress[webviewIndex]; + webview.initialScrollProgress = this.initialScrollProgress.get(webviewIndex) || 0; webview.layoutWebviewOverElement(template.content); webview.html = body; @@ -610,7 +615,7 @@ export class ExtensionEditor extends BaseEditor { this.contentDisposables.add(webview.onDidFocus(() => this.fireOnDidFocus())); - this.contentDisposables.add(webview.onDidScroll(() => this.initialScrollProgress[webviewIndex] = webview.initialScrollProgress)); + this.contentDisposables.add(webview.onDidScroll(() => this.initialScrollProgress.set(webviewIndex, webview.initialScrollProgress))); const removeLayoutParticipant = arrays.insert(this.layoutParticipants, { layout: () => { @@ -854,7 +859,7 @@ export class ExtensionEditor extends BaseEditor { if (manifest && manifest.extensionPack && manifest.extensionPack.length) { return this.openExtensionPackReadme(manifest, template); } - return this.openMarkdown(this.extensionReadme!.get(), localize('noReadme', "No README available."), template, 0); + return this.openMarkdown(this.extensionReadme!.get(), localize('noReadme', "No README available."), template, WebviewIndex.Readme); } private async openExtensionPackReadme(manifest: IExtensionManifest, template: IExtensionEditorTemplate): Promise { @@ -882,14 +887,14 @@ export class ExtensionEditor extends BaseEditor { await Promise.all([ this.renderExtensionPack(manifest, extensionPackContent), - this.openMarkdown(this.extensionReadme!.get(), localize('noReadme', "No README available."), { ...template, ...{ content: readmeContent } }, 0), + this.openMarkdown(this.extensionReadme!.get(), localize('noReadme', "No README available."), { ...template, ...{ content: readmeContent } }, WebviewIndex.Readme), ]); return { focus: () => extensionPackContent.focus() }; } private openChangelog(template: IExtensionEditorTemplate): Promise { - return this.openMarkdown(this.extensionChangelog!.get(), localize('noChangelog', "No Changelog available."), template, 1); + return this.openMarkdown(this.extensionChangelog!.get(), localize('noChangelog', "No Changelog available."), template, WebviewIndex.Changelog); } private openContributions(template: IExtensionEditorTemplate): Promise { From 9ef31f7f5e3de7ed16ed902c44a2553f0e4853b0 Mon Sep 17 00:00:00 2001 From: gjsjohnmurray Date: Mon, 5 Oct 2020 16:45:08 +0100 Subject: [PATCH 4/6] revert setTimeout hack --- src/vs/workbench/contrib/webview/browser/pre/main.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/vs/workbench/contrib/webview/browser/pre/main.js b/src/vs/workbench/contrib/webview/browser/pre/main.js index 0776b3bf612d9..25915ea8fa27d 100644 --- a/src/vs/workbench/contrib/webview/browser/pre/main.js +++ b/src/vs/workbench/contrib/webview/browser/pre/main.js @@ -495,11 +495,7 @@ // Workaround for https://github.com/Microsoft/vscode/issues/12865 // check new scrollY and reset if neccessary - // Delay this in the hope that all of the iframe content will have loaded by the time we set - // the desired initial scroll position. But how can we be sure how long is long enough? - setTimeout(() => { - setInitialScrollPosition(contentDocument.body, contentWindow); - }, 1000); + setInitialScrollPosition(contentDocument.body, contentWindow); } const newFrame = getPendingFrame(); From 07385056b36c7a78154224f3ab716b8276e017ea Mon Sep 17 00:00:00 2001 From: gjsjohnmurray Date: Mon, 5 Oct 2020 16:51:30 +0100 Subject: [PATCH 5/6] fix my spelling error in comment --- src/vs/workbench/contrib/webview/browser/pre/main.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/webview/browser/pre/main.js b/src/vs/workbench/contrib/webview/browser/pre/main.js index 25915ea8fa27d..1e74c6128d78d 100644 --- a/src/vs/workbench/contrib/webview/browser/pre/main.js +++ b/src/vs/workbench/contrib/webview/browser/pre/main.js @@ -493,7 +493,7 @@ const onLoad = (contentDocument, contentWindow) => { if (contentDocument && contentDocument.body) { // Workaround for https://github.com/Microsoft/vscode/issues/12865 - // check new scrollY and reset if neccessary + // check new scrollY and reset if necessary setInitialScrollPosition(contentDocument.body, contentWindow); } From ff81a4581621354c19022dae8ae63085b7a2c9f8 Mon Sep 17 00:00:00 2001 From: gjsjohnmurray Date: Mon, 5 Oct 2020 16:53:48 +0100 Subject: [PATCH 6/6] remove blank line to perfect the undo --- src/vs/workbench/contrib/webview/browser/pre/main.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/vs/workbench/contrib/webview/browser/pre/main.js b/src/vs/workbench/contrib/webview/browser/pre/main.js index 1e74c6128d78d..759d9eff733d8 100644 --- a/src/vs/workbench/contrib/webview/browser/pre/main.js +++ b/src/vs/workbench/contrib/webview/browser/pre/main.js @@ -494,7 +494,6 @@ if (contentDocument && contentDocument.body) { // Workaround for https://github.com/Microsoft/vscode/issues/12865 // check new scrollY and reset if necessary - setInitialScrollPosition(contentDocument.body, contentWindow); }