Refactor Preference UI; Add commonly used section

Signed-off-by: Colin Grant <colin.grant@ericsson.com>

packages/core/src/browser/preferences/preference-provider.ts

@@ -231,6 +231,15 @@ export abstract class PreferenceProvider implements Disposable {
         return source;
     }
 
+    /**
+     * Handles deep equality with the possibility of `undefined`
+     */
+    static deepEqual(a: JSONValue | undefined, b: JSONValue | undefined): boolean {
+        if (a === b) { return true; }
+        if (a === undefined || b === undefined) { return false; }
+        return JSONExt.deepEqual(a, b);
+    }
+
     protected getParsedContent(jsonData: any): { [key: string]: any } {
         const preferences: { [key: string]: any } = {};
         if (typeof jsonData !== 'object') {

packages/core/src/browser/preferences/preference-service.ts

@@ -24,7 +24,7 @@ import { PreferenceSchemaProvider } from './preference-contribution';
 import URI from '../../common/uri';
 import { PreferenceScope } from './preference-scope';
 import { PreferenceConfigurations } from './preference-configurations';
-import { JSONExt } from '@phosphor/coreutils/lib/json';
+import { JSONExt, JSONValue } from '@phosphor/coreutils/lib/json';
 import { OverridePreferenceName, PreferenceLanguageOverrideService } from './preference-language-override-service';
 
 export { PreferenceScope };
@@ -186,7 +186,7 @@ export interface PreferenceService extends Disposable {
      *
      * @return an object containing the value of the given preference for all scopes.
      */
-    inspect<T>(preferenceName: string, resourceUri?: string): PreferenceInspection<T> | undefined;
+    inspect<T extends JSONValue>(preferenceName: string, resourceUri?: string): PreferenceInspection<T> | undefined;
     /**
      * Returns a new preference identifier based on the given OverridePreferenceName.
      *
@@ -238,7 +238,7 @@ export interface PreferenceService extends Disposable {
 /**
  * Return type of the {@link PreferenceService.inspect} call.
  */
-export interface PreferenceInspection<T> {
+export interface PreferenceInspection<T = JSONValue> {
     /**
      * The preference identifier.
      */

packages/core/src/browser/widgets/widget.ts

@@ -36,6 +36,10 @@ export const COLLAPSED_CLASS = 'theia-mod-collapsed';
 export const BUSY_CLASS = 'theia-mod-busy';
 export const SELECTED_CLASS = 'theia-mod-selected';
 export const FOCUS_CLASS = 'theia-mod-focus';
+export const DEFAULT_SCROLL_OPTIONS: PerfectScrollbar.Options = {
+    suppressScrollX: true,
+    minScrollbarLength: 35,
+};
 
 @injectable()
 export class BaseWidget extends Widget {

packages/preferences/src/browser/abstract-resource-preference-provider.ts

@@ -18,7 +18,6 @@
 /* eslint-disable no-null/no-null */
 
 import * as jsoncparser from 'jsonc-parser';
-import { JSONExt } from '@theia/core/shared/@phosphor/coreutils';
 import { inject, injectable, postConstruct } from '@theia/core/shared/inversify';
 import { MessageService } from '@theia/core/lib/common/message-service';
 import { Disposable } from '@theia/core/lib/common/disposable';
@@ -216,9 +215,7 @@ export abstract class AbstractResourcePreferenceProvider extends PreferenceProvi
                     continue;
                 }
             }
-            if (newValue === undefined && oldValue !== newValue
-                || oldValue === undefined && newValue !== oldValue // JSONExt.deepEqual() does not support handling `undefined`
-                || !JSONExt.deepEqual(oldValue, newValue)) {
+            if (!PreferenceProvider.deepEqual(newValue, oldValue)) {
                 prefChanges.push({
                     preferenceName: prefName, newValue, oldValue, scope: this.getScope(), domain: this.getDomain()
                 });

packages/preferences/src/browser/preference-tree-model.ts

@@ -24,25 +24,34 @@ import {
     PreferenceSchemaProvider,
     PreferenceDataProperty,
     NodeProps,
-    ExpandableTreeNode
+    ExpandableTreeNode,
+    SelectableTreeNode,
 } from '@theia/core/lib/browser';
 import { Emitter } from '@theia/core';
 import { PreferencesSearchbarWidget } from './views/preference-searchbar-widget';
-import { PreferenceTreeGenerator } from './util/preference-tree-generator';
+import { PreferenceTreeGenerator, COMMONLY_USED_SECTION_PREFIX } from './util/preference-tree-generator';
 import * as fuzzy from '@theia/core/shared/fuzzy';
 import { PreferencesScopeTabBar } from './views/preference-scope-tabbar-widget';
 import { Preference } from './util/preference-types';
-import { Event } from '@theia/core/src/common';
+import { Event } from '@theia/core/lib/common';
 
-export interface PreferenceTreeNodeRow extends TreeWidget.NodeRow {
-    visibleChildren: number;
-    isExpansible?: boolean;
-}
 export interface PreferenceTreeNodeProps extends NodeProps {
     visibleChildren: number;
     isExpansible?: boolean;
 }
 
+export interface PreferenceTreeNodeRow extends Readonly<TreeWidget.NodeRow>, PreferenceTreeNodeProps {
+    node: Preference.TreeNode;
+}
+export enum PreferenceFilterChangeSource {
+    Schema,
+    Search,
+    Scope,
+}
+export interface PreferenceFilterChangeEvent {
+    source: PreferenceFilterChangeSource
+}
+
 @injectable()
 export class PreferenceTreeModel extends TreeModelImpl {
 
@@ -51,7 +60,7 @@ export class PreferenceTreeModel extends TreeModelImpl {
     @inject(PreferenceTreeGenerator) protected readonly treeGenerator: PreferenceTreeGenerator;
     @inject(PreferencesScopeTabBar) protected readonly scopeTracker: PreferencesScopeTabBar;
 
-    protected readonly onTreeFilterChangedEmitter = new Emitter<{ filterCleared: boolean; rows: Map<string, PreferenceTreeNodeRow>; }>();
+    protected readonly onTreeFilterChangedEmitter = new Emitter<PreferenceFilterChangeEvent>();
     readonly onFilterChanged = this.onTreeFilterChangedEmitter.event;
 
     protected lastSearchedFuzzy: string = '';
@@ -91,18 +100,21 @@ export class PreferenceTreeModel extends TreeModelImpl {
         this.toDispose.pushAll([
             this.treeGenerator.onSchemaChanged(newTree => {
                 this.root = newTree;
-                this.updateFilteredRows();
+                this.updateFilteredRows(PreferenceFilterChangeSource.Schema);
             }),
             this.scopeTracker.onScopeChanged(scopeDetails => {
-                this._currentScope = Number(scopeDetails.scope);
-                this.updateFilteredRows();
+                this._currentScope = scopeDetails.scope;
+                this.updateFilteredRows(PreferenceFilterChangeSource.Scope);
             }),
             this.filterInput.onFilterChanged(newSearchTerm => {
+                const wasFiltered = this._isFiltered;
                 this.lastSearchedLiteral = newSearchTerm;
                 this.lastSearchedFuzzy = newSearchTerm.replace(/\s/g, '');
-                const wasFiltered = this._isFiltered;
                 this._isFiltered = newSearchTerm.length > 2;
-                this.updateFilteredRows(wasFiltered && !this._isFiltered);
+                this.updateFilteredRows(PreferenceFilterChangeSource.Search);
+                if (this.isFiltered) {
+                    this.expandAll();
+                }
             }),
             this.onFilterChanged(() => {
                 this.filterInput.updateResultsCount(this._totalVisibleLeaves);
@@ -123,10 +135,10 @@ export class PreferenceTreeModel extends TreeModelImpl {
                 pruneCollapsed: false,
                 pruneSiblings: true
             })) {
-                if (TreeNode.isVisible(node)) {
-                    if (CompositeTreeNode.is(node) || this.passesCurrentFilters(node.id)) {
+                if (TreeNode.isVisible(node) && Preference.TreeNode.is(node)) {
+                    const { id } = Preference.TreeNode.getGroupAndIdFromNodeId(node.id);
+                    if (CompositeTreeNode.is(node) || this.passesCurrentFilters(node, id)) {
                         const depth = this.getDepthForNode(depths, node);
-
                         this.updateVisibleChildren(node);
 
                         this._currentRows.set(node.id, {
@@ -141,21 +153,27 @@ export class PreferenceTreeModel extends TreeModelImpl {
         }
     }
 
-    protected updateFilteredRows(filterWasCleared: boolean = false): void {
+    protected updateFilteredRows(source: PreferenceFilterChangeSource): void {
         this.updateRows();
-        this.onTreeFilterChangedEmitter.fire({ filterCleared: filterWasCleared, rows: this._currentRows });
+        this.onTreeFilterChangedEmitter.fire({ source });
     }
 
-    protected passesCurrentFilters(nodeID: string): boolean {
-        const currentNodeShouldBeVisible = this.schemaProvider.isValidInScope(nodeID, this._currentScope)
-            && (
-                !this._isFiltered // search too short.
-                || fuzzy.test(this.lastSearchedFuzzy, nodeID || '') // search matches preference name.
-                // search matches description. Fuzzy isn't ideal here because the score depends on the order of discovery.
-                || (this.schemaProvider.getCombinedSchema().properties[nodeID].description || '').includes(this.lastSearchedLiteral)
-            );
-
-        return currentNodeShouldBeVisible;
+    protected passesCurrentFilters(node: Preference.LeafNode, prefID: string): boolean {
+        if (!this.schemaProvider.isValidInScope(prefID, this._currentScope)) {
+            return false;
+        }
+        if (!this._isFiltered) {
+            return true;
+        }
+        // When filtering, VSCode will render an item that is present in the commonly used section only once but render both its possible parents in the left-hand tree.
+        // E.g. searching for editor.renderWhitespace will show one item in the main panel, but both 'Commonly Used' and 'Text Editor' in the left tree.
+        // That seems counterintuitive and introduces a number of special cases, so I prefer to remove the commonly used section entirely when the user searches.
+        if (node.id.startsWith(COMMONLY_USED_SECTION_PREFIX)) {
+            return false;
+        }
+        return fuzzy.test(this.lastSearchedFuzzy, prefID) // search matches preference name.
+            // search matches description. Fuzzy isn't ideal here because the score depends on the order of discovery.
+            || (node.preference.data.description ?? '').includes(this.lastSearchedLiteral);
     }
 
     protected getDepthForNode(depths: Map<CompositeTreeNode | undefined, number>, node: TreeNode): number {
@@ -183,13 +201,38 @@ export class PreferenceTreeModel extends TreeModelImpl {
         }
     }
 
-    collapseAllExcept(openNode: ExpandableTreeNode | undefined): void {
-        this.expandNode(openNode);
-        const children = (this.root as CompositeTreeNode).children as ExpandableTreeNode[];
-        children.forEach(child => {
-            if (child !== openNode && child.expanded) {
-                this.collapseNode(child);
-            }
-        });
+    collapseAllExcept(openNode: TreeNode | undefined): void {
+        if (ExpandableTreeNode.is(openNode)) {
+            this.expandNode(openNode);
+        }
+        if (CompositeTreeNode.is(this.root)) {
+            this.root.children.forEach(child => {
+                if (child !== openNode && ExpandableTreeNode.is(child)) {
+                    this.collapseNode(child);
+                }
+            });
+        }
+    }
+
+    protected expandAll(): void {
+        if (CompositeTreeNode.is(this.root)) {
+            this.root.children.forEach(child => {
+                if (ExpandableTreeNode.is(child)) {
+                    this.expandNode(child);
+                }
+            });
+        }
+    }
+
+    /**
+     * @returns true if selection changed, false otherwise
+     */
+    selectIfNotSelected(node: SelectableTreeNode): boolean {
+        const currentlySelected = this.selectedNodes[0];
+        if (node !== currentlySelected) {
+            this.selectNode(node);
+            return true;
+        }
+        return false;
     }
 }

packages/preferences/src/browser/preferences-contribution.ts

@@ -64,8 +64,8 @@ export class PreferencesContribution extends AbstractViewContribution<Preference
         commands.registerCommand(PreferencesCommands.OPEN_PREFERENCES_JSON_TOOLBAR, {
             isEnabled: () => true,
             isVisible: w => this.withWidget(w, () => true),
-            execute: (preferenceNode: Preference.NodeWithValueInAllScopes) => {
-                this.openPreferencesJSON(preferenceNode);
+            execute: (preferenceId: string) => {
+                this.openPreferencesJSON(preferenceId);
             }
         });
         commands.registerCommand(PreferencesCommands.COPY_JSON_NAME, {
@@ -136,15 +136,14 @@ export class PreferencesContribution extends AbstractViewContribution<Preference
         });
     }
 
-    protected async openPreferencesJSON(preferenceNode: Preference.NodeWithValueInAllScopes): Promise<void> {
-        const wasOpenedFromEditor = preferenceNode.constructor !== PreferencesWidget;
+    protected async openPreferencesJSON(opener: string | PreferencesWidget): Promise<void> {
         const { scope, activeScopeIsFolder, uri } = this.scopeTracker.currentScope;
         const scopeID = Number(scope);
-        const preferenceId = wasOpenedFromEditor ? preferenceNode.id : '';
-        // when opening from toolbar, widget is passed as arg by default (we don't need this info)
-        if (wasOpenedFromEditor && preferenceNode.preference.values) {
-            const currentPreferenceValue = preferenceNode.preference.values;
-            const valueInCurrentScope = Preference.getValueInScope(currentPreferenceValue, scopeID) ?? currentPreferenceValue.defaultValue;
+        let preferenceId = '';
+        if (typeof opener === 'string') {
+            preferenceId = opener;
+            const currentPreferenceValue = this.preferenceService.inspect(preferenceId, uri);
+            const valueInCurrentScope = Preference.getValueInScope(currentPreferenceValue, scopeID) ?? currentPreferenceValue?.defaultValue;
             this.preferenceService.set(preferenceId, valueInCurrentScope, scopeID, uri);
         }
 
@@ -153,7 +152,7 @@ export class PreferencesContribution extends AbstractViewContribution<Preference
         if (jsonUriToOpen) {
             jsonEditorWidget = await this.editorManager.open(jsonUriToOpen);
 
-            if (wasOpenedFromEditor) {
+            if (preferenceId) {
                 const text = jsonEditorWidget.editor.document.getText();
                 if (preferenceId) {
                     const { index } = text.match(preferenceId)!;
@@ -164,9 +163,9 @@ export class PreferencesContribution extends AbstractViewContribution<Preference
         }
     }
 
-    private async obtainConfigUri(serializedScope: number, activeScopeIsFolder: string, resource: string): Promise<URI | undefined> {
+    private async obtainConfigUri(serializedScope: number, activeScopeIsFolder: boolean, resource?: string): Promise<URI | undefined> {
         let scope: PreferenceScope = serializedScope;
-        if (activeScopeIsFolder === 'true') {
+        if (activeScopeIsFolder) {
             scope = PreferenceScope.Folder;
         }
         const resourceUri = !!resource ? resource : undefined;