Add loading spinner for connection tree element refresh (#20621)

* added loading status to tree elements during refresh

* added message

* added lambda function

* added async

* moved addtraits to below null check

* Added debug lines for now

* added wip stack

* moved loading icon logic to inside tree item code

* added dispose logic

* Added comment explaining actions

* removed reference to NodeJs Timeout

* added showLoading status to Tree creation utils

* fixed spaces

* removed unnecessary space

* removed unnecessary timer for more responsive results

* removed call in connectionTreeActions test

* added fix to test

* stick to refresh only

* added 100 ms timeout

* added request comment

src/sql/base/parts/tree/browser/treeModel.ts

@@ -959,7 +959,7 @@ export class TreeModel {
 		return this.input ? this.input.getElement() : null;
 	}
 
-	public refresh(element: any = null, recursive: boolean = true): Promise<any> {
+	public async refresh(element: any = null, recursive: boolean = true): Promise<any> {
 		let item = this.getItem(element);
 
 		if (!item) {
@@ -968,9 +968,8 @@ export class TreeModel {
 
 		let eventData: IRefreshEvent = { item: item, recursive: recursive };
 		this._onRefresh.fire(eventData);
-		return item.refresh(recursive).then(() => {
+		await item.refresh(recursive);
-			this._onDidRefresh.fire(eventData);
+		this._onDidRefresh.fire(eventData);
-		});
 	}
 
 	public expand(element: any): Promise<any> {

src/sql/base/parts/tree/browser/treeView.ts

@@ -392,7 +392,13 @@ function reactionEquals(one: _.IDragOverReaction, other: _.IDragOverReaction | n
 export class TreeView extends HeightMap {
 
 	static readonly BINDING = 'monaco-tree-row';
-	static readonly LOADING_DECORATION_DELAY = 800;
+
+	/**
+	 * The delay here is used here to display the loading status after a certain number of milliseconds.
+	 * It was changed from 800 as it turns out that refreshes happen instantly before the status
+	 * can be notified to the user. With 100 ms, the refresh indicator will now be visible in most instances.
+	 * */
+	static readonly LOADING_DECORATION_DELAY = 100;
 
 	private static counter: number = 0;
 	private instance: number;

src/sql/workbench/contrib/objectExplorer/test/browser/connectionTreeActions.test.ts

@@ -450,7 +450,7 @@ suite('SQL Connection Tree Action tests', () => {
 		return refreshAction.run().then((value) => {
 			connectionManagementService.verify(x => x.isConnected(undefined, TypeMoq.It.isAny()), TypeMoq.Times.atLeastOnce());
 			objectExplorerService.verify(x => x.getObjectExplorerNode(TypeMoq.It.isAny()), TypeMoq.Times.atLeastOnce());
-			objectExplorerService.verify(x => x.refreshTreeNode(TypeMoq.It.isAny(), TypeMoq.It.isAny()), TypeMoq.Times.atLeastOnce());
+			objectExplorerService.verify(x => x.refreshTreeNode(TypeMoq.It.isAny(), TypeMoq.It.isAny()), TypeMoq.Times.exactly(0));
 			tree.verify(x => x.refresh(TypeMoq.It.isAny()), TypeMoq.Times.atLeastOnce());
 		});
 	});

src/sql/workbench/services/objectExplorer/browser/connectionTreeAction.ts

@@ -60,16 +60,17 @@ export class RefreshAction extends Action {
 
 		if (treeNode) {
 			try {
-				try {
-					const session = treeNode.getSession();
-					if (session) {
-						await this._objectExplorerService.refreshTreeNode(session, treeNode);
-					}
-				} catch (error) {
-					this.showError(error);
-					return;
-				}
 				if (this._tree instanceof AsyncServerTree) {
+					// Code moved here as non async tree already does it in it's refresh function (required to show loading spinner)
+					try {
+						const session = treeNode.getSession();
+						if (session) {
+							await this._objectExplorerService.refreshTreeNode(session, treeNode);
+						}
+					} catch (error) {
+						this.showError(error);
+						return;
+					}
 					await this._tree.updateChildren(this.element);
 				} else {
 					await this._tree.refresh(this.element);

src/sql/workbench/services/objectExplorer/browser/serverTreeDataSource.ts

@@ -62,21 +62,20 @@ export class ServerTreeDataSource implements IDataSource {
 			return (element as ConnectionProfileGroup).getChildren();
 		} else if (element instanceof TreeNode) {
 			let node = element;
-			if (node.children) {
+			try {
+				// Grab the latest data from the server of the node's children.
+				await this._objectExplorerService.refreshTreeNode(node.getSession()!, node);
 				return node.children;
-			} else {
+			}
-				try {
+			catch (expandRefreshError) {
-					return this._objectExplorerService.resolveTreeNodeChildren(node.getSession()!, node);
+				await node.setExpandedState(TreeItemCollapsibleState.Collapsed);
-				} catch (expandError) {
+				node.errorStateMessage = expandRefreshError;
-					await node.setExpandedState(TreeItemCollapsibleState.Collapsed);
+				this.showError(expandRefreshError);
-					node.errorStateMessage = expandError;
+				// collapse node and refresh in case of error so remove tree cache
-					this.showError(expandError);
+				setTimeout(() => {
-					// collapse node and refresh in case of error so remove tree cache
+					tree.collapse(element).then(() => tree.refresh(element));
-					setTimeout(() => {
+				});
-						tree.collapse(element).then(() => tree.refresh(element));
+				return [];
-					});
-					return [];
-				}
 			}
 		}
 		return [];

src/sql/workbench/services/objectExplorer/browser/treeCreationUtils.ts

@@ -134,7 +134,8 @@ export class TreeCreationUtils {
 					indentPixels: 10,
 					twistiePixels: 20,
 					ariaLabel: nls.localize('treeCreation.regTreeAriaLabel', "Servers"),
-					horizontalScrollMode: horizontalScrollMode ? ScrollbarVisibility.Auto : ScrollbarVisibility.Hidden
+					horizontalScrollMode: horizontalScrollMode ? ScrollbarVisibility.Auto : ScrollbarVisibility.Hidden,
+					showLoading: true
 				});
 		}