Edit book using drag and drop (#16906)

- Use the onDrop method for moving notebooks/sections in the Notebooks Tree View.
- Allow multi selection in tree view
- Modify notebook commands to only show when a single tree item is selected.

extensions/notebook/package.json

@@ -470,24 +470,24 @@
         },
         {
           "command": "notebook.command.closeBook",
-          "when": "view == bookTreeView && viewItem == savedBook"
+          "when": "view == bookTreeView && viewItem == savedBook && listMultiSelection == false"
         },
         {
           "command": "notebook.command.closeNotebook",
-          "when": "view == bookTreeView && viewItem == savedNotebook"
+          "when": "view == bookTreeView && viewItem == savedNotebook && listMultiSelection == false"
         },
         {
           "command": "notebook.command.removeNotebook",
-          "when": "view == bookTreeView && viewItem == savedBookNotebook"
+          "when": "view == bookTreeView && viewItem == savedBookNotebook && listMultiSelection == false"
         },
         {
           "command": "notebook.command.addNotebook",
-          "when": "view == bookTreeView && viewItem == section || view == bookTreeView && viewItem == savedBook",
+          "when": "view == bookTreeView && viewItem == section && listMultiSelection == false || view == bookTreeView && viewItem == savedBook && listMultiSelection == false",
           "group": "newFile@1"
         },
         {
           "command": "notebook.command.addMarkdown",
-          "when": "view == bookTreeView && viewItem == section || view == bookTreeView && viewItem == savedBook",
+          "when": "view == bookTreeView && viewItem == section && listMultiSelection == false || view == bookTreeView && viewItem == savedBook && listMultiSelection == false",
           "group": "newFile@1"
         },
         {

extensions/notebook/src/book/bookTocManager.ts

@@ -15,7 +15,7 @@ import { TocEntryPathHandler } from './tocEntryPathHandler';
 import { FileExtension } from '../common/utils';
 
 export interface IBookTocManager {
-	updateBook(element: BookTreeItem, book: BookTreeItem, targetSection?: JupyterBookSection): Promise<void>;
+	updateBook(sources: BookTreeItem[], target: BookTreeItem, targetSection?: JupyterBookSection): Promise<void>;
 	removeNotebook(element: BookTreeItem): Promise<void>;
 	createBook(bookContentPath: string, contentFolder: string): Promise<void>;
 	addNewFile(pathDetails: TocEntryPathHandler, bookItem: BookTreeItem): Promise<void>;
@@ -339,7 +339,7 @@ export class BookTocManager implements IBookTocManager {
 				this.newSection = sectionTOC;
 			}
 		}
-		this.newSection.title = section.title;
+		this.newSection.title = section.book.title;
 		this.newSection.file = path.posix.join(path.parse(uri).dir, fileName);
 		if (section.sections) {
 			const files = section.sections as JupyterBookSection[];
@@ -366,8 +366,11 @@ export class BookTocManager implements IBookTocManager {
 		const filePath = path.parse(file.book.contentPath);
 		let fileName = undefined;
 		try {
-			this.movedFiles.set(file.book.contentPath, path.join(rootPath, filePath.base));
-			await fs.move(file.book.contentPath, path.join(rootPath, filePath.base), { overwrite: false });
+			// no op if the notebook is already in the dest location
+			if (file.book.contentPath !== path.join(rootPath, filePath.base)) {
+				this.movedFiles.set(file.book.contentPath, path.join(rootPath, filePath.base));
+				await fs.move(file.book.contentPath, path.join(rootPath, filePath.base), { overwrite: false });
+			}
 		} catch (error) {
 			if (error.code === 'EEXIST') {
 				fileName = await this.renameFile(file.book.contentPath, path.join(rootPath, filePath.base));
@@ -394,44 +397,47 @@ export class BookTocManager implements IBookTocManager {
 
 	/**
 	 * Moves the element to the target book's folder and adds it to the table of contents.
-	 * @param element Notebook, Markdown File, or section that will be added to the book.
-	 * @param targetBook Book that will be modified.
-	 * @param targetSection Book section that'll be modified.
+	 * @param sources The tree items that are been moved.
+	 * @param target Target tree item on which the sources will be added
+	 * @param section (Optional) book section that'll be modified. Not required when using drag and drop.
 	*/
-	public async updateBook(element: BookTreeItem, targetItem: BookTreeItem, targetSection?: JupyterBookSection): Promise<void> {
-		try {
-			if (element.contextValue === 'section') {
-				// modify the sourceBook toc and remove the section
-				const findSection: JupyterBookSection = { file: element.book.page.file, title: element.book.page.title };
-				await this.moveSectionFiles(element, targetItem);
-				// remove section from book
-				await this.updateTOC(element.book.version, element.tableOfContentsPath, findSection, undefined);
-				// add section to book
-				await this.updateTOC(targetItem.book.version, targetItem.tableOfContentsPath, targetSection, this.newSection);
-			}
-			else {
-				// the notebook is part of a book so we need to modify its toc as well
-				const findSection = { file: element.book.page.file, title: element.book.page.title };
-				await this.moveFile(element, targetItem);
-				if (element.contextValue === 'savedBookNotebook' || element.contextValue === 'Markdown') {
-					// remove notebook entry from book toc
-					await this.updateTOC(element.book.version, element.tableOfContentsPath, findSection, undefined);
-				} else {
-					// close the standalone notebook, so it doesn't throw an error when we move the notebook to new location.
-					await vscode.commands.executeCommand('notebook.command.closeNotebook', element);
-				}
-				await this.updateTOC(targetItem.book.version, targetItem.tableOfContentsPath, targetSection, this.newSection);
-			}
-		} catch (e) {
-			await this.recovery();
-			void vscode.window.showErrorMessage(loc.editBookError(element.book.contentPath, e instanceof Error ? e.message : e));
-		} finally {
+	public async updateBook(sources: BookTreeItem[], target: BookTreeItem, section?: JupyterBookSection): Promise<void> {
+		for (let element of sources) {
 			try {
-				await this._targetBook.reinitializeContents();
+				const targetSection = section ? section : (target.contextValue === 'section' ? { file: target.book.page.file, title: target.book.page.title } : undefined);
+				if (element.contextValue === 'section') {
+					// modify the sourceBook toc and remove the section
+					const findSection: JupyterBookSection = { file: element.book.page.file, title: element.book.page.title };
+					await this.moveSectionFiles(element, target);
+					// remove section from book
+					await this.updateTOC(element.book.version, element.tableOfContentsPath, findSection, undefined);
+					// add section to book
+					await this.updateTOC(target.book.version, target.tableOfContentsPath, targetSection, this.newSection);
+				}
+				else {
+					// the notebook is part of a book so we need to modify its toc as well
+					const findSection = { file: element.book.page.file, title: element.book.page.title };
+					await this.moveFile(element, target);
+					if (element.contextValue === 'savedBookNotebook' || element.contextValue === 'Markdown') {
+						// remove notebook entry from book toc
+						await this.updateTOC(element.book.version, element.tableOfContentsPath, findSection, undefined);
+					} else {
+						// close the standalone notebook, so it doesn't throw an error when we move the notebook to new location.
+						await vscode.commands.executeCommand('notebook.command.closeNotebook', element);
+					}
+					await this.updateTOC(target.book.version, target.tableOfContentsPath, targetSection, this.newSection);
+				}
+			} catch (e) {
+				await this.recovery();
+				void vscode.window.showErrorMessage(loc.editBookError(element.book.contentPath, e instanceof Error ? e.message : e));
 			} finally {
-				if (this._sourceBook && this._sourceBook.bookPath !== this._targetBook.bookPath) {
-					// refresh source book model to pick up latest changes
-					await this._sourceBook.reinitializeContents();
+				try {
+					await this._targetBook.reinitializeContents();
+				} finally {
+					if (this._sourceBook && this._sourceBook.bookPath !== this._targetBook.bookPath) {
+						// refresh source book model to pick up latest changes
+						await this._sourceBook.reinitializeContents();
+					}
 				}
 			}
 		}

extensions/notebook/src/book/bookTreeView.ts

@@ -159,7 +159,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 		return dialog.createDialog();
 	}
 
-	async getSelectionQuickPick(movingElement: BookTreeItem): Promise<quickPickResults> {
+	async bookSectionQuickPick(): Promise<quickPickResults> {
 		let bookOptions: vscode.QuickPickItem[] = [];
 		let pickedSection: vscode.QuickPickItem;
 		this.books.forEach(book => {
@@ -172,7 +172,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 			placeHolder: loc.labelBookFolder
 		});
 
-		if (pickedBook && movingElement) {
+		if (pickedBook) {
 			const updateBook = this.books.find(book => book.bookPath === pickedBook.detail).bookItems[0];
 			if (updateBook) {
 				let bookSections = updateBook.sections;
@@ -194,11 +194,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 							break;
 						}
 						else if (pickedSection && pickedSection.detail) {
-							if (updateBook.root === movingElement.root && pickedSection.detail === movingElement.uri) {
-								pickedSection = undefined;
-							} else {
-								bookSections = updateBook.findChildSection(pickedSection.detail).sections;
-							}
+							bookSections = updateBook.findChildSection(pickedSection.detail).sections;
 						}
 					}
 				}
@@ -208,17 +204,27 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 		return undefined;
 	}
 
-	async editBook(movingElement: BookTreeItem): Promise<void> {
+	/**
+	 * Move selected tree items (sections/notebooks) using the move option tree item entry point.
+	 * A quick pick menu will show all the possible books and sections for the user to choose
+	 * the target element to add the selected tree items.
+	 * @param treeItems Elements to be moved
+	 */
+	async moveTreeItems(treeItems: BookTreeItem[]): Promise<void> {
 		TelemetryReporter.sendActionEvent(BookTelemetryView, NbTelemetryActions.MoveNotebook);
-		const selectionResults = await this.getSelectionQuickPick(movingElement);
+		const selectionResults = await this.bookSectionQuickPick();
 		if (selectionResults) {
-			const pickedSection = selectionResults.quickPickSection;
-			const updateBook = selectionResults.book;
-			const targetSection = pickedSection.detail !== undefined ? updateBook.findChildSection(pickedSection.detail) : undefined;
-			const sourceBook = this.books.find(book => book.bookPath === movingElement.book.root);
-			const targetBook = this.books.find(book => book.bookPath === updateBook.book.root);
-			this.bookTocManager = new BookTocManager(sourceBook, targetBook);
-			await this.bookTocManager.updateBook(movingElement, updateBook, targetSection);
+			let pickedSection = selectionResults.quickPickSection;
+			// filter target from sources
+			let movingElements = treeItems.filter(item => item.uri !== pickedSection.detail);
+			const targetBookItem = selectionResults.book;
+			const targetSection = pickedSection.detail !== undefined ? targetBookItem.findChildSection(pickedSection.detail) : undefined;
+			let sourcesByBook = this.groupTreeItemsByBookModel(movingElements);
+			const targetBookModel = this.books.find(book => book.bookPath === targetBookItem.book.root);
+			for (let [bookModel, items] of sourcesByBook) {
+				this.bookTocManager = new BookTocManager(bookModel, targetBookModel);
+				await this.bookTocManager.updateBook(items, targetBookItem, targetSection);
+			}
 		}
 	}
 
@@ -717,11 +723,56 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 		return Promise.resolve(result);
 	}
 
-	async onDrop(sources: vscode.TreeDataTransfer, target: BookTreeItem): Promise<void> {
-		let treeItems = JSON.parse(await sources.items.get('text/treeitems')!.asString());
-		if (treeItems) {
-
+	groupTreeItemsByBookModel(treeItems: BookTreeItem[]): Map<BookModel, BookTreeItem[]> {
+		const sourcesByBook = new Map<BookModel, BookTreeItem[]>();
+		for (let item of treeItems) {
+			const book = this.books.find(book => book.bookPath === item.book.root);
+			if (sourcesByBook.has(book)) {
+				sourcesByBook.get(book).push(item);
+			} else {
+				sourcesByBook.set(book, [item]);
+			}
 		}
+		return sourcesByBook;
+	}
+
+	async onDrop(sources: vscode.TreeDataTransfer, target: BookTreeItem): Promise<void> {
+		TelemetryReporter.sendActionEvent(BookTelemetryView, NbTelemetryActions.DragAndDrop);
+		// gets the tree items that are dragged and dropped
+		let treeItems = JSON.parse(await sources.items.get(this.supportedTypes[0])!.asString()) as BookTreeItem[];
+		let rootItems = this.getLocalRoots(treeItems);
+		rootItems = rootItems.filter(item => item.resourceUri !== target.resourceUri);
+		if (rootItems && target) {
+			let sourcesByBook = this.groupTreeItemsByBookModel(rootItems);
+			const targetBook = this.books.find(book => book.bookPath === target.book.root);
+			for (let [book, items] of sourcesByBook) {
+				this.bookTocManager = new BookTocManager(book, targetBook);
+				await this.bookTocManager.updateBook(items, target);
+			}
+		}
+	}
+
+	/**
+	 * From the tree items moved find the local roots.
+	 * We don't need to move a child element if the parent element has been selected as well, since every time that an element is moved
+	 * we add its children.
+	 * @param bookItems that have been dragged and dropped
+	 * @returns an array of tree items that do not share a parent element.
+	 */
+	public getLocalRoots(bookItems: BookTreeItem[]): BookTreeItem[] {
+		const localRoots = [];
+		for (let i = 0; i < bookItems.length; i++) {
+			const parent = bookItems[i].book.parent;
+			if (parent) {
+				const isInList = bookItems.find(item => item.resourceUri.path === parent.resourceUri.path && parent.contextValue !== 'savedBook');
+				if (isInList === undefined) {
+					localRoots.push(bookItems[i]);
+				}
+			} else {
+				localRoots.push(bookItems[i]);
+			}
+		}
+		return localRoots;
 	}
 
 	dispose(): void { }

extensions/notebook/src/extension.ts

@@ -62,8 +62,9 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi
 		await pinnedBookTreeViewProvider.removeNotebookFromPinnedView(book);
 	}));
 
-	extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.moveTo', async (book: BookTreeItem) => {
-		await bookTreeViewProvider.editBook(book);
+	extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.moveTo', async (firstTreeItem: BookTreeItem, treeItems?: BookTreeItem[]) => {
+		let allTreeItems = treeItems ? [firstTreeItem, ...treeItems] : [firstTreeItem];
+		await bookTreeViewProvider.moveTreeItems(allTreeItems);
 	}));
 
 	let model = new RemoteBookDialogModel();

extensions/notebook/src/telemetry.ts

@@ -21,5 +21,6 @@ export enum NbTelemetryActions {
 	PinNotebook = 'NotebookPinned',
 	OpenNotebookFromBook = 'NotebookOpenedFromBook',
 	MoveNotebook = 'MoveNotebook',
+	DragAndDrop = 'DragAndDrop'
 }
 

extensions/notebook/src/test/book/bookTocManager.test.ts

@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 import * as should from 'should';
 import * as path from 'path';
-import { BookTocManager, hasSections, quickPickResults } from '../../book/bookTocManager';
+import { BookTocManager, hasSections } from '../../book/bookTocManager';
 import { BookTreeItem, BookTreeItemFormat, BookTreeItemType } from '../../book/bookTreeItem';
 import * as sinon from 'sinon';
 import { IJupyterBookSectionV1, IJupyterBookSectionV2, JupyterBookSection } from '../../contracts/content';
@@ -18,7 +18,6 @@ import { BookModel } from '../../book/bookModel';
 import { MockExtensionContext } from '../common/stubs';
 import { BookTreeViewProvider } from '../../book/bookTreeView';
 import { NavigationProviders } from '../../common/constants';
-import * as loc from '../../common/localizedConstants';
 import { BookVersion } from '../../book/bookVersionHandler';
 import * as yaml from 'js-yaml';
 
@@ -461,7 +460,7 @@ describe('BookTocManagerTests', function () {
 
 				it('Add section to book', async () => {
 					bookTocManager = new BookTocManager(sourceBookModel, targetBookModel);
-					await bookTocManager.updateBook(sectionA, targetBook, undefined);
+					await bookTocManager.updateBook([sectionA], targetBook, undefined);
 					const listFiles = await fs.promises.readdir(path.join(run.targetBook.bookContentFolderPath, 'sectionA'));
 					const listSourceFiles = await fs.promises.readdir(path.join(run.sourceBook.bookContentFolderPath));
 					should(JSON.stringify(listSourceFiles).includes('sectionA')).be.false('The source book files should not contain the section A files');
@@ -470,7 +469,7 @@ describe('BookTocManagerTests', function () {
 
 				it('Add section to section', async () => {
 					bookTocManager = new BookTocManager(sourceBookModel, targetBookModel);
-					await bookTocManager.updateBook(sectionB, sectionC, {
+					await bookTocManager.updateBook([sectionB], sectionC, {
 						'title': 'Notebook 6',
 						'file': path.posix.join(path.posix.sep, 'sectionC', 'notebook6')
 					});
@@ -482,7 +481,7 @@ describe('BookTocManagerTests', function () {
 
 				it('Add notebook to book', async () => {
 					bookTocManager = new BookTocManager(undefined, targetBookModel);
-					await bookTocManager.updateBook(notebook, targetBook);
+					await bookTocManager.updateBook([notebook], targetBook);
 					const listFiles = await fs.promises.readdir(run.targetBook.bookContentFolderPath);
 					should(JSON.stringify(listFiles).includes('notebook5.ipynb')).be.true('Notebook 5 should be under the target book content folder');
 				});
@@ -514,8 +513,8 @@ describe('BookTocManagerTests', function () {
 
 				it('Add duplicated notebook to book', async () => {
 					bookTocManager = new BookTocManager(undefined, targetBookModel);
-					await bookTocManager.updateBook(notebook, targetBook);
-					await bookTocManager.updateBook(duplicatedNotebook, targetBook);
+					await bookTocManager.updateBook([notebook], targetBook);
+					await bookTocManager.updateBook([duplicatedNotebook], targetBook);
 					const listFiles = await fs.promises.readdir(run.targetBook.bookContentFolderPath);
 					should(JSON.stringify(listFiles).includes('notebook5 - 2.ipynb')).be.true('Should rename the notebook to notebook5 - 2.ipynb');
 					should(JSON.stringify(listFiles).includes('notebook5.ipynb')).be.true('Should keep notebook5.ipynb');
@@ -526,17 +525,10 @@ describe('BookTocManagerTests', function () {
 					const recoverySpy = sinon.spy(BookTocManager.prototype, 'recovery');
 					sinon.stub(BookTocManager.prototype, 'updateTOC').throws(new Error('Unexpected error.'));
 					const bookTreeViewProvider = new BookTreeViewProvider([], mockExtensionContext, false, 'bookTreeView', NavigationProviders.NotebooksNavigator);
-					const results: quickPickResults = {
-						book: targetBook,
-						quickPickSection: {
-							label: loc.labelAddToLevel,
-							description: undefined
-						}
-					};
 					bookTocManager = new BookTocManager(targetBookModel);
-					sinon.stub(bookTreeViewProvider, 'getSelectionQuickPick').returns(Promise.resolve(results));
+					sinon.stub(bookTreeViewProvider, 'moveTreeItems').returns(Promise.resolve(bookTocManager.updateBook([notebook], targetBook)));
 					try {
-						await bookTreeViewProvider.editBook(notebook);
+						await bookTreeViewProvider.moveTreeItems([notebook]);
 					} catch (error) {
 						should(recoverySpy.calledOnce).be.true('If unexpected error then recovery method is called.');
 					}