From bcb954e8158daa91f9ab7114df4b080e0f504339 Mon Sep 17 00:00:00 2001 From: Benjamin Christopher Simmonds <44439583+benibenj@users.noreply.github.com> Date: Fri, 6 Sep 2024 15:08:40 +0200 Subject: [PATCH] Support Swapping the tree model (#227773) * Support Swapping the model * remove get model as it is provided by abstract tree --- src/vs/base/browser/ui/tree/abstractTree.ts | 184 +++++++++------ .../test/browser/ui/tree/objectTree.test.ts | 217 +++++++++++++++--- .../testing/test/browser/testObjectTree.ts | 4 - 3 files changed, 304 insertions(+), 101 deletions(-) diff --git a/src/vs/base/browser/ui/tree/abstractTree.ts b/src/vs/base/browser/ui/tree/abstractTree.ts index b40fe8a52e30c..97f3c6015fb32 100644 --- a/src/vs/base/browser/ui/tree/abstractTree.ts +++ b/src/vs/base/browser/ui/tree/abstractTree.ts @@ -66,7 +66,7 @@ class TreeNodeListDragAndDrop implements IListDragAndDrop< private autoExpandDisposable: IDisposable = Disposable.None; private readonly disposables = new DisposableStore(); - constructor(private model: ITreeModel, private dnd: ITreeDragAndDrop) { } + constructor(private modelProvider: () => ITreeModel, private dnd: ITreeDragAndDrop) { } getDragURI(node: ITreeNode): string | null { return this.dnd.getDragURI(node.element); @@ -99,10 +99,11 @@ class TreeNodeListDragAndDrop implements IListDragAndDrop< if (didChangeAutoExpandNode && typeof result !== 'boolean' && result.autoExpand) { this.autoExpandDisposable = disposableTimeout(() => { - const ref = this.model.getNodeLocation(targetNode); + const model = this.modelProvider(); + const ref = model.getNodeLocation(targetNode); - if (this.model.isCollapsed(ref)) { - this.model.setCollapsed(ref, false); + if (model.isCollapsed(ref)) { + model.setCollapsed(ref, false); } this.autoExpandNode = undefined; @@ -120,17 +121,19 @@ class TreeNodeListDragAndDrop implements IListDragAndDrop< } if (result.bubble === TreeDragOverBubble.Up) { - const ref = this.model.getNodeLocation(targetNode); - const parentRef = this.model.getParentNodeLocation(ref); - const parentNode = this.model.getNode(parentRef); - const parentIndex = parentRef && this.model.getListIndex(parentRef); + const model = this.modelProvider(); + const ref = model.getNodeLocation(targetNode); + const parentRef = model.getParentNodeLocation(ref); + const parentNode = model.getNode(parentRef); + const parentIndex = parentRef && model.getListIndex(parentRef); return this.onDragOver(data, parentNode, parentIndex, targetSector, originalEvent, false); } - const ref = this.model.getNodeLocation(targetNode); - const start = this.model.getListIndex(ref); - const length = this.model.getListRenderCount(ref); + const model = this.modelProvider(); + const ref = model.getNodeLocation(targetNode); + const start = model.getListIndex(ref); + const length = model.getListRenderCount(ref); return { ...result, feedback: range(start, start + length) }; } @@ -152,7 +155,7 @@ class TreeNodeListDragAndDrop implements IListDragAndDrop< } } -function asListOptions(model: ITreeModel, options?: IAbstractTreeOptions): IListOptions> | undefined { +function asListOptions(modelProvider: () => ITreeModel, options?: IAbstractTreeOptions): IListOptions> | undefined { return options && { ...options, identityProvider: options.identityProvider && { @@ -160,7 +163,7 @@ function asListOptions(model: ITreeModel(model: ITreeModel implements IListR this.activeIndentNodes = set; } + setModel(model: ITreeModel): void { + this.model = model; + } + dispose(): void { this.renderedNodes.clear(); this.renderedElements.clear(); @@ -1039,7 +1047,6 @@ class FindController implements IDisposable { constructor( private tree: AbstractTree, - model: ITreeModel, private view: List>, private filter: FindFilter, private readonly contextViewProvider: IContextViewProvider, @@ -1047,7 +1054,6 @@ class FindController implements IDisposable { ) { this._mode = tree.options.defaultFindMode ?? TreeFindMode.Highlight; this._matchType = tree.options.defaultFindMatchType ?? TreeFindMatchType.Fuzzy; - model.onDidSpliceModel(this.onDidSpliceModel, this, this.disposables); } updateOptions(optionsUpdate: IAbstractTreeOptionsUpdate = {}): void { @@ -1060,6 +1066,10 @@ class FindController implements IDisposable { } } + isOpened(): boolean { + return !!this.widget; + } + open(): void { if (this.widget) { this.widget.focus(); @@ -1125,16 +1135,7 @@ class FindController implements IDisposable { this.render(); } - private onDidSpliceModel(): void { - if (!this.widget || this.pattern.length === 0) { - return; - } - - this.tree.refilter(); - this.render(); - } - - private render(): void { + render(): void { const noMatches = this.filter.totalCount > 0 && this.filter.matchCount === 0; if (this.pattern && noMatches) { @@ -1272,7 +1273,7 @@ class StickyScrollController extends Disposable { constructor( private readonly tree: AbstractTree, - private readonly model: ITreeModel, + private model: ITreeModel, private readonly view: List>, renderers: TreeRenderer[], private readonly treeDelegate: IListVirtualDelegate>, @@ -1552,6 +1553,10 @@ class StickyScrollController extends Disposable { } return { stickyScrollMaxItemCount }; } + + setModel(model: ITreeModel): void { + this.model = model; + } } class StickyScrollWidget implements IDisposable { @@ -2493,7 +2498,14 @@ export abstract class AbstractTree implements IDisposable get onDidFocus(): Event { return this.view.onDidFocus; } get onDidBlur(): Event { return this.view.onDidBlur; } - get onDidChangeModel(): Event { return Event.signal(this.model.onDidSpliceModel); } + private readonly onDidSwapModel = this.disposables.add(new Emitter()); + private readonly onDidChangeModelRelay = this.disposables.add(new Relay()); + private readonly onDidSpliceModelRelay = this.disposables.add(new Relay>()); + private readonly onDidChangeCollapseStateRelay = this.disposables.add(new Relay>()); + private readonly onDidChangeRenderNodeCountRelay = this.disposables.add(new Relay>()); + private readonly onDidChangeActiveNodesRelay = this.disposables.add(new Relay[]>()); + + get onDidChangeModel(): Event { return Event.any(this.onDidChangeModelRelay.event, this.onDidSwapModel.event); } get onDidChangeCollapseState(): Event> { return this.model.onDidChangeCollapseState; } get onDidChangeRenderNodeCount(): Event> { return this.model.onDidChangeRenderNodeCount; } @@ -2535,11 +2547,9 @@ export abstract class AbstractTree implements IDisposable this.model = this.createModel(_user, _options); this.treeDelegate = new ComposedTreeDelegate>(delegate); - const onDidChangeCollapseStateRelay = new Relay>(); - const onDidChangeActiveNodes = new Relay[]>(); - const activeNodes = this.disposables.add(new EventCollection(onDidChangeActiveNodes.event)); + const activeNodes = this.disposables.add(new EventCollection(this.onDidChangeActiveNodesRelay.event)); const renderedIndentGuides = new SetMap, HTMLDivElement>(); - this.renderers = renderers.map(r => new TreeRenderer(r, this.model, onDidChangeCollapseStateRelay.event, activeNodes, renderedIndentGuides, _options)); + this.renderers = renderers.map(r => new TreeRenderer(r, this.model, this.onDidChangeCollapseStateRelay.event, activeNodes, renderedIndentGuides, _options)); for (const r of this.renderers) { this.disposables.add(r); } @@ -2547,44 +2557,9 @@ export abstract class AbstractTree implements IDisposable this.focus = new Trait(() => this.view.getFocusedElements()[0], _options.identityProvider); this.selection = new Trait(() => this.view.getSelectedElements()[0], _options.identityProvider); this.anchor = new Trait(() => this.view.getAnchorElement(), _options.identityProvider); - this.view = new TreeNodeList(_user, container, this.treeDelegate, this.renderers, this.focus, this.selection, this.anchor, { ...asListOptions(this.model, _options), tree: this, stickyScrollProvider: () => this.stickyScrollController }); - - this.disposables.add(this.model.onDidSpliceRenderedNodes(({ start, deleteCount, elements }) => this.view.splice(start, deleteCount, elements))); - - onDidChangeCollapseStateRelay.input = this.model.onDidChangeCollapseState; - - const onDidModelSplice = Event.forEach(this.model.onDidSpliceModel, e => { - this.eventBufferer.bufferEvents(() => { - this.focus.onDidModelSplice(e); - this.selection.onDidModelSplice(e); - }); - }, this.disposables); - - // Make sure the `forEach` always runs - onDidModelSplice(() => null, null, this.disposables); - - // Active nodes can change when the model changes or when focus or selection change. - // We debounce it with 0 delay since these events may fire in the same stack and we only - // want to run this once. It also doesn't matter if it runs on the next tick since it's only - // a nice to have UI feature. - const activeNodesEmitter = this.disposables.add(new Emitter[]>()); - const activeNodesDebounce = this.disposables.add(new Delayer(0)); - this.disposables.add(Event.any(onDidModelSplice, this.focus.onDidChange, this.selection.onDidChange)(() => { - activeNodesDebounce.trigger(() => { - const set = new Set>(); + this.view = new TreeNodeList(_user, container, this.treeDelegate, this.renderers, this.focus, this.selection, this.anchor, { ...asListOptions(() => this.model, _options), tree: this, stickyScrollProvider: () => this.stickyScrollController }); - for (const node of this.focus.getNodes()) { - set.add(node); - } - - for (const node of this.selection.getNodes()) { - set.add(node); - } - - activeNodesEmitter.fire([...set.values()]); - }); - })); - onDidChangeActiveNodes.input = activeNodesEmitter.event; + this.setupModel(this.model); // model needs to be setup after the traits have been created if (_options.keyboardSupport !== false) { const onKeyDown = Event.chain(this.view.onKeyDown, $ => @@ -2599,12 +2574,19 @@ export abstract class AbstractTree implements IDisposable if ((_options.findWidgetEnabled ?? true) && _options.keyboardNavigationLabelProvider && _options.contextViewProvider) { const opts = this.options.findWidgetStyles ? { styles: this.options.findWidgetStyles } : undefined; - this.findController = new FindController(this, this.model, this.view, filter!, _options.contextViewProvider, opts); + this.findController = new FindController(this, this.view, filter!, _options.contextViewProvider, opts); this.focusNavigationFilter = node => this.findController!.shouldAllowFocus(node); this.onDidChangeFindOpenState = this.findController.onDidChangeOpenState; this.disposables.add(this.findController); this.onDidChangeFindMode = this.findController.onDidChangeMode; this.onDidChangeFindMatchType = this.findController.onDidChangeMatchType; + this.disposables.add(this.onDidSpliceModelRelay.event(() => { + if (!this.findController!.isOpened() || this.findController!.pattern.length === 0) { + return; + } + this.refilter(); + this.findController!.render(); + })); } else { this.onDidChangeFindMode = Event.None; this.onDidChangeFindMatchType = Event.None; @@ -3109,6 +3091,69 @@ export abstract class AbstractTree implements IDisposable protected abstract createModel(user: string, options: IAbstractTreeOptions): ITreeModel; + private readonly modelDisposables = new DisposableStore(); + private setupModel(model: ITreeModel) { + this.modelDisposables.clear(); + + this.modelDisposables.add(model.onDidSpliceRenderedNodes(({ start, deleteCount, elements }) => this.view.splice(start, deleteCount, elements))); + + const onDidModelSplice = Event.forEach(model.onDidSpliceModel, e => { + this.eventBufferer.bufferEvents(() => { + this.focus.onDidModelSplice(e); + this.selection.onDidModelSplice(e); + }); + }, this.modelDisposables); + + // Make sure the `forEach` always runs + onDidModelSplice(() => null, null, this.modelDisposables); + + // Active nodes can change when the model changes or when focus or selection change. + // We debounce it with 0 delay since these events may fire in the same stack and we only + // want to run this once. It also doesn't matter if it runs on the next tick since it's only + // a nice to have UI feature. + const activeNodesEmitter = this.modelDisposables.add(new Emitter[]>()); + const activeNodesDebounce = this.modelDisposables.add(new Delayer(0)); + this.modelDisposables.add(Event.any(onDidModelSplice, this.focus.onDidChange, this.selection.onDidChange)(() => { + activeNodesDebounce.trigger(() => { + const set = new Set>(); + + for (const node of this.focus.getNodes()) { + set.add(node); + } + + for (const node of this.selection.getNodes()) { + set.add(node); + } + + activeNodesEmitter.fire([...set.values()]); + }); + })); + + this.onDidChangeActiveNodesRelay.input = activeNodesEmitter.event; + this.onDidChangeModelRelay.input = Event.signal(model.onDidSpliceModel); + this.onDidChangeCollapseStateRelay.input = model.onDidChangeCollapseState; + this.onDidChangeRenderNodeCountRelay.input = model.onDidChangeRenderNodeCount; + } + + setModel(newModel: ITreeModel) { + const oldModel = this.model; + + this.model = newModel; + this.setupModel(newModel); + + this.renderers.forEach(r => r.setModel(newModel)); + this.stickyScrollController?.setModel(newModel); + + this.view.splice(0, oldModel.getListRenderCount(oldModel.rootRef)); + this.model.refilter(); + + this.onDidSwapModel.fire(); + } + + getModel(): ITreeModel { + return this.model; + } + navigate(start?: TRef): ITreeNavigator { return new TreeNavigator(this.view, this.model, start); } @@ -3117,6 +3162,7 @@ export abstract class AbstractTree implements IDisposable dispose(this.disposables); this.stickyScrollController?.dispose(); this.view.dispose(); + this.modelDisposables.dispose(); } } diff --git a/src/vs/base/test/browser/ui/tree/objectTree.test.ts b/src/vs/base/test/browser/ui/tree/objectTree.test.ts index 7062517ebb812..af45e2b2254fb 100644 --- a/src/vs/base/test/browser/ui/tree/objectTree.test.ts +++ b/src/vs/base/test/browser/ui/tree/objectTree.test.ts @@ -5,10 +5,17 @@ import assert from 'assert'; import { IIdentityProvider, IListVirtualDelegate } from '../../../../browser/ui/list/list.js'; -import { ICompressedTreeNode } from '../../../../browser/ui/tree/compressedObjectTreeModel.js'; +import { CompressibleObjectTreeModel, ICompressedTreeNode } from '../../../../browser/ui/tree/compressedObjectTreeModel.js'; import { CompressibleObjectTree, ICompressibleTreeRenderer, ObjectTree } from '../../../../browser/ui/tree/objectTree.js'; import { ITreeNode, ITreeRenderer } from '../../../../browser/ui/tree/tree.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../common/utils.js'; +import { ObjectTreeModel } from '../../../../browser/ui/tree/objectTreeModel.js'; + +function getRowsTextContent(container: HTMLElement): string[] { + const rows = [...container.querySelectorAll('.monaco-list-row')]; + rows.sort((a, b) => parseInt(a.getAttribute('data-index')!) - parseInt(b.getAttribute('data-index')!)); + return rows.map(row => row.querySelector('.monaco-tl-contents')!.textContent!); +} suite('ObjectTree', function () { @@ -184,32 +191,36 @@ suite('ObjectTree', function () { }); }); + class Delegate implements IListVirtualDelegate { + getHeight() { return 20; } + getTemplateId(): string { return 'default'; } + } + + class Renderer implements ITreeRenderer { + readonly templateId = 'default'; + renderTemplate(container: HTMLElement): HTMLElement { + return container; + } + renderElement(element: ITreeNode, index: number, templateData: HTMLElement): void { + templateData.textContent = `${element.element}`; + } + disposeTemplate(): void { } + } + + class IdentityProvider implements IIdentityProvider { + getId(element: number): { toString(): string } { + return `${element % 100}`; + } + } + test('traits are preserved according to string identity', function () { const container = document.createElement('div'); container.style.width = '200px'; container.style.height = '200px'; - const delegate = new class implements IListVirtualDelegate { - getHeight() { return 20; } - getTemplateId(): string { return 'default'; } - }; - - const renderer = new class implements ITreeRenderer { - readonly templateId = 'default'; - renderTemplate(container: HTMLElement): HTMLElement { - return container; - } - renderElement(element: ITreeNode, index: number, templateData: HTMLElement): void { - templateData.textContent = `${element.element}`; - } - disposeTemplate(): void { } - }; - - const identityProvider = new class implements IIdentityProvider { - getId(element: number): { toString(): string } { - return `${element % 100}`; - } - }; + const delegate = new Delegate(); + const renderer = new Renderer(); + const identityProvider = new IdentityProvider(); const tree = new ObjectTree('test', container, delegate, [renderer], { identityProvider }); tree.layout(200); @@ -221,13 +232,108 @@ suite('ObjectTree', function () { tree.setChildren(null, [{ element: 100 }, { element: 101 }, { element: 102 }, { element: 103 }]); assert.deepStrictEqual(tree.getFocus(), [101]); }); -}); -function getRowsTextContent(container: HTMLElement): string[] { - const rows = [...container.querySelectorAll('.monaco-list-row')]; - rows.sort((a, b) => parseInt(a.getAttribute('data-index')!) - parseInt(b.getAttribute('data-index')!)); - return rows.map(row => row.querySelector('.monaco-tl-contents')!.textContent!); -} + test('swap model', function () { + const container = document.createElement('div'); + container.style.width = '200px'; + container.style.height = '200px'; + + const delegate = new Delegate(); + const renderer = new Renderer(); + const identityProvider = new IdentityProvider(); + + const tree = new ObjectTree('test', container, delegate, [renderer], { identityProvider }); + tree.layout(200); + + tree.setChildren(null, [{ element: 1 }, { element: 2 }, { element: 3 }, { element: 4 }]); + assert.deepStrictEqual(getRowsTextContent(container), ['1', '2', '3', '4']); + + const oldModel = tree.getModel(); + + const newModel = new ObjectTreeModel('test', {}); + tree.setModel(newModel); + assert.deepStrictEqual(getRowsTextContent(container), []); + + newModel.setChildren(null, [ + { element: 1, children: [{ element: 11 }] }, + { element: 2 } + ]); + assert.deepStrictEqual(getRowsTextContent(container), ['1', '11', '2']); + + tree.setChildren(11, [ + { element: 111, children: [{ element: 1111 }] }, + { element: 112 }, + ]); + assert.deepStrictEqual(getRowsTextContent(container), ['1', '11', '111', '1111', '112', '2']); + + tree.setModel(oldModel); + assert.deepStrictEqual(getRowsTextContent(container), ['1', '2', '3', '4']); + }); + + test('swap model events', function () { + const container = document.createElement('div'); + container.style.width = '200px'; + container.style.height = '200px'; + + const delegate = new Delegate(); + const renderer = new Renderer(); + const identityProvider = new IdentityProvider(); + + const tree = new ObjectTree('test', container, delegate, [renderer], { identityProvider }); + tree.layout(200); + + tree.setChildren(null, [{ element: 1 }, { element: 2 }, { element: 3 }, { element: 4 }]); + assert.deepStrictEqual(getRowsTextContent(container), ['1', '2', '3', '4']); + + const newModel = new ObjectTreeModel('test', {}); + newModel.setChildren(null, [ + { element: 1, children: [{ element: 11 }] }, + { element: 2 } + ]); + + let didChangeModel = false; + let didChangeRenderNodeCount = false; + let didChangeCollapseState = false; + + tree.onDidChangeModel(() => { didChangeModel = true; }); + tree.onDidChangeRenderNodeCount(() => { didChangeRenderNodeCount = true; }); + tree.onDidChangeCollapseState(() => { didChangeCollapseState = true; }); + + tree.setModel(newModel); + + assert.strictEqual(didChangeModel, true); + assert.strictEqual(didChangeRenderNodeCount, false); + assert.strictEqual(didChangeCollapseState, false); + }); + + // TODO@benibenj: Make sure user is updated in the tree when model is swapped + test.skip('swap model TreeError uses updated user', function () { + const container = document.createElement('div'); + container.style.width = '200px'; + container.style.height = '200px'; + + const delegate = new Delegate(); + const renderer = new Renderer(); + + const tree = new ObjectTree('test', container, delegate, [renderer], {}); + tree.layout(200); + + tree.setChildren(null, [{ element: 1 }]); + + const newModel = new ObjectTreeModel('NEW_USER_NAME', {}); + tree.setModel(newModel); + + try { + // This should throw an error because no identity provider is provided + tree.getViewState(); + } catch (e) { + assert.strictEqual(e.message.includes('NEW_USER_NAME'), true); + return; + } + + assert.fail('Expected error'); + }); +}); suite('CompressibleObjectTree', function () { @@ -368,4 +474,59 @@ suite('CompressibleObjectTree', function () { tree.updateOptions({ compressionEnabled: true }); assert.deepStrictEqual(getRowsTextContent(container), ['1/11/111', '1111', '1112', '1113']); }); + + test('swapModel', () => { + const container = document.createElement('div'); + container.style.width = '200px'; + container.style.height = '200px'; + + const tree = ds.add(new CompressibleObjectTree('test', container, new Delegate(), [new Renderer()])); + tree.layout(200); + + tree.setChildren(null, [ + { + element: 1, children: [{ + element: 11, children: [{ + element: 111, children: [ + { element: 1111 }, + { element: 1112 }, + { element: 1113 }, + ] + }] + }] + } + ]); + + assert.deepStrictEqual(getRowsTextContent(container), ['1/11/111', '1111', '1112', '1113']); + + const newModel = new CompressibleObjectTreeModel('test', {}); + newModel.setChildren(null, [ + { + element: 1, children: [{ + element: 11 + }] + }, + { + element: 2, children: [{ + element: 21, children: [ + { element: 211 }, + { element: 212 }, + { element: 213 }, + ] + }] + } + ]); + + tree.setModel(newModel); + assert.deepStrictEqual(getRowsTextContent(container), ['1/11', '2/21', '211', '212', '213']); + + tree.setChildren(11, [ + { element: 111 }, + { element: 112 }, + { element: 113 }, + ]); + + assert.deepStrictEqual(getRowsTextContent(container), ['1/11', '111', '112', '113', '2/21', '211', '212', '213']); + + }); }); diff --git a/src/vs/workbench/contrib/testing/test/browser/testObjectTree.ts b/src/vs/workbench/contrib/testing/test/browser/testObjectTree.ts index b1212f1d07001..74601edb73d95 100644 --- a/src/vs/workbench/contrib/testing/test/browser/testObjectTree.ts +++ b/src/vs/workbench/contrib/testing/test/browser/testObjectTree.ts @@ -57,10 +57,6 @@ class TestObjectTree extends ObjectTree { this.layout(1000, 200); } - public getModel() { - return this.model; - } - public getRendered(getProperty?: string) { const elements = element.querySelectorAll('.monaco-tl-contents'); const sorted = [...elements].sort((a, b) => pos(a) - pos(b));