Notebooks View: Do not recenter view when selecting a notebook that is visible (#15442) (#15455)

* do not focus element when tree item is visible

* reset reveal behavior

* add comment
This commit is contained in:
Barbara Valdez
2021-05-13 10:46:46 -07:00
committed by GitHub
parent 166faccf1e
commit 419c2324c9
3 changed files with 17 additions and 10 deletions

View File

@@ -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) 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 (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)); 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 bookItem: BookTreeItem;
let notebookPath: string; let notebookPath: string;
// If no uri is passed in, try to use the current active notebook editor // 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) { 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 // Select + focus item in viewlet if books viewlet is already open, or if we pass in variable
if (bookItem?.contextValue && bookItem.contextValue !== 'pinnedNotebook') { if (bookItem?.contextValue && bookItem.contextValue !== 'pinnedNotebook') {
// Note: 3 is the maximum number of levels that the vscode APIs let you expand to // 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; return bookItem;
} }
async findAndExpandParentNode(notebookPath: string): Promise<BookTreeItem | undefined> { async findAndExpandParentNode(notebookPath: string, shouldFocus: boolean): Promise<BookTreeItem | undefined> {
notebookPath = notebookPath.replace(/\\/g, '/'); notebookPath = notebookPath.replace(/\\/g, '/');
const parentBook = this.books.find(b => notebookPath.indexOf(b.bookPath) > -1); const parentBook = this.books.find(b => notebookPath.indexOf(b.bookPath) > -1);
if (!parentBook) { if (!parentBook) {
@@ -470,7 +477,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
} }
try { try {
// TO DO: Check why the reveal fails during initial load with 'TreeError [bookTreeView] Tree element not found' // 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) { catch (e) {
console.error(e); console.error(e);

View File

@@ -150,9 +150,9 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi
azdata.nb.onDidChangeActiveNotebookEditor(e => { azdata.nb.onDidChangeActiveNotebookEditor(e => {
if (e.document.uri.scheme === 'untitled') { if (e.document.uri.scheme === 'untitled') {
providedBookTreeViewProvider.revealDocumentInTreeView(e.document.uri, false); providedBookTreeViewProvider.revealDocumentInTreeView(e.document.uri, false, false);
} else { } else {
bookTreeViewProvider.revealDocumentInTreeView(e.document.uri, false); bookTreeViewProvider.revealDocumentInTreeView(e.document.uri, false, false);
} }
}); });

View File

@@ -244,7 +244,7 @@ describe('BooksTreeViewTests', function () {
it('revealActiveDocumentInViewlet should return correct bookItem for highlight', async () => { it('revealActiveDocumentInViewlet should return correct bookItem for highlight', async () => {
let notebook1Path = path.join(rootFolderPath, 'Book', 'content', 'notebook1.ipynb').replace(/\\/g, '/'); 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(); should(currentSelection).not.be.undefined();
equalBookItems(currentSelection, expectedNotebook1); equalBookItems(currentSelection, expectedNotebook1);
}); });
@@ -329,7 +329,7 @@ describe('BooksTreeViewTests', function () {
it('revealActiveDocumentInViewlet should return correct bookItem for highlight', async () => { it('revealActiveDocumentInViewlet should return correct bookItem for highlight', async () => {
let notebook1Path = path.join(rootFolderPath, 'Book', 'content', 'notebook1.ipynb').replace(/\\/g, '/'); 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(); should(currentSelection).not.be.undefined();
equalBookItems(currentSelection, expectedNotebook1); equalBookItems(currentSelection, expectedNotebook1);
}); });