From 5da30384d55260143bf2c48310473c6a9f7fb96f Mon Sep 17 00:00:00 2001 From: Colin Grant Date: Mon, 1 Jun 2020 21:53:52 -0500 Subject: [PATCH] scroll protection Signed-off-by: Colin Grant --- .../src/browser/preference-tree-provider.ts | 20 +++++++++++--- .../browser/util/preference-event-service.ts | 2 +- .../views/preference-editor-widget.tsx | 16 +++++++----- .../browser/views/preference-tree-widget.tsx | 26 ++++++++++++++----- 4 files changed, 46 insertions(+), 18 deletions(-) diff --git a/packages/preferences/src/browser/preference-tree-provider.ts b/packages/preferences/src/browser/preference-tree-provider.ts index acf41d99b5341..e373e30c0cb0a 100644 --- a/packages/preferences/src/browser/preference-tree-provider.ts +++ b/packages/preferences/src/browser/preference-tree-provider.ts @@ -26,11 +26,13 @@ import { Preference } from './util/preference-types'; interface PreferenceFilterOptions { minLength?: number; baseSchemaAltered?: boolean; + requiresFilter?: boolean; }; const filterDefaults: Required = { minLength: 1, baseSchemaAltered: false, + requiresFilter: true, }; @injectable() @@ -58,7 +60,15 @@ export class PreferencesTreeProvider { this.updateUnderlyingData({ baseSchemaAltered: true }); this.schemaProvider.onDidPreferenceSchemaChanged(() => this.handleUnderlyingDataChange({ baseSchemaAltered: true })); this.preferencesEventService.onSearch.event(searchEvent => this.updateDisplay(searchEvent.query)); - this.preferencesEventService.onTabScopeSelected.event(scopeEvent => this.handleUnderlyingDataChange({}, scopeEvent)); + this.preferencesEventService.onTabScopeSelected.event(scopeEvent => { + const newScope = Number(scopeEvent.scope); + const currentScope = Number(this.currentScope.scope); + const scopeChangesPreferenceVisibility = + ((newScope === PreferenceScope.User || newScope === PreferenceScope.Workspace) && currentScope === PreferenceScope.Folder) + || (newScope === PreferenceScope.Folder && (currentScope === PreferenceScope.User || currentScope === PreferenceScope.Workspace)); + + this.handleUnderlyingDataChange({ requiresFilter: scopeChangesPreferenceVisibility }, scopeEvent); + }); } protected updateUnderlyingData(options: PreferenceFilterOptions, newScope?: Preference.SelectedScopeDetails): void { @@ -75,10 +85,12 @@ export class PreferencesTreeProvider { if (options.baseSchemaAltered) { this.baseTree = this.preferencesTreeGenerator.generateTree(); } + const shouldBuildNewTree = options.requiresFilter !== false; + if (shouldBuildNewTree) { + this._currentTree = this.filter(term, Number(this.currentScope.scope), this.baseTree, options); + } - this._currentTree = this.filter(term, Number(this.currentScope.scope), this.baseTree, options); - - this.preferencesEventService.onDisplayChanged.fire(); + this.preferencesEventService.onDisplayChanged.fire(shouldBuildNewTree || !!options.baseSchemaAltered); } protected filter( diff --git a/packages/preferences/src/browser/util/preference-event-service.ts b/packages/preferences/src/browser/util/preference-event-service.ts index 24542fabaa17b..b48dbc35910ff 100644 --- a/packages/preferences/src/browser/util/preference-event-service.ts +++ b/packages/preferences/src/browser/util/preference-event-service.ts @@ -23,5 +23,5 @@ export class PreferencesEventService { onSearch = new Emitter(); onEditorScroll = new Emitter(); onNavTreeSelection = new Emitter(); - onDisplayChanged = new Emitter(); + onDisplayChanged = new Emitter(); } diff --git a/packages/preferences/src/browser/views/preference-editor-widget.tsx b/packages/preferences/src/browser/views/preference-editor-widget.tsx index 0c400c8588b1d..f9a79b06c0b2f 100644 --- a/packages/preferences/src/browser/views/preference-editor-widget.tsx +++ b/packages/preferences/src/browser/views/preference-editor-widget.tsx @@ -60,7 +60,7 @@ export class PreferencesEditorWidget extends ReactWidget { this.preferenceValueRetrievalService.onPreferenceChanged((preferenceChange): void => { this.update(); }); - this.preferencesEventService.onDisplayChanged.event(() => this.handleChangeDisplay()); + this.preferencesEventService.onDisplayChanged.event(didChangeTree => this.handleChangeDisplay(didChangeTree)); this.preferencesEventService.onNavTreeSelection.event(e => this.scrollToEditorElement(e.nodeID)); this.currentDisplay = this.preferenceTreeProvider.currentTree; this.properties = this.preferenceTreeProvider.propertyList; @@ -98,11 +98,12 @@ export class PreferencesEditorWidget extends ReactWidget { ); } - protected handleChangeDisplay = (): void => { - // This is here to avoid using the synthetic event asynchronously - this.currentDisplay = this.preferenceTreeProvider.currentTree; - this.properties = this.preferenceTreeProvider.propertyList; - this.node.scrollTop = 0; + protected handleChangeDisplay = (didGenerateNewTree: boolean): void => { + if (didGenerateNewTree) { + this.currentDisplay = this.preferenceTreeProvider.currentTree; + this.properties = this.preferenceTreeProvider.propertyList; + this.node.scrollTop = 0; + } this.update(); }; @@ -180,7 +181,8 @@ export class PreferencesEditorWidget extends ReactWidget { if (nodeID) { const el = document.getElementById(`${nodeID}-editor`); if (el) { - el.scrollIntoView(); + // Timeout to allow render cycle to finish. + setTimeout(() => el.scrollIntoView()); } } } diff --git a/packages/preferences/src/browser/views/preference-tree-widget.tsx b/packages/preferences/src/browser/views/preference-tree-widget.tsx index bd3d208f0c42c..48a9dfdfc7965 100644 --- a/packages/preferences/src/browser/views/preference-tree-widget.tsx +++ b/packages/preferences/src/browser/views/preference-tree-widget.tsx @@ -55,7 +55,11 @@ export class PreferencesTreeWidget extends TreeWidget { @postConstruct() init(): void { super.init(); - this.preferencesEventService.onDisplayChanged.event(() => this.updateDisplay()); + this.preferencesEventService.onDisplayChanged.event(didChangeTree => { + if (didChangeTree) { + this.updateDisplay(); + } + }); this.preferencesEventService.onEditorScroll.event(e => { this.handleEditorScroll(e.firstVisibleChildId); }); @@ -70,12 +74,14 @@ export class PreferencesTreeWidget extends TreeWidget { this.firstVisibleLeafNodeID = firstVisibleChildId; this.model.expandNode(expansionAncestor); this.collapseAllExcept(expansionAncestor); - this.model.selectNode(selectionAncestor); + if (selectionAncestor) { + this.model.selectNode(selectionAncestor); + } } this.shouldFireSelectionEvents = true; } - protected collapseAllExcept(openNode: Preference.TreeExtension): void { + protected collapseAllExcept(openNode: Preference.TreeExtension | undefined): void { const children = (this.model.root as CompositeTreeNode).children as ExpandableTreeNode[]; children.forEach(child => { if (child !== openNode && child.expanded) { @@ -84,7 +90,7 @@ export class PreferencesTreeWidget extends TreeWidget { }); } - protected getAncestorsForVisibleNode(visibleNodeID: string): { selectionAncestor: SelectableTreeNode, expansionAncestor: ExpandableTreeNode; } { + protected getAncestorsForVisibleNode(visibleNodeID: string): { selectionAncestor: SelectableTreeNode | undefined, expansionAncestor: ExpandableTreeNode | undefined; } { const isNonLeafNode = visibleNodeID.endsWith('-id'); const isSubgroupNode = isNonLeafNode && visibleNodeID.includes('.'); let expansionAncestor: ExpandableTreeNode; @@ -92,7 +98,7 @@ export class PreferencesTreeWidget extends TreeWidget { if (isSubgroupNode) { selectionAncestor = this.model.getNode(visibleNodeID) as SelectableTreeNode; - expansionAncestor = selectionAncestor.parent as ExpandableTreeNode; + expansionAncestor = selectionAncestor?.parent as ExpandableTreeNode; } else if (isNonLeafNode) { selectionAncestor = this.model.getNode(visibleNodeID) as SelectableTreeNode; expansionAncestor = selectionAncestor as Preference.TreeExtension as ExpandableTreeNode; @@ -106,7 +112,8 @@ export class PreferencesTreeWidget extends TreeWidget { selectionAncestor = this.model.getNode(subgroupID) as SelectableTreeNode; } else { // The last selectable child that precedes the visible item alphabetically - selectionAncestor = [...expansionAncestor.children].reverse().find(child => child.visible && child.id < visibleNodeID) as SelectableTreeNode || expansionAncestor; + selectionAncestor = [...(expansionAncestor?.children || [])] + .reverse().find(child => child.visible && child.id < visibleNodeID) as SelectableTreeNode || expansionAncestor; } } return { selectionAncestor, expansionAncestor }; @@ -124,6 +131,13 @@ export class PreferencesTreeWidget extends TreeWidget { const nodes = Object.keys(this.preferenceTreeProvider.propertyList) .map(propertyName => ({ [propertyName]: this.preferenceTreeProvider.propertyList[propertyName] })); this.decorator.fireDidChangeDecorations(nodes); + // If the tree has changed but we know the visible node, scroll to it. + if (this.firstVisibleLeafNodeID) { + const { selectionAncestor } = this.getAncestorsForVisibleNode(this.firstVisibleLeafNodeID); + if (selectionAncestor?.visible) { + this.preferencesEventService.onNavTreeSelection.fire({ nodeID: this.firstVisibleLeafNodeID }); + } + } this.update(); } }