From 558dfd693c75f6b6796c96e0e8321673ff519ab2 Mon Sep 17 00:00:00 2001
From: Barbara Valdez <34872381+barbaravaldez@users.noreply.github.com>
Date: Thu, 13 May 2021 10:19:19 -0700
Subject: [PATCH] Notebooks View: Do not recenter view when selecting a
 notebook that is visible (#15442)

* do not focus element when tree item is visible

* reset reveal behavior

* add comment
---
 extensions/notebook/src/book/bookTreeView.ts  | 19 +++++++++++++------
 extensions/notebook/src/extension.ts          |  4 ++--
 .../notebook/src/test/book/book.test.ts       |  4 ++--
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/extensions/notebook/src/book/bookTreeView.ts b/extensions/notebook/src/book/bookTreeView.ts
index b1796d1b1d..13f6c6aa4c 100644
--- a/extensions/notebook/src/book/bookTreeView.ts
+++ b/extensions/notebook/src/book/bookTreeView.ts
@@ -68,7 +68,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 				book && // The notebook is part of a book in the viewlet (otherwise nothing to reveal)
 				(this._openAsUntitled ? notebookPath?.scheme === 'untitled' : notebookPath?.scheme !== 'untitled')) // The notebook is of the correct type for this tree view
 			{
-				await this.revealDocumentInTreeView(notebookPath);
+				await this.revealDocumentInTreeView(notebookPath, true, true);
 			}
 		});
 		this._extensionContext.subscriptions.push(azdata.nb.registerNavigationProvider(this));
@@ -391,7 +391,13 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 		}
 	}
 
-	async revealDocumentInTreeView(uri?: vscode.Uri, shouldReveal: boolean = true): Promise<BookTreeItem | undefined> {
+	/**
+	 * Reveals the given uri in the tree view.
+	 * @param uri The path to the notebook. If it's undefined then the current active notebook is revealed in the Tree View.
+	 * @param shouldReveal A boolean to expand the parent node.
+	 * @param shouldFocus A boolean to focus on the tree item.
+	 */
+	async revealDocumentInTreeView(uri: vscode.Uri | undefined, shouldReveal: boolean, shouldFocus: boolean): Promise<BookTreeItem | undefined> {
 		let bookItem: BookTreeItem;
 		let notebookPath: string;
 		// If no uri is passed in, try to use the current active notebook editor
@@ -405,17 +411,18 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 		}
 
 		if (shouldReveal || this._bookViewer?.visible) {
-			bookItem = notebookPath ? await this.findAndExpandParentNode(notebookPath) : undefined;
+			bookItem = notebookPath ? await this.findAndExpandParentNode(notebookPath, shouldFocus) : undefined;
 			// Select + focus item in viewlet if books viewlet is already open, or if we pass in variable
 			if (bookItem?.contextValue && bookItem.contextValue !== 'pinnedNotebook') {
 				// Note: 3 is the maximum number of levels that the vscode APIs let you expand to
-				await this._bookViewer.reveal(bookItem, { select: true, focus: true, expand: true });
+				await this._bookViewer.reveal(bookItem, { select: true, focus: shouldFocus, expand: true });
 			}
 		}
+
 		return bookItem;
 	}
 
-	async findAndExpandParentNode(notebookPath: string): Promise<BookTreeItem | undefined> {
+	async findAndExpandParentNode(notebookPath: string, shouldFocus: boolean): Promise<BookTreeItem | undefined> {
 		notebookPath = notebookPath.replace(/\\/g, '/');
 		const parentBook = this.books.find(b => notebookPath.indexOf(b.bookPath) > -1);
 		if (!parentBook) {
@@ -470,7 +477,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 			}
 			try {
 				// TO DO: Check why the reveal fails during initial load with 'TreeError [bookTreeView] Tree element not found'
-				await this._bookViewer.reveal(bookItemToExpand, { select: false, focus: true, expand: true });
+				await this._bookViewer.reveal(bookItemToExpand, { select: false, focus: shouldFocus, expand: true });
 			}
 			catch (e) {
 				console.error(e);
diff --git a/extensions/notebook/src/extension.ts b/extensions/notebook/src/extension.ts
index 96a6e92cc0..a1f66df95c 100644
--- a/extensions/notebook/src/extension.ts
+++ b/extensions/notebook/src/extension.ts
@@ -150,9 +150,9 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi
 
 	azdata.nb.onDidChangeActiveNotebookEditor(e => {
 		if (e.document.uri.scheme === 'untitled') {
-			providedBookTreeViewProvider.revealDocumentInTreeView(e.document.uri, false);
+			providedBookTreeViewProvider.revealDocumentInTreeView(e.document.uri, false, false);
 		} else {
-			bookTreeViewProvider.revealDocumentInTreeView(e.document.uri, false);
+			bookTreeViewProvider.revealDocumentInTreeView(e.document.uri, false, false);
 		}
 	});
 
diff --git a/extensions/notebook/src/test/book/book.test.ts b/extensions/notebook/src/test/book/book.test.ts
index 42422606b3..f56fc3df7a 100644
--- a/extensions/notebook/src/test/book/book.test.ts
+++ b/extensions/notebook/src/test/book/book.test.ts
@@ -244,7 +244,7 @@ describe('BooksTreeViewTests', function () {
 
 			it('revealActiveDocumentInViewlet should return correct bookItem for highlight', async () => {
 				let notebook1Path = path.join(rootFolderPath, 'Book', 'content', 'notebook1.ipynb').replace(/\\/g, '/');
-				let currentSelection = await bookTreeViewProvider.findAndExpandParentNode(notebook1Path);
+				let currentSelection = await bookTreeViewProvider.findAndExpandParentNode(notebook1Path, true);
 				should(currentSelection).not.be.undefined();
 				equalBookItems(currentSelection, expectedNotebook1);
 			});
@@ -329,7 +329,7 @@ describe('BooksTreeViewTests', function () {
 
 			it('revealActiveDocumentInViewlet should return correct bookItem for highlight', async () => {
 				let notebook1Path = path.join(rootFolderPath, 'Book', 'content', 'notebook1.ipynb').replace(/\\/g, '/');
-				let currentSelection = await providedbookTreeViewProvider.findAndExpandParentNode(notebook1Path);
+				let currentSelection = await providedbookTreeViewProvider.findAndExpandParentNode(notebook1Path, true);
 				should(currentSelection).not.be.undefined();
 				equalBookItems(currentSelection, expectedNotebook1);
 			});