From 0149146f973e0f739facdf61cc7ef6a72190d155 Mon Sep 17 00:00:00 2001 From: Vlad Arama Date: Wed, 12 Jul 2023 08:36:15 -0400 Subject: [PATCH 01/10] search next/prev result commands and keybindings --- ...arch-in-workspace-frontend-contribution.ts | 42 ++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts b/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts index 754c2020e434f..ed990039993bb 100644 --- a/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts +++ b/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts @@ -57,6 +57,16 @@ export namespace SearchInWorkspaceCommands { category: SEARCH_CATEGORY, label: 'Find in Folder...' }); + export const FOCUS_NEXT_RESULT = Command.toDefaultLocalizedCommand({ + id: 'search-in-workspace.next-result', + category: SEARCH_CATEGORY, + label: 'Focus Next Search Result' + }); + export const FOCUS_PREV_RESULT = Command.toDefaultLocalizedCommand({ + id: 'search-in-workspace.prev-result', + category: SEARCH_CATEGORY, + label: 'Focus Previous Search Result' + }); export const REFRESH_RESULTS = Command.toDefaultLocalizedCommand({ id: 'search-in-workspace.refresh', category: SEARCH_CATEGORY, @@ -169,6 +179,20 @@ export class SearchInWorkspaceFrontendContribution extends AbstractViewContribut } }); + commands.registerCommand(SearchInWorkspaceCommands.FOCUS_NEXT_RESULT, { + execute: async () => { + const widget = await this.openView({ activate: true }); + widget.resultTreeWidget.model.selectNextNode(); + } + }); + + commands.registerCommand(SearchInWorkspaceCommands.FOCUS_PREV_RESULT, { + execute: async () => { + const widget = await this.openView({ activate: true }); + widget.resultTreeWidget.model.selectPrevNode(); + } + }); + commands.registerCommand(SearchInWorkspaceCommands.FIND_IN_FOLDER, this.newMultiUriAwareCommandHandler({ execute: async uris => { const resources: string[] = []; @@ -343,6 +367,14 @@ export class SearchInWorkspaceFrontendContribution extends AbstractViewContribut keybinding: 'shift+alt+f', when: 'explorerResourceIsFolder' }); + keybindings.registerKeybinding({ + command: SearchInWorkspaceCommands.FOCUS_NEXT_RESULT.id, + keybinding: 'f4', + }); + keybindings.registerKeybinding({ + command: SearchInWorkspaceCommands.FOCUS_PREV_RESULT.id, + keybinding: 'shift+f4', + }); keybindings.registerKeybinding({ command: SearchInWorkspaceCommands.DISMISS_RESULT.id, keybinding: isOSX ? 'cmd+backspace' : 'del', @@ -376,9 +408,17 @@ export class SearchInWorkspaceFrontendContribution extends AbstractViewContribut order: '2' }); menus.registerMenuAction(CommonMenus.EDIT_FIND, { - commandId: SearchInWorkspaceCommands.REPLACE_IN_FILES.id, + commandId: SearchInWorkspaceCommands.FOCUS_NEXT_RESULT.id, order: '3' }); + menus.registerMenuAction(CommonMenus.EDIT_FIND, { + commandId: SearchInWorkspaceCommands.FOCUS_PREV_RESULT.id, + order: '4' + }); + menus.registerMenuAction(CommonMenus.EDIT_FIND, { + commandId: SearchInWorkspaceCommands.REPLACE_IN_FILES.id, + order: '5' + }); menus.registerMenuAction(SearchInWorkspaceResultTreeWidget.Menus.INTERNAL, { commandId: SearchInWorkspaceCommands.REPLACE_RESULT.id, label: nls.localizeByDefault('Replace'), From 67840a94f4f44ad4bcbf66e510fa4ba5d720dc1e Mon Sep 17 00:00:00 2001 From: Vlad Arama Date: Wed, 12 Jul 2023 11:20:23 -0400 Subject: [PATCH 02/10] Fix review comments and remove menu --- .../search-in-workspace-frontend-contribution.ts | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts b/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts index ed990039993bb..8207e90061633 100644 --- a/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts +++ b/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts @@ -58,12 +58,12 @@ export namespace SearchInWorkspaceCommands { label: 'Find in Folder...' }); export const FOCUS_NEXT_RESULT = Command.toDefaultLocalizedCommand({ - id: 'search-in-workspace.next-result', + id: 'search-in-workspace.focusNextSearchResult', category: SEARCH_CATEGORY, label: 'Focus Next Search Result' }); export const FOCUS_PREV_RESULT = Command.toDefaultLocalizedCommand({ - id: 'search-in-workspace.prev-result', + id: 'search-in-workspace.focusPreviousSearchResult', category: SEARCH_CATEGORY, label: 'Focus Previous Search Result' }); @@ -407,17 +407,9 @@ export class SearchInWorkspaceFrontendContribution extends AbstractViewContribut commandId: SearchInWorkspaceCommands.OPEN_SIW_WIDGET.id, order: '2' }); - menus.registerMenuAction(CommonMenus.EDIT_FIND, { - commandId: SearchInWorkspaceCommands.FOCUS_NEXT_RESULT.id, - order: '3' - }); - menus.registerMenuAction(CommonMenus.EDIT_FIND, { - commandId: SearchInWorkspaceCommands.FOCUS_PREV_RESULT.id, - order: '4' - }); menus.registerMenuAction(CommonMenus.EDIT_FIND, { commandId: SearchInWorkspaceCommands.REPLACE_IN_FILES.id, - order: '5' + order: '3' }); menus.registerMenuAction(SearchInWorkspaceResultTreeWidget.Menus.INTERNAL, { commandId: SearchInWorkspaceCommands.REPLACE_RESULT.id, From 165214a907a2d09b7f8b599bc8b3e7c2b79df22d Mon Sep 17 00:00:00 2001 From: Vlad Arama Date: Wed, 12 Jul 2023 14:26:19 -0400 Subject: [PATCH 03/10] fix id values and add execution condition --- .../search-in-workspace-frontend-contribution.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts b/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts index 8207e90061633..0be484a7f1060 100644 --- a/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts +++ b/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts @@ -58,12 +58,12 @@ export namespace SearchInWorkspaceCommands { label: 'Find in Folder...' }); export const FOCUS_NEXT_RESULT = Command.toDefaultLocalizedCommand({ - id: 'search-in-workspace.focusNextSearchResult', + id: 'search.action.focusNextSearchResult', category: SEARCH_CATEGORY, label: 'Focus Next Search Result' }); export const FOCUS_PREV_RESULT = Command.toDefaultLocalizedCommand({ - id: 'search-in-workspace.focusPreviousSearchResult', + id: 'search.action.focusPreviousSearchResult', category: SEARCH_CATEGORY, label: 'Focus Previous Search Result' }); @@ -181,15 +181,19 @@ export class SearchInWorkspaceFrontendContribution extends AbstractViewContribut commands.registerCommand(SearchInWorkspaceCommands.FOCUS_NEXT_RESULT, { execute: async () => { - const widget = await this.openView({ activate: true }); - widget.resultTreeWidget.model.selectNextNode(); + if (this.tryGetWidget()?.hasResultList()) { + const widget = await this.openView({ activate: true }); + widget.resultTreeWidget.model.selectNextNode(); + } } }); commands.registerCommand(SearchInWorkspaceCommands.FOCUS_PREV_RESULT, { execute: async () => { - const widget = await this.openView({ activate: true }); - widget.resultTreeWidget.model.selectPrevNode(); + if (this.tryGetWidget()?.hasResultList()) { + const widget = await this.openView({ activate: true }); + widget.resultTreeWidget.model.selectPrevNode(); + } } }); From 331ea08f20051e36ca58bc3c3bdcb7a43e0ffa2c Mon Sep 17 00:00:00 2001 From: Vlad Arama Date: Wed, 12 Jul 2023 14:44:41 -0400 Subject: [PATCH 04/10] Add `isEnabled` argument --- .../search-in-workspace-frontend-contribution.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts b/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts index 0be484a7f1060..c63134510cd36 100644 --- a/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts +++ b/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts @@ -180,20 +180,18 @@ export class SearchInWorkspaceFrontendContribution extends AbstractViewContribut }); commands.registerCommand(SearchInWorkspaceCommands.FOCUS_NEXT_RESULT, { + isEnabled: () => this.tryGetWidget()?.hasResultList() || false, execute: async () => { - if (this.tryGetWidget()?.hasResultList()) { - const widget = await this.openView({ activate: true }); - widget.resultTreeWidget.model.selectNextNode(); - } + const widget = await this.openView({ activate: true }); + widget.resultTreeWidget.model.selectNextNode(); } }); commands.registerCommand(SearchInWorkspaceCommands.FOCUS_PREV_RESULT, { + isEnabled: () => this.tryGetWidget()?.hasResultList() || false, execute: async () => { - if (this.tryGetWidget()?.hasResultList()) { - const widget = await this.openView({ activate: true }); - widget.resultTreeWidget.model.selectPrevNode(); - } + const widget = await this.openView({ activate: true }); + widget.resultTreeWidget.model.selectPrevNode(); } }); From f56658f4e513b9f8d76b9b676ce0b4e5f59218fd Mon Sep 17 00:00:00 2001 From: Vlad Arama Date: Mon, 17 Jul 2023 08:26:07 -0400 Subject: [PATCH 05/10] working next and previous search results --- ...arch-in-workspace-frontend-contribution.ts | 8 +-- ...search-in-workspace-result-tree-widget.tsx | 71 +++++++++++++++++++ 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts b/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts index c63134510cd36..7ece15920d3d0 100644 --- a/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts +++ b/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts @@ -180,18 +180,18 @@ export class SearchInWorkspaceFrontendContribution extends AbstractViewContribut }); commands.registerCommand(SearchInWorkspaceCommands.FOCUS_NEXT_RESULT, { - isEnabled: () => this.tryGetWidget()?.hasResultList() || false, + isEnabled: () => this.tryGetWidget()?.hasResultList() ?? false, execute: async () => { const widget = await this.openView({ activate: true }); - widget.resultTreeWidget.model.selectNextNode(); + widget.resultTreeWidget.selectNextResult(); } }); commands.registerCommand(SearchInWorkspaceCommands.FOCUS_PREV_RESULT, { - isEnabled: () => this.tryGetWidget()?.hasResultList() || false, + isEnabled: () => this.tryGetWidget()?.hasResultList() ?? false, execute: async () => { const widget = await this.openView({ activate: true }); - widget.resultTreeWidget.model.selectPrevNode(); + widget.resultTreeWidget.selectPreviousResult(); } }); diff --git a/packages/search-in-workspace/src/browser/search-in-workspace-result-tree-widget.tsx b/packages/search-in-workspace/src/browser/search-in-workspace-result-tree-widget.tsx index cf5fbb4fbc13e..346d2165afdc8 100644 --- a/packages/search-in-workspace/src/browser/search-in-workspace-result-tree-widget.tsx +++ b/packages/search-in-workspace/src/browser/search-in-workspace-result-tree-widget.tsx @@ -281,6 +281,54 @@ export class SearchInWorkspaceResultTreeWidget extends TreeWidget { return true; } + selectNextResult(): void { + if (!SearchInWorkspaceResultLineNode.is(this.model.getFocusedNode())) { + this.selectFirstResult(); + return; + } + let foundFocusedNode = false; + for (const rootFolderNode of this.resultTree.values()) { + for (const fileNode of rootFolderNode.children) { + for (const result of fileNode.children) { + if (foundFocusedNode) { + this.model.expandNode(fileNode); + this.model.selectNode(result); + return; + } + if (result.selected) { + foundFocusedNode = true; + } + } + } + } + this.selectFirstResult(); + } + + selectPreviousResult(): void { + if (!SearchInWorkspaceResultLineNode.is(this.model.getFocusedNode())) { + this.selectLastResult(); + return; + } + let foundFocusedNode = false; + for (const rootFolderNodes of this.resultTree.values()) { + for (let j = rootFolderNodes.children.length - 1; j >= 0; j--) { + const fileNode = rootFolderNodes.children[j]; + for (let k = fileNode.children.length - 1; k >= 0; k--) { + const result = fileNode.children[k]; + if (foundFocusedNode) { + this.model.expandNode(fileNode); + this.model.selectNode(result); + return; + } + if (result.selected) { + foundFocusedNode = true; + } + } + } + } + this.selectLastResult(); + } + /** * Find matches for the given editor. * @param searchTerm the search term. @@ -599,6 +647,29 @@ export class SearchInWorkspaceResultTreeWidget extends TreeWidget { } } + selectFirstResult(): void { + const rootFolder = this.resultTree.values(); + for (const files of rootFolder) { + const result = files.children[0].children[0]; + if (SelectableTreeNode.is(result)) { + this.model.expandNode(result.parent); + this.model.selectNode(result); + } + } + } + + selectLastResult(): void { + const rootFolder = this.resultTree.values(); + for (const files of rootFolder) { + const fileNode = files.children[files.children.length - 1]; + const result = fileNode.children[fileNode.children.length - 1]; + if (SelectableTreeNode.is(result)) { + this.model.expandNode(result.parent); + this.model.selectNode(result); + } + } + } + /** * Collapse the search-in-workspace file node * based on the preference value. From e9864e25b4a7f74bdab389115de66938afa9ae65 Mon Sep 17 00:00:00 2001 From: Vlad Arama Date: Wed, 19 Jul 2023 09:47:42 -0400 Subject: [PATCH 06/10] improve code logic by using iterators --- packages/core/src/browser/tree/tree-model.ts | 74 ++++++++++++ ...search-in-workspace-result-tree-widget.tsx | 110 ++++++++++-------- 2 files changed, 134 insertions(+), 50 deletions(-) diff --git a/packages/core/src/browser/tree/tree-model.ts b/packages/core/src/browser/tree/tree-model.ts index da808e5b2f0f7..621617e73aa33 100644 --- a/packages/core/src/browser/tree/tree-model.ts +++ b/packages/core/src/browser/tree/tree-model.ts @@ -99,21 +99,41 @@ export interface TreeModel extends Tree, TreeSelectionService, TreeExpansionServ */ navigateBackward(): Promise; + /** + * Selects the previous tree node, regardless of its selection or visibility state. + */ + selectPrev(): void; + /** * Selects the previous node relatively to the currently selected one. This method takes the expansion state of the tree into consideration. */ selectPrevNode(type?: TreeSelection.SelectionType): void; + /** + * Returns the previous tree node, regardless of its selection or visibility state. + */ + getPrevNode(node?: TreeNode): TreeNode | undefined; + /** * Returns the previous selectable tree node. */ getPrevSelectableNode(node?: TreeNode): SelectableTreeNode | undefined; + /** + * Selects the next tree node, regardless of its selection or visibility state. + */ + selectNext(): void; + /** * Selects the next node relatively to the currently selected one. This method takes the expansion state of the tree into consideration. */ selectNextNode(type?: TreeSelection.SelectionType): void; + /** + * Returns the next tree node, regardless of its selection or visibility state. + */ + getNextNode(node?: TreeNode): TreeNode | undefined; + /** * Returns the next selectable tree node. */ @@ -294,6 +314,11 @@ export class TreeModelImpl implements TreeModel, SelectionProvider(iterator: TreeIterator, criterion: (node: TreeNode) => node is T): T | undefined { // Skip the first item. // TODO: clean this up, and skip the first item in a different way without loading everything. iterator.next(); @@ -338,6 +390,17 @@ export class TreeModelImpl implements TreeModel, SelectionProvider= 0; j--) { - const fileNode = rootFolderNodes.children[j]; - for (let k = fileNode.children.length - 1; k >= 0; k--) { - const result = fileNode.children[k]; - if (foundFocusedNode) { - this.model.expandNode(fileNode); + let foundSelectedNode = false; + while (!foundSelectedNode) { + const prevNode = this.model.getPrevNode(); + if (!prevNode) { + this.selectLastResult(); + return; + } else if (SearchInWorkspaceResultLineNode.is(prevNode)) { + foundSelectedNode = true; + this.model.expandNode(prevNode.parent.parent); + this.model.expandNode(prevNode.parent); + this.model.selectNode(prevNode); + } else if (prevNode.id === 'ResultTree') { + this.selectLastResult(); + return; + } else { + this.model.selectPrev(); + } + } + } + + protected selectFirstResult(): void { + for (const rootFolder of this.resultTree.values()) { + for (const file of rootFolder.children) { + for (const result of file.children) { + if (SelectableTreeNode.is(result)) { + this.model.expandNode(result.parent.parent); + this.model.expandNode(result.parent); this.model.selectNode(result); return; } - if (result.selected) { - foundFocusedNode = true; + } + } + } + } + + protected selectLastResult(): void { + const rootFolders = Array.from(this.resultTree.values()); + for (let i = rootFolders.length - 1; i >= 0; i--) { + const rootFolder = rootFolders[i]; + for (let j = rootFolder.children.length - 1; j >= 0; j--) { + const file = rootFolder.children[j]; + for (let k = file.children.length - 1; k >= 0; k--) { + const result = file.children[k]; + if (SelectableTreeNode.is(result)) { + this.model.expandNode(result.parent.parent); + this.model.expandNode(result.parent); + this.model.selectNode(result); + return; } } } } - this.selectLastResult(); } /** @@ -647,29 +680,6 @@ export class SearchInWorkspaceResultTreeWidget extends TreeWidget { } } - selectFirstResult(): void { - const rootFolder = this.resultTree.values(); - for (const files of rootFolder) { - const result = files.children[0].children[0]; - if (SelectableTreeNode.is(result)) { - this.model.expandNode(result.parent); - this.model.selectNode(result); - } - } - } - - selectLastResult(): void { - const rootFolder = this.resultTree.values(); - for (const files of rootFolder) { - const fileNode = files.children[files.children.length - 1]; - const result = fileNode.children[fileNode.children.length - 1]; - if (SelectableTreeNode.is(result)) { - this.model.expandNode(result.parent); - this.model.selectNode(result); - } - } - } - /** * Collapse the search-in-workspace file node * based on the preference value. From 630cdd1176bfed10c0fe905b9063d43f6fe5f2c0 Mon Sep 17 00:00:00 2001 From: Vlad Arama Date: Wed, 2 Aug 2023 14:45:39 -0400 Subject: [PATCH 07/10] refactor code and focus editor --- ...arch-in-workspace-frontend-contribution.ts | 4 +- ...search-in-workspace-result-tree-widget.tsx | 40 ++++++++----------- 2 files changed, 18 insertions(+), 26 deletions(-) diff --git a/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts b/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts index 7ece15920d3d0..ef3d14cfa2dbc 100644 --- a/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts +++ b/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts @@ -182,7 +182,7 @@ export class SearchInWorkspaceFrontendContribution extends AbstractViewContribut commands.registerCommand(SearchInWorkspaceCommands.FOCUS_NEXT_RESULT, { isEnabled: () => this.tryGetWidget()?.hasResultList() ?? false, execute: async () => { - const widget = await this.openView({ activate: true }); + const widget = await this.openView({ reveal: true }); widget.resultTreeWidget.selectNextResult(); } }); @@ -190,7 +190,7 @@ export class SearchInWorkspaceFrontendContribution extends AbstractViewContribut commands.registerCommand(SearchInWorkspaceCommands.FOCUS_PREV_RESULT, { isEnabled: () => this.tryGetWidget()?.hasResultList() ?? false, execute: async () => { - const widget = await this.openView({ activate: true }); + const widget = await this.openView({ reveal: true }); widget.resultTreeWidget.selectPreviousResult(); } }); diff --git a/packages/search-in-workspace/src/browser/search-in-workspace-result-tree-widget.tsx b/packages/search-in-workspace/src/browser/search-in-workspace-result-tree-widget.tsx index b41d2a2087d3b..a89262dbbdcb5 100644 --- a/packages/search-in-workspace/src/browser/search-in-workspace-result-tree-widget.tsx +++ b/packages/search-in-workspace/src/browser/search-in-workspace-result-tree-widget.tsx @@ -283,20 +283,16 @@ export class SearchInWorkspaceResultTreeWidget extends TreeWidget { selectNextResult(): void { if (!this.model.getFocusedNode()) { - this.selectFirstResult(); - return; + return this.selectFirstResult(); } let foundNextResult = false; while (!foundNextResult) { const nextNode = this.model.getNextNode(); if (!nextNode) { - this.selectFirstResult(); - return; + return this.selectFirstResult(); } else if (SearchInWorkspaceResultLineNode.is(nextNode)) { foundNextResult = true; - this.model.expandNode(nextNode.parent.parent); - this.model.expandNode(nextNode.parent); - this.model.selectNode(nextNode); + this.selectExpandOpenResultNode(nextNode); } else { this.model.selectNext(); } @@ -305,38 +301,37 @@ export class SearchInWorkspaceResultTreeWidget extends TreeWidget { selectPreviousResult(): void { if (!this.model.getFocusedNode()) { - this.selectLastResult(); - return; + return this.selectLastResult(); } let foundSelectedNode = false; while (!foundSelectedNode) { const prevNode = this.model.getPrevNode(); if (!prevNode) { - this.selectLastResult(); - return; + return this.selectLastResult(); } else if (SearchInWorkspaceResultLineNode.is(prevNode)) { foundSelectedNode = true; - this.model.expandNode(prevNode.parent.parent); - this.model.expandNode(prevNode.parent); - this.model.selectNode(prevNode); + this.selectExpandOpenResultNode(prevNode); } else if (prevNode.id === 'ResultTree') { - this.selectLastResult(); - return; + return this.selectLastResult(); } else { this.model.selectPrev(); } } } + protected selectExpandOpenResultNode(node: SearchInWorkspaceResultLineNode): void { + this.model.expandNode(node.parent.parent); + this.model.expandNode(node.parent); + this.model.selectNode(node); + this.model.openNode(node); + } + protected selectFirstResult(): void { for (const rootFolder of this.resultTree.values()) { for (const file of rootFolder.children) { for (const result of file.children) { if (SelectableTreeNode.is(result)) { - this.model.expandNode(result.parent.parent); - this.model.expandNode(result.parent); - this.model.selectNode(result); - return; + return this.selectExpandOpenResultNode(result); } } } @@ -352,10 +347,7 @@ export class SearchInWorkspaceResultTreeWidget extends TreeWidget { for (let k = file.children.length - 1; k >= 0; k--) { const result = file.children[k]; if (SelectableTreeNode.is(result)) { - this.model.expandNode(result.parent.parent); - this.model.expandNode(result.parent); - this.model.selectNode(result); - return; + return this.selectExpandOpenResultNode(result); } } } From 3367c5ff59c78b0ce07a648cb3ce299762313990 Mon Sep 17 00:00:00 2001 From: Vlad Arama Date: Tue, 8 Aug 2023 08:52:39 -0400 Subject: [PATCH 08/10] fix review comments - Simplify the `isEnabled` command. - Add `when` context to the keybinding. --- .../browser/search-in-workspace-frontend-contribution.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts b/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts index ef3d14cfa2dbc..19f041953eb22 100644 --- a/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts +++ b/packages/search-in-workspace/src/browser/search-in-workspace-frontend-contribution.ts @@ -180,7 +180,7 @@ export class SearchInWorkspaceFrontendContribution extends AbstractViewContribut }); commands.registerCommand(SearchInWorkspaceCommands.FOCUS_NEXT_RESULT, { - isEnabled: () => this.tryGetWidget()?.hasResultList() ?? false, + isEnabled: () => this.withWidget(undefined, widget => widget.hasResultList()), execute: async () => { const widget = await this.openView({ reveal: true }); widget.resultTreeWidget.selectNextResult(); @@ -188,7 +188,7 @@ export class SearchInWorkspaceFrontendContribution extends AbstractViewContribut }); commands.registerCommand(SearchInWorkspaceCommands.FOCUS_PREV_RESULT, { - isEnabled: () => this.tryGetWidget()?.hasResultList() ?? false, + isEnabled: () => this.withWidget(undefined, widget => widget.hasResultList()), execute: async () => { const widget = await this.openView({ reveal: true }); widget.resultTreeWidget.selectPreviousResult(); @@ -372,10 +372,12 @@ export class SearchInWorkspaceFrontendContribution extends AbstractViewContribut keybindings.registerKeybinding({ command: SearchInWorkspaceCommands.FOCUS_NEXT_RESULT.id, keybinding: 'f4', + when: 'hasSearchResult' }); keybindings.registerKeybinding({ command: SearchInWorkspaceCommands.FOCUS_PREV_RESULT.id, keybinding: 'shift+f4', + when: 'hasSearchResult' }); keybindings.registerKeybinding({ command: SearchInWorkspaceCommands.DISMISS_RESULT.id, From bdec8b134b46475469bf5c1646b75aa85684b4b8 Mon Sep 17 00:00:00 2001 From: Vlad Arama Date: Thu, 10 Aug 2023 09:07:15 -0400 Subject: [PATCH 09/10] add unit tests to the selectable trees - Added a new mock tree model that contains selectable nodes Unit tests cover: - `getNextNode`, `getPrevNode` - `getNextSelectableNode`, `getPrevSelectableNode` - `selectNext`, `selectPrev` - `selectNextNode`, `selectPrevNode` --- .../tree/test/mock-selectable-tree-model.ts | 109 ++++++++ .../src/browser/tree/tree-selectable.spec.ts | 242 ++++++++++++++++++ 2 files changed, 351 insertions(+) create mode 100644 packages/core/src/browser/tree/test/mock-selectable-tree-model.ts create mode 100644 packages/core/src/browser/tree/tree-selectable.spec.ts diff --git a/packages/core/src/browser/tree/test/mock-selectable-tree-model.ts b/packages/core/src/browser/tree/test/mock-selectable-tree-model.ts new file mode 100644 index 0000000000000..7a6c263c14096 --- /dev/null +++ b/packages/core/src/browser/tree/test/mock-selectable-tree-model.ts @@ -0,0 +1,109 @@ +// ***************************************************************************** +// Copyright (C) 2023 Ericsson and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// http://www.eclipse.org/legal/epl-2.0. +// +// This Source Code may also be made available under the following Secondary +// Licenses when the conditions for such availability set forth in the Eclipse +// Public License v. 2.0 are satisfied: GNU General Public License, version 2 +// with the GNU Classpath Exception which is available at +// https://www.gnu.org/software/classpath/license.html. +// +// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0 +// ***************************************************************************** + +import { CompositeTreeNode } from '../tree'; +import { SelectableTreeNode } from '../tree-selection'; +import { ExpandableTreeNode } from '../tree-expansion'; + +export namespace MockSelectableTreeModel { + + export interface SelectableNode { + readonly id: string; + readonly selected: boolean; + readonly focused?: boolean; + readonly children?: SelectableNode[]; + } + + export namespace SelectableNode { + export function toTreeNode(root: SelectableNode, parent?: SelectableTreeNode & CompositeTreeNode): SelectableTreeNode { + const { id } = root; + const name = id; + const selected = false; + const focus = false; + const expanded = true; + const node: CompositeTreeNode & SelectableTreeNode = { + id, + name, + selected, + focus, + parent: parent, + children: [] + }; + const children = (root.children || []).map(child => SelectableNode.toTreeNode(child, node)); + if (children.length === 0) { + return node; + } else { + node.children = children; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (node as any).expanded = expanded; + return node as CompositeTreeNode & SelectableTreeNode & ExpandableTreeNode; + } + } + } + + export const HIERARCHICAL_MOCK_ROOT = () => SelectableNode.toTreeNode({ + 'id': '1', + 'selected': false, + 'children': [ + { + 'id': '1.1', + 'selected': false, + 'children': [ + { + 'id': '1.1.1', + 'selected': false, + }, + { + 'id': '1.1.2', + 'selected': false, + } + ] + }, + { + 'id': '1.2', + 'selected': false, + 'children': [ + { + 'id': '1.2.1', + 'selected': false, + 'children': [ + { + 'id': '1.2.1.1', + 'selected': false, + }, + { + 'id': '1.2.1.2', + 'selected': false, + } + ] + }, + { + 'id': '1.2.2', + 'selected': false, + }, + { + 'id': '1.2.3', + 'selected': false, + } + ] + }, + { + 'id': '1.3', + 'selected': false, + } + ] + }); +} diff --git a/packages/core/src/browser/tree/tree-selectable.spec.ts b/packages/core/src/browser/tree/tree-selectable.spec.ts new file mode 100644 index 0000000000000..5a9dc9d3aa384 --- /dev/null +++ b/packages/core/src/browser/tree/tree-selectable.spec.ts @@ -0,0 +1,242 @@ +// ***************************************************************************** +// Copyright (C) 2018 TypeFox and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// http://www.eclipse.org/legal/epl-2.0. +// +// This Source Code may also be made available under the following Secondary +// Licenses when the conditions for such availability set forth in the Eclipse +// Public License v. 2.0 are satisfied: GNU General Public License, version 2 +// with the GNU Classpath Exception which is available at +// https://www.gnu.org/software/classpath/license.html. +// +// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0 +// ***************************************************************************** + +import { TreeNode } from './tree'; +import { TreeModel } from './tree-model'; +import { notEmpty } from '../../common/objects'; +import { expect } from 'chai'; +import { createTreeTestContainer } from './test/tree-test-container'; +import { SelectableTreeNode } from './tree-selection'; +import { MockSelectableTreeModel } from './test/mock-selectable-tree-model'; +import { ExpandableTreeNode } from './tree-expansion'; + +describe('Selectable Tree', () => { + let model: TreeModel; + beforeEach(() => { + model = createTreeModel(); + model.root = MockSelectableTreeModel.HIERARCHICAL_MOCK_ROOT(); + }); + describe('Get Nodes Methods', () => { + it('`getNextNode()` should select each node in sequence (uncollapsed)', done => { + const expectedSelectionOrder = ['1.1', '1.1.1', '1.1.2', '1.2', '1.2.1', '1.2.1.1', '1.2.1.2', '1.2.2', '1.2.3', '1.3']; + const rootNode = retrieveNode('1'); + model.addSelection(rootNode); + for (const expectedNodeId of expectedSelectionOrder) { + const actualNode = model.getNextNode(); + const expectedNode = retrieveNode(expectedNodeId); + expect(actualNode?.id).to.be.equal(expectedNode.id); + model.addSelection(expectedNode); + } + done(); + }); + it('`getNextNode()` should select each node in sequence (collapsed)', done => { + collapseNode('1.1', '1.2.1'); + const expectedSelectionOrder = ['1.1', '1.1.1', '1.1.2', '1.2', '1.2.1', '1.2.1.1', '1.2.1.2', '1.2.2', '1.2.3', '1.3']; + const rootNode = retrieveNode('1'); + model.addSelection(rootNode); + for (const expectedNodeId of expectedSelectionOrder) { + const actualNode = model.getNextNode(); + const expectedNode = retrieveNode(expectedNodeId); + expect(actualNode?.id).to.be.equal(expectedNode.id); + model.addSelection(expectedNode); + } + done(); + }); + it('`getNextSelectableNode()` should select each node in sequence (uncollapsed)', done => { + const expectedSelectionOrder = ['1.1', '1.1.1', '1.1.2', '1.2', '1.2.1', '1.2.1.1', '1.2.1.2', '1.2.2', '1.2.3', '1.3']; + const rootNode = retrieveNode('1'); + model.addSelection(rootNode); + for (const expectedNodeId of expectedSelectionOrder) { + const actualNode = model.getNextSelectableNode(); + const expectedNode = retrieveNode(expectedNodeId); + expect(actualNode?.id).to.be.equal(expectedNode.id); + model.addSelection(expectedNode); + } + done(); + }); + it('`getNextSelectableNode()` should select each node in sequence (collapsed)', done => { + collapseNode('1.1', '1.2.1'); + const expectedSelectionOrder = ['1.1', '1.2', '1.2.1', '1.2.2', '1.2.3', '1.3']; + const rootNode = retrieveNode('1'); + model.addSelection(rootNode); + for (const expectedNodeId of expectedSelectionOrder) { + const actualNode = model.getNextSelectableNode(); + const expectedNode = retrieveNode(expectedNodeId); + expect(actualNode?.id).to.be.equal(expectedNode.id); + model.addSelection(expectedNode); + } + done(); + }); + it('`getPrevNode()` should select each node in reverse sequence (uncollapsed)', done => { + const expectedSelectionOrder = ['1.2.3', '1.2.2', '1.2.1.2', '1.2.1.1', '1.2.1', '1.2', '1.1.2', '1.1.1', '1.1']; + const rootNode = retrieveNode('1.3'); + model.addSelection(rootNode); + for (const expectedNodeId of expectedSelectionOrder) { + const actualNode = model.getPrevNode(); + const expectedNode = retrieveNode(expectedNodeId); + expect(actualNode?.id).to.be.equal(expectedNode.id); + model.addSelection(expectedNode); + } + done(); + }); + it('`getPrevNode()` should select each node in reverse sequence (collapsed)', done => { + collapseNode('1.1', '1.2.1'); + const expectedSelectionOrder = ['1.2.3', '1.2.2', '1.2.1.2', '1.2.1.1', '1.2.1', '1.2', '1.1.2', '1.1.1', '1.1']; + const rootNode = retrieveNode('1.3'); + model.addSelection(rootNode); + for (const expectedNodeId of expectedSelectionOrder) { + const actualNode = model.getPrevNode(); + const expectedNode = retrieveNode(expectedNodeId); + expect(actualNode?.id).to.be.equal(expectedNode.id); + model.addSelection(expectedNode); + } + done(); + }); + it('`getPrevSelectableNode()` should select each node in reverse sequence (uncollapsed)', done => { + const expectedSelectionOrder = ['1.2.3', '1.2.2', '1.2.1.2', '1.2.1.1', '1.2.1', '1.2', '1.1.2', '1.1.1', '1.1']; + const rootNode = retrieveNode('1.3'); + model.addSelection(rootNode); + for (const expectedNodeId of expectedSelectionOrder) { + const actualNode = model.getPrevSelectableNode(); + const expectedNode = retrieveNode(expectedNodeId); + expect(actualNode?.id).to.be.equal(expectedNode.id); + model.addSelection(expectedNode); + } + done(); + }); + it('`getPrevSelectableNode()` should select each node in reverse sequence (collapsed)', done => { + collapseNode('1.1', '1.2.1'); + const expectedSelectionOrder = ['1.2.3', '1.2.2', '1.2.1', '1.2', '1.1']; + const rootNode = retrieveNode('1.3'); + model.addSelection(rootNode); + for (const expectedNodeId of expectedSelectionOrder) { + const actualNode = model.getPrevSelectableNode(); + const expectedNode = retrieveNode(expectedNodeId); + expect(actualNode?.id).to.be.equal(expectedNode.id); + model.addSelection(expectedNode); + } + done(); + }); + }); + + describe('Selection Methods', () => { + it('`selectNext()` should select each node in sequence (uncollapsed)', done => { + const expectedSelectionOrder = ['1.1', '1.1.1', '1.1.2', '1.2', '1.2.1', '1.2.1.1', '1.2.1.2', '1.2.2', '1.2.3', '1.3']; + const rootNode = retrieveNode('1'); + model.addSelection(rootNode); + for (const expectedNodeId of expectedSelectionOrder) { + model.selectNext(); + const node = retrieveNode(expectedNodeId); + expect(node.selected).to.be.true; + } + done(); + }); + it('`selectNext()` should select each node in sequence (collapsed)', done => { + collapseNode('1.1', '1.2.1'); + const expectedSelectionOrder = ['1.1', '1.1.1', '1.1.2', '1.2', '1.2.1', '1.2.1.1', '1.2.1.2', '1.2.2', '1.2.3', '1.3']; + const rootNode = retrieveNode('1'); + model.addSelection(rootNode); + for (const expectedNodeId of expectedSelectionOrder) { + model.selectNext(); + const node = retrieveNode(expectedNodeId); + expect(node.selected).to.be.true; + } + done(); + }); + it('`selectNextNode()` should select each node in sequence (uncollapsed)', done => { + const expectedSelectionOrder = ['1.1', '1.1.1', '1.1.2', '1.2', '1.2.1', '1.2.1.1', '1.2.1.2', '1.2.2', '1.2.3', '1.3']; + for (const expectedNodeId of expectedSelectionOrder) { + model.selectNextNode(); + const node = retrieveNode(expectedNodeId); + expect(node.selected).to.be.true; + } + done(); + }); + it('`selectNextNode()` should select each node in sequence (collapsed)', done => { + collapseNode('1.1', '1.2.1'); + const expectedSelectionOrder = ['1.1', '1.2', '1.2.1', '1.2.2', '1.2.3', '1.3']; + for (const expectedNodeId of expectedSelectionOrder) { + model.selectNextNode(); + const node = retrieveNode(expectedNodeId); + expect(node.selected).to.be.true; + } + done(); + }); + it('`selectPrev()` should select each node in reverse sequence (uncollapsed)', done => { + const expectedSelectionOrder = ['1.2.3', '1.2.2', '1.2.1.2', '1.2.1.1', '1.2.1', '1.2', '1.1.2', '1.1.1', '1.1']; + const rootNode = retrieveNode('1.3'); + model.addSelection(rootNode); + for (const expectedNodeId of expectedSelectionOrder) { + model.selectPrev(); + const node = retrieveNode(expectedNodeId); + expect(node.selected).to.be.true; + } + done(); + }); + it('`selectPrev()` should select each node in reverse sequence (collapsed)', done => { + collapseNode('1.1', '1.2.1'); + const expectedSelectionOrder = ['1.2.3', '1.2.2', '1.2.1.2', '1.2.1.1', '1.2.1', '1.2', '1.1.2', '1.1.1', '1.1']; + const rootNode = retrieveNode('1.3'); + model.addSelection(rootNode); + for (const expectedNodeId of expectedSelectionOrder) { + model.selectPrev(); + const node = retrieveNode(expectedNodeId); + expect(node.selected).to.be.true; + } + done(); + }); + it('`selectPrevNode()` should select each node in reverse sequence (uncollapsed)', done => { + const expectedSelectionOrder = ['1.2.3', '1.2.2', '1.2.1.2', '1.2.1.1', '1.2.1', '1.2', '1.1.2', '1.1.1', '1.1']; + const rootNode = retrieveNode('1.3'); + model.addSelection(rootNode); + for (const expectedNodeId of expectedSelectionOrder) { + model.selectPrevNode(); + const node = retrieveNode(expectedNodeId); + expect(node.selected).to.be.true; + } + done(); + }); + it('`selectPrevNode()` should select each node in reverse sequence (collapsed)', done => { + collapseNode('1.1', '1.2.1'); + const expectedSelectionOrder = ['1.2.3', '1.2.2', '1.2.1', '1.2', '1.1']; + const rootNode = retrieveNode('1.3'); + model.addSelection(rootNode); + for (const expectedNodeId of expectedSelectionOrder) { + model.selectPrevNode(); + const node = retrieveNode(expectedNodeId); + expect(node.selected).to.be.true; + } + done(); + }); + }); + + const findNode = (id: string) => model.getNode(id); + function createTreeModel(): TreeModel { + const container = createTreeTestContainer(); + return container.get(TreeModel); + } + function retrieveNode(id: string): Readonly { + const readonlyNode: Readonly = model.getNode(id) as T; + return readonlyNode; + } + function collapseNode(...ids: string[]): void { + ids.map(findNode).filter(notEmpty).filter(ExpandableTreeNode.is).forEach(node => { + model.collapseNode(node); + expect(node).to.have.property('expanded', false); + }); + } + +}); From 10728e9dfd383d14e1863c8199e81fa39e315ca5 Mon Sep 17 00:00:00 2001 From: Vlad Arama Date: Thu, 10 Aug 2023 14:59:47 -0400 Subject: [PATCH 10/10] refactor and simplify unit tests This commit simplifies the code by significantly reducing the repetition in the tests. --- .../src/browser/tree/tree-selectable.spec.ts | 202 +++++------------- 1 file changed, 56 insertions(+), 146 deletions(-) diff --git a/packages/core/src/browser/tree/tree-selectable.spec.ts b/packages/core/src/browser/tree/tree-selectable.spec.ts index 5a9dc9d3aa384..b8cc16d22a8a7 100644 --- a/packages/core/src/browser/tree/tree-selectable.spec.ts +++ b/packages/core/src/browser/tree/tree-selectable.spec.ts @@ -25,200 +25,110 @@ import { ExpandableTreeNode } from './tree-expansion'; describe('Selectable Tree', () => { let model: TreeModel; - beforeEach(() => { - model = createTreeModel(); - model.root = MockSelectableTreeModel.HIERARCHICAL_MOCK_ROOT(); - }); - describe('Get Nodes Methods', () => { + function assertNodeRetrieval(method: () => TreeNode | undefined, sequence: string[]): void { + for (const expectedNodeId of sequence) { + const actualNode = method(); + const expectedNode = retrieveNode(expectedNodeId); + expect(actualNode?.id).to.be.equal(expectedNode.id); + model.addSelection(expectedNode); + } + } + function assertNodeSelection(method: () => void, sequence: string[]): void { + for (const expectedNodeId of sequence) { + method(); + const node = retrieveNode(expectedNodeId); + expect(node.selected).to.be.true; + } + } + describe('Get and Set Next Nodes Methods', () => { + const uncollapsedSelectionOrder = ['1.1', '1.1.1', '1.1.2', '1.2', '1.2.1', '1.2.1.1', '1.2.1.2', '1.2.2', '1.2.3', '1.3']; + const collapsedSelectionOrder = ['1.1', '1.2', '1.2.1', '1.2.2', '1.2.3', '1.3']; + beforeEach(() => { + model = createTreeModel(); + model.root = MockSelectableTreeModel.HIERARCHICAL_MOCK_ROOT(); + model.addSelection(retrieveNode('1')); + + }); it('`getNextNode()` should select each node in sequence (uncollapsed)', done => { - const expectedSelectionOrder = ['1.1', '1.1.1', '1.1.2', '1.2', '1.2.1', '1.2.1.1', '1.2.1.2', '1.2.2', '1.2.3', '1.3']; - const rootNode = retrieveNode('1'); - model.addSelection(rootNode); - for (const expectedNodeId of expectedSelectionOrder) { - const actualNode = model.getNextNode(); - const expectedNode = retrieveNode(expectedNodeId); - expect(actualNode?.id).to.be.equal(expectedNode.id); - model.addSelection(expectedNode); - } + assertNodeRetrieval(model.getNextNode.bind(model), uncollapsedSelectionOrder); done(); }); it('`getNextNode()` should select each node in sequence (collapsed)', done => { collapseNode('1.1', '1.2.1'); - const expectedSelectionOrder = ['1.1', '1.1.1', '1.1.2', '1.2', '1.2.1', '1.2.1.1', '1.2.1.2', '1.2.2', '1.2.3', '1.3']; - const rootNode = retrieveNode('1'); - model.addSelection(rootNode); - for (const expectedNodeId of expectedSelectionOrder) { - const actualNode = model.getNextNode(); - const expectedNode = retrieveNode(expectedNodeId); - expect(actualNode?.id).to.be.equal(expectedNode.id); - model.addSelection(expectedNode); - } + assertNodeRetrieval(model.getNextNode.bind(model), uncollapsedSelectionOrder); done(); }); it('`getNextSelectableNode()` should select each node in sequence (uncollapsed)', done => { - const expectedSelectionOrder = ['1.1', '1.1.1', '1.1.2', '1.2', '1.2.1', '1.2.1.1', '1.2.1.2', '1.2.2', '1.2.3', '1.3']; - const rootNode = retrieveNode('1'); - model.addSelection(rootNode); - for (const expectedNodeId of expectedSelectionOrder) { - const actualNode = model.getNextSelectableNode(); - const expectedNode = retrieveNode(expectedNodeId); - expect(actualNode?.id).to.be.equal(expectedNode.id); - model.addSelection(expectedNode); - } + assertNodeRetrieval(model.getNextSelectableNode.bind(model), uncollapsedSelectionOrder); done(); }); it('`getNextSelectableNode()` should select each node in sequence (collapsed)', done => { collapseNode('1.1', '1.2.1'); - const expectedSelectionOrder = ['1.1', '1.2', '1.2.1', '1.2.2', '1.2.3', '1.3']; - const rootNode = retrieveNode('1'); - model.addSelection(rootNode); - for (const expectedNodeId of expectedSelectionOrder) { - const actualNode = model.getNextSelectableNode(); - const expectedNode = retrieveNode(expectedNodeId); - expect(actualNode?.id).to.be.equal(expectedNode.id); - model.addSelection(expectedNode); - } + assertNodeRetrieval(model.getNextSelectableNode.bind(model), collapsedSelectionOrder); done(); }); - it('`getPrevNode()` should select each node in reverse sequence (uncollapsed)', done => { - const expectedSelectionOrder = ['1.2.3', '1.2.2', '1.2.1.2', '1.2.1.1', '1.2.1', '1.2', '1.1.2', '1.1.1', '1.1']; - const rootNode = retrieveNode('1.3'); - model.addSelection(rootNode); - for (const expectedNodeId of expectedSelectionOrder) { - const actualNode = model.getPrevNode(); - const expectedNode = retrieveNode(expectedNodeId); - expect(actualNode?.id).to.be.equal(expectedNode.id); - model.addSelection(expectedNode); - } + it('`selectNext()` should select each node in sequence (uncollapsed)', done => { + assertNodeSelection(model.selectNext.bind(model), uncollapsedSelectionOrder); done(); }); - it('`getPrevNode()` should select each node in reverse sequence (collapsed)', done => { + it('`selectNext()` should select each node in sequence (collapsed)', done => { collapseNode('1.1', '1.2.1'); - const expectedSelectionOrder = ['1.2.3', '1.2.2', '1.2.1.2', '1.2.1.1', '1.2.1', '1.2', '1.1.2', '1.1.1', '1.1']; - const rootNode = retrieveNode('1.3'); - model.addSelection(rootNode); - for (const expectedNodeId of expectedSelectionOrder) { - const actualNode = model.getPrevNode(); - const expectedNode = retrieveNode(expectedNodeId); - expect(actualNode?.id).to.be.equal(expectedNode.id); - model.addSelection(expectedNode); - } + assertNodeSelection(model.selectNext.bind(model), uncollapsedSelectionOrder); done(); }); - it('`getPrevSelectableNode()` should select each node in reverse sequence (uncollapsed)', done => { - const expectedSelectionOrder = ['1.2.3', '1.2.2', '1.2.1.2', '1.2.1.1', '1.2.1', '1.2', '1.1.2', '1.1.1', '1.1']; - const rootNode = retrieveNode('1.3'); - model.addSelection(rootNode); - for (const expectedNodeId of expectedSelectionOrder) { - const actualNode = model.getPrevSelectableNode(); - const expectedNode = retrieveNode(expectedNodeId); - expect(actualNode?.id).to.be.equal(expectedNode.id); - model.addSelection(expectedNode); - } + it('`selectNextNode()` should select each node in sequence (uncollapsed)', done => { + assertNodeSelection(model.selectNextNode.bind(model), uncollapsedSelectionOrder); done(); }); - it('`getPrevSelectableNode()` should select each node in reverse sequence (collapsed)', done => { + it('`selectNextNode()` should select each node in sequence (collapsed)', done => { collapseNode('1.1', '1.2.1'); - const expectedSelectionOrder = ['1.2.3', '1.2.2', '1.2.1', '1.2', '1.1']; - const rootNode = retrieveNode('1.3'); - model.addSelection(rootNode); - for (const expectedNodeId of expectedSelectionOrder) { - const actualNode = model.getPrevSelectableNode(); - const expectedNode = retrieveNode(expectedNodeId); - expect(actualNode?.id).to.be.equal(expectedNode.id); - model.addSelection(expectedNode); - } + assertNodeSelection(model.selectNextNode.bind(model), collapsedSelectionOrder); done(); }); }); - describe('Selection Methods', () => { - it('`selectNext()` should select each node in sequence (uncollapsed)', done => { - const expectedSelectionOrder = ['1.1', '1.1.1', '1.1.2', '1.2', '1.2.1', '1.2.1.1', '1.2.1.2', '1.2.2', '1.2.3', '1.3']; - const rootNode = retrieveNode('1'); - model.addSelection(rootNode); - for (const expectedNodeId of expectedSelectionOrder) { - model.selectNext(); - const node = retrieveNode(expectedNodeId); - expect(node.selected).to.be.true; - } + describe('Get and Set Previous Nodes Methods', () => { + const uncollapsedSelectionOrder = ['1.2.3', '1.2.2', '1.2.1.2', '1.2.1.1', '1.2.1', '1.2', '1.1.2', '1.1.1', '1.1']; + const collapsedSelectionOrder = ['1.2.3', '1.2.2', '1.2.1', '1.2', '1.1']; + beforeEach(() => { + model = createTreeModel(); + model.root = MockSelectableTreeModel.HIERARCHICAL_MOCK_ROOT(); + model.addSelection(retrieveNode('1.3')); + }); + it('`getPrevNode()` should select each node in reverse sequence (uncollapsed)', done => { + assertNodeRetrieval(model.getPrevNode.bind(model), uncollapsedSelectionOrder); done(); }); - it('`selectNext()` should select each node in sequence (collapsed)', done => { + it('`getPrevNode()` should select each node in reverse sequence (collapsed)', done => { collapseNode('1.1', '1.2.1'); - const expectedSelectionOrder = ['1.1', '1.1.1', '1.1.2', '1.2', '1.2.1', '1.2.1.1', '1.2.1.2', '1.2.2', '1.2.3', '1.3']; - const rootNode = retrieveNode('1'); - model.addSelection(rootNode); - for (const expectedNodeId of expectedSelectionOrder) { - model.selectNext(); - const node = retrieveNode(expectedNodeId); - expect(node.selected).to.be.true; - } + assertNodeRetrieval(model.getPrevNode.bind(model), uncollapsedSelectionOrder); done(); }); - it('`selectNextNode()` should select each node in sequence (uncollapsed)', done => { - const expectedSelectionOrder = ['1.1', '1.1.1', '1.1.2', '1.2', '1.2.1', '1.2.1.1', '1.2.1.2', '1.2.2', '1.2.3', '1.3']; - for (const expectedNodeId of expectedSelectionOrder) { - model.selectNextNode(); - const node = retrieveNode(expectedNodeId); - expect(node.selected).to.be.true; - } + it('`getPrevSelectableNode()` should select each node in reverse sequence (uncollapsed)', done => { + assertNodeRetrieval(model.getPrevSelectableNode.bind(model), uncollapsedSelectionOrder); done(); }); - it('`selectNextNode()` should select each node in sequence (collapsed)', done => { + it('`getPrevSelectableNode()` should select each node in reverse sequence (collapsed)', done => { collapseNode('1.1', '1.2.1'); - const expectedSelectionOrder = ['1.1', '1.2', '1.2.1', '1.2.2', '1.2.3', '1.3']; - for (const expectedNodeId of expectedSelectionOrder) { - model.selectNextNode(); - const node = retrieveNode(expectedNodeId); - expect(node.selected).to.be.true; - } + assertNodeRetrieval(model.getPrevSelectableNode.bind(model), collapsedSelectionOrder); done(); }); it('`selectPrev()` should select each node in reverse sequence (uncollapsed)', done => { - const expectedSelectionOrder = ['1.2.3', '1.2.2', '1.2.1.2', '1.2.1.1', '1.2.1', '1.2', '1.1.2', '1.1.1', '1.1']; - const rootNode = retrieveNode('1.3'); - model.addSelection(rootNode); - for (const expectedNodeId of expectedSelectionOrder) { - model.selectPrev(); - const node = retrieveNode(expectedNodeId); - expect(node.selected).to.be.true; - } + assertNodeSelection(model.selectPrev.bind(model), uncollapsedSelectionOrder); done(); }); it('`selectPrev()` should select each node in reverse sequence (collapsed)', done => { collapseNode('1.1', '1.2.1'); - const expectedSelectionOrder = ['1.2.3', '1.2.2', '1.2.1.2', '1.2.1.1', '1.2.1', '1.2', '1.1.2', '1.1.1', '1.1']; - const rootNode = retrieveNode('1.3'); - model.addSelection(rootNode); - for (const expectedNodeId of expectedSelectionOrder) { - model.selectPrev(); - const node = retrieveNode(expectedNodeId); - expect(node.selected).to.be.true; - } + assertNodeSelection(model.selectPrev.bind(model), uncollapsedSelectionOrder); done(); }); it('`selectPrevNode()` should select each node in reverse sequence (uncollapsed)', done => { - const expectedSelectionOrder = ['1.2.3', '1.2.2', '1.2.1.2', '1.2.1.1', '1.2.1', '1.2', '1.1.2', '1.1.1', '1.1']; - const rootNode = retrieveNode('1.3'); - model.addSelection(rootNode); - for (const expectedNodeId of expectedSelectionOrder) { - model.selectPrevNode(); - const node = retrieveNode(expectedNodeId); - expect(node.selected).to.be.true; - } + assertNodeSelection(model.selectPrevNode.bind(model), uncollapsedSelectionOrder); done(); }); it('`selectPrevNode()` should select each node in reverse sequence (collapsed)', done => { collapseNode('1.1', '1.2.1'); - const expectedSelectionOrder = ['1.2.3', '1.2.2', '1.2.1', '1.2', '1.1']; - const rootNode = retrieveNode('1.3'); - model.addSelection(rootNode); - for (const expectedNodeId of expectedSelectionOrder) { - model.selectPrevNode(); - const node = retrieveNode(expectedNodeId); - expect(node.selected).to.be.true; - } + assertNodeSelection(model.selectPrevNode.bind(model), collapsedSelectionOrder); done(); }); });