From 6b509820b35837b686d9b022fb91e631f430a5f9 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sun, 13 Mar 2016 20:31:15 -0400 Subject: [PATCH] Fixes #918, Enabling "Show Unfiled Items" or "Show Duplicates" breaks UI --- chrome/content/zotero/zoteroPane.js | 13 ++++++------- test/content/support.js | 27 +++++++++++++++++++++++++++ test/tests/collectionTreeViewTest.js | 4 ++-- test/tests/duplicatesTest.js | 28 +++------------------------- test/tests/zoteroPaneTest.js | 24 ++++++++++++++++++++++-- 5 files changed, 60 insertions(+), 36 deletions(-) diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 914e8211b4..5bef9a5adc 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -912,17 +912,16 @@ var ZoteroPane = new function() yield this.collectionsView.refresh(); - // If group is closed, open it - yield this.collectionsView.selectLibrary(libraryID); - row = this.collectionsView.selection.currentIndex; - if (!this.collectionsView.isContainerOpen(row)) { - this.collectionsView.toggleOpenState(row); - } - // Select new row if (show) { yield this.collectionsView.selectByID(lastViewedFolderID); } + // Select library root when hiding + else { + yield this.collectionsView.selectLibrary(libraryID); + } + + this.collectionsView.selection.selectEventsSuppressed = false; }); diff --git a/test/content/support.js b/test/content/support.js index 38da3e99c5..d9d9ddb35b 100644 --- a/test/content/support.js +++ b/test/content/support.js @@ -251,6 +251,33 @@ function waitForCallback(cb, interval, timeout) { } +function clickOnItemsRow(itemsView, row, button = 0) { + var x = {}; + var y = {}; + var width = {}; + var height = {}; + itemsView._treebox.getCoordsForCellItem( + row, + itemsView._treebox.columns.getNamedColumn('zotero-items-column-title'), + 'text', + x, y, width, height + ); + + // Select row to trigger multi-select + var tree = itemsView._treebox.treeBody; + var rect = tree.getBoundingClientRect(); + var x = rect.left + x.value; + var y = rect.top + y.value; + tree.dispatchEvent(new MouseEvent("mousedown", { + clientX: x, + clientY: y, + button, + detail: 1 + })); +} + + + /** * Get a default group used by all tests that want one, creating one if necessary */ diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js index e5b8e9c4a8..25706b8394 100644 --- a/test/tests/collectionTreeViewTest.js +++ b/test/tests/collectionTreeViewTest.js @@ -22,8 +22,8 @@ describe("Zotero.CollectionTreeView", function() { Zotero.Prefs.clear('duplicateLibraries'); Zotero.Prefs.clear('unfiledLibraries'); yield cv.refresh(); - assert.isTrue(cv.getRowIndexByID("D" + userLibraryID)); - assert.isTrue(cv.getRowIndexByID("U" + userLibraryID)); + assert.ok(cv.getRowIndexByID("D" + userLibraryID)); + assert.ok(cv.getRowIndexByID("U" + userLibraryID)); assert.equal(Zotero.Prefs.get('duplicateLibraries'), "" + userLibraryID); assert.equal(Zotero.Prefs.get('unfiledLibraries'), "" + userLibraryID); }); diff --git a/test/tests/duplicatesTest.js b/test/tests/duplicatesTest.js index bcb3290d86..d7d84a1d83 100644 --- a/test/tests/duplicatesTest.js +++ b/test/tests/duplicatesTest.js @@ -32,31 +32,9 @@ describe("Duplicate Items", function () { // Select the first item, which should select both var iv = zp.itemsView; - var index = iv.getRowIndexByID(item1.id); - assert.isNumber(index); - - var x = {}; - var y = {}; - var width = {}; - var height = {}; - iv._treebox.getCoordsForCellItem( - index, - iv._treebox.columns.getNamedColumn('zotero-items-column-title'), - 'text', - x, y, width, height - ); - - // Select row to trigger multi-select - var tree = iv._treebox.treeBody; - var rect = tree.getBoundingClientRect(); - var x = rect.left + x.value; - var y = rect.top + y.value; - tree.dispatchEvent(new MouseEvent("mousedown", { - clientX: x, - clientY: y, - detail: 1 - })); - + var row = iv.getRowIndexByID(item1.id); + assert.isNumber(row); + clickOnItemsRow(iv, row); assert.equal(iv.selection.count, 2); // Click merge button diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index 1b88d040c3..a86c23df97 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -217,6 +217,11 @@ describe("ZoteroPane", function() { }) it("should show a hidden virtual folder", function* () { + // Create unfiled, duplicate items + var title = Zotero.Utilities.randomString(); + var item1 = yield createDataObject('item', { title }); + var item2 = yield createDataObject('item', { title }); + // Start hidden Zotero.Prefs.set('duplicateLibraries', ""); Zotero.Prefs.set('unfiledLibraries', ""); @@ -226,7 +231,17 @@ describe("ZoteroPane", function() { var id = "D" + userLibraryID; assert.isFalse(cv.getRowIndexByID(id)); yield zp.setVirtual(userLibraryID, 'duplicates', true); - assert.ok(cv.getRowIndexByID(id)); + + // Clicking should select both items + var row = cv.getRowIndexByID(id); + assert.ok(row); + assert.equal(cv.selection.currentIndex, row); + yield waitForItemsLoad(win); + var iv = zp.itemsView; + row = iv.getRowIndexByID(item1.id); + assert.isNumber(row); + clickOnItemsRow(iv, row); + assert.equal(iv.selection.count, 2); // Show Unfiled Items id = "U" + userLibraryID; @@ -254,20 +269,25 @@ describe("ZoteroPane", function() { it("should hide an explicitly shown virtual folder", function* () { // Start shown Zotero.Prefs.set('duplicateLibraries', "" + userLibraryID); - Zotero.Prefs.set('unfiledLibraries' "" + userLibraryID); + Zotero.Prefs.set('unfiledLibraries', "" + userLibraryID); yield cv.refresh(); // Hide Duplicate Items var id = "D" + userLibraryID; assert.ok(yield cv.selectByID(id)); + yield waitForItemsLoad(win); yield zp.setVirtual(userLibraryID, 'duplicates', false); assert.isFalse(cv.getRowIndexByID(id)); + assert.equal(cv.getSelectedLibraryID(), userLibraryID); + // Hide Unfiled Items id = "U" + userLibraryID; assert.ok(yield cv.selectByID(id)); + yield waitForItemsLoad(win); yield zp.setVirtual(userLibraryID, 'unfiled', false); assert.isFalse(cv.getRowIndexByID(id)); + assert.equal(cv.getSelectedLibraryID(), userLibraryID); }); }); })