From e1706e15e2a7ca730d341613c5b85c98b1c89ee6 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sat, 7 May 2016 04:02:42 -0400 Subject: [PATCH] Expand/collapse library fixes - Fixes #994, 5.0: "+" doesn't expand all collections within a library - If a container (library, collection) is closed directly, the open state of all containers below it are now restored when it's reopened. Previously all collections would be closed on a manual reopen (though they might have been restored on the next Zotero restart). - If "-" is pressed, all containers are closed, and reopening the library will show only top-level collections. --- .../zotero/xpcom/collectionTreeView.js | 71 +++++++++++++++++-- test/tests/collectionTreeViewTest.js | 46 ++++++++++++ 2 files changed, 111 insertions(+), 6 deletions(-) diff --git a/chrome/content/zotero/xpcom/collectionTreeView.js b/chrome/content/zotero/xpcom/collectionTreeView.js index d06954765..cfee184fa 100644 --- a/chrome/content/zotero/xpcom/collectionTreeView.js +++ b/chrome/content/zotero/xpcom/collectionTreeView.js @@ -98,7 +98,7 @@ Zotero.CollectionTreeView.prototype.setTree = Zotero.Promise.coroutine(function* var key = String.fromCharCode(event.which); if (key == '+' && !(event.ctrlKey || event.altKey || event.metaKey)) { - this.expandLibrary(libraryID); + this.expandLibrary(libraryID, true); } else if (key == '-' && !(event.shiftKey || event.ctrlKey || event.altKey || event.metaKey)) { @@ -854,6 +854,7 @@ Zotero.CollectionTreeView.prototype.toggleOpenState = Zotero.Promise.coroutine(f this._rows[row].isOpen = true; this._treebox.invalidateRow(row); this._refreshRowMap(); + this._startRememberOpenStatesTimer(); }); @@ -873,9 +874,25 @@ Zotero.CollectionTreeView.prototype._closeContainer = function (row) { this._rows[row].isOpen = false; this._treebox.invalidateRow(row); this._refreshRowMap(); + this._startRememberOpenStatesTimer(); } +/** + * After a short delay, persist the open states of the tree, or if already queued, cancel and requeue. + * This avoids repeated saving while opening or closing multiple rows. + */ +Zotero.CollectionTreeView.prototype._startRememberOpenStatesTimer = function () { + if (this._rememberOpenStatesTimeoutID) { + clearTimeout(this._rememberOpenStatesTimeoutID); + } + this._rememberOpenStatesTimeoutID = setTimeout(() => { + this._rememberOpenStates(); + this._rememberOpenStatesTimeoutID = null; + }, 250) +}; + + Zotero.CollectionTreeView.prototype.isSelectable = function (row, col) { var treeRow = this.getRow(row); switch (treeRow.type) { @@ -915,7 +932,11 @@ Zotero.CollectionTreeView.prototype.__defineGetter__('editable', function () { }); -Zotero.CollectionTreeView.prototype.expandLibrary = Zotero.Promise.coroutine(function* (libraryID) { +/** + * @param {Integer} libraryID + * @param {Boolean} [recursive=false] - Expand all collections and subcollections + */ +Zotero.CollectionTreeView.prototype.expandLibrary = Zotero.Promise.coroutine(function* (libraryID, recursive) { var row = this._rowMap['L' + libraryID] if (row === undefined) { return false; @@ -923,6 +944,15 @@ Zotero.CollectionTreeView.prototype.expandLibrary = Zotero.Promise.coroutine(fun if (!this.isContainerOpen(row)) { yield this.toggleOpenState(row); } + + if (recursive) { + for (let i = row; i < this.rowCount && this.getRow(i).ref.libraryID == libraryID; i++) { + if (this.isContainer(i) && !this.isContainerOpen(i)) { + yield this.toggleOpenState(i); + } + } + } + return true; }); @@ -932,8 +962,35 @@ Zotero.CollectionTreeView.prototype.collapseLibrary = function (libraryID) { if (row === undefined) { return false; } - this._closeContainer(row); + + var closed = []; + var found = false; + for (let i = this.rowCount - 1; i >= row; i--) { + let treeRow = this.getRow(i); + if (treeRow.ref.libraryID !== libraryID) { + // Once we've moved beyond the original library, stop looking + if (found) { + break; + } + continue; + } + found = true; + + if (this.isContainer(i) && this.isContainerOpen(i)) { + closed.push(treeRow.id); + this._closeContainer(i); + } + } + + // Select the collapsed library this.selection.select(row); + + // We have to manually delete closed rows from the container state object, because otherwise + // _rememberOpenStates() wouldn't see any of the rows under the library (since the library is now + // collapsed) and they'd remain as open in the persisted object. + closed.forEach(id => { delete this._containerState[id]; }); + this._rememberOpenStates(); + return true; }; @@ -1145,7 +1202,7 @@ Zotero.CollectionTreeView.prototype.deleteSelection = Zotero.Promise.coroutine(f /** - * Expand row based on last state, or manually from toggleOpenState() + * Expand row based on last state */ Zotero.CollectionTreeView.prototype._expandRow = Zotero.Promise.coroutine(function* (rows, row, forceOpen) { var treeRow = rows[row]; @@ -1179,8 +1236,7 @@ Zotero.CollectionTreeView.prototype._expandRow = Zotero.Promise.coroutine(functi var showTrash = false; } - // If not a manual open and either the library is set to be hidden - // or this is a collection that isn't explicitly opened, + // If not a manual open and either the library is set to be collapsed or this is a collection that isn't explicitly opened, // set the initial state to closed if (!forceOpen && (this._containerState[treeRow.id] === false @@ -1312,6 +1368,9 @@ Zotero.CollectionTreeView.prototype._refreshRowMap = function() { } +/** + * Persist the current open/closed state of rows to a pref + */ Zotero.CollectionTreeView.prototype._rememberOpenStates = Zotero.Promise.coroutine(function* () { var state = this._containerState; diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js index 66bf22698..54da165b6 100644 --- a/test/tests/collectionTreeViewTest.js +++ b/test/tests/collectionTreeViewTest.js @@ -70,6 +70,52 @@ describe("Zotero.CollectionTreeView", function() { }) }) + describe("#expandLibrary()", function () { + var libraryRow, col1, col2, col3; + + before(function* () { + yield cv.selectLibrary(userLibraryID); + libraryRow = cv.selection.currentIndex; + }); + + beforeEach(function* () { + // My Library + // - A + // - B + // - C + col1 = yield createDataObject('collection'); + col2 = yield createDataObject('collection', { parentID: col1.id }); + col3 = yield createDataObject('collection', { parentID: col2.id }); + }); + + it("should open a library and respect stored container state", function* () { + // Collapse B + yield cv.toggleOpenState(cv.getRowIndexByID(col2.collectionTreeViewID)); + yield cv._rememberOpenStates(); + + // Close and reopen library + yield cv.toggleOpenState(libraryRow); + yield cv.expandLibrary(userLibraryID); + + assert.ok(cv.getRowIndexByID(col1.collectionTreeViewID)) + assert.ok(cv.getRowIndexByID(col2.collectionTreeViewID)) + assert.isFalse(cv.getRowIndexByID(col3.collectionTreeViewID)) + }); + + it("should open a library and all subcollections in recursive mode", function* () { + yield cv.toggleOpenState(cv.getRowIndexByID(col2.collectionTreeViewID)); + yield cv._rememberOpenStates(); + + // Close and reopen library + yield cv.toggleOpenState(libraryRow); + yield cv.expandLibrary(userLibraryID, true); + + assert.ok(cv.getRowIndexByID(col1.collectionTreeViewID)) + assert.ok(cv.getRowIndexByID(col2.collectionTreeViewID)) + assert.ok(cv.getRowIndexByID(col3.collectionTreeViewID)) + }); + }); + describe("#expandToCollection()", function () { it("should expand a collection to a subcollection", function* () { var collection1 = yield createDataObject('collection');