Collection tree selection fixes

Fixes various logic around what gets selected when collections and
searches are moved to or restored from the trash (which has never been
exposed) or when they're erased
This commit is contained in:
Dan Stillman 2023-08-11 04:01:28 -04:00
parent c4eb9df716
commit 9dd182e9ca
3 changed files with 108 additions and 12 deletions

View file

@ -712,12 +712,14 @@ var CollectionTree = class CollectionTree extends LibraryTree {
this._removeRow(row - 1);
}
}
this._selectAfterRowRemoval(selectedIndex);
}
else if (action == 'modify') {
let row;
let id = ids[0];
let rowID = "C" + id;
let selectedIndex = this.selection.count ? this.selection.currentIndex : 0;
let selectedIndex = this.selection.focused;
switch (type) {
case 'collection':
@ -734,7 +736,10 @@ var CollectionTree = class CollectionTree extends LibraryTree {
// Collection was moved to trash, so don't add it back
if (collection.deleted) {
this._refreshRowMap();
this.selectAfterRowRemoval(selectedIndex);
// If collection was selected, select next row
if (selectedIndex == row) {
this._selectAfterRowRemoval(selectedIndex);
}
}
else {
await this._addSortedRow('collection', id);
@ -751,6 +756,7 @@ var CollectionTree = class CollectionTree extends LibraryTree {
// undeleted), add it (if possible without opening any containers)
else if (!collection.deleted) {
await this._addSortedRow('collection', id);
await this.selectByID(currentTreeRow.id);
// Invalidate parent in case it's become non-empty
let parentRow = this.getRowIndexByID("C" + collection.parentID);
if (parentRow !== false) {
@ -766,10 +772,13 @@ var CollectionTree = class CollectionTree extends LibraryTree {
// TODO: Only move if name changed
this._removeRow(row);
// Search moved to trash
// Search was moved to trash
if (search.deleted) {
this._refreshRowMap();
this.selectAfterRowRemoval(selectedIndex);
// If search was selected, select next row
if (selectedIndex == row) {
this._selectAfterRowRemoval(selectedIndex);
}
}
// If search isn't in trash, add it back
else {
@ -777,6 +786,20 @@ var CollectionTree = class CollectionTree extends LibraryTree {
await this.selectByID(currentTreeRow.id);
}
}
// If search isn't currently visible and it isn't in the trash (because it was
// undeleted), add it
else if (!search.deleted) {
await this._addSortedRow('search', id);
await this.selectByID(currentTreeRow.id);
// Invalidate parent in case it's become non-empty
// NOTE: Not currently used, because searches can't yet have parents
if (search.parentID) {
let parentRow = this.getRowIndexByID("S" + search.parentID);
if (parentRow !== false) {
this.tree.invalidateRow(parentRow);
}
}
}
break;
case 'feed':
@ -2269,6 +2292,9 @@ var CollectionTree = class CollectionTree extends LibraryTree {
// Add searches
for (var i = 0, len = savedSearches.length; i < len; i++) {
// Skip searches in trash
if (savedSearches[i].deleted) continue;
rows.splice(row + 1 + newRows, 0,
new Zotero.CollectionTreeRow(this, 'search', savedSearches[i], level + 1));
newRows++;
@ -2463,16 +2489,24 @@ var CollectionTree = class CollectionTree extends LibraryTree {
beforeRow
);
}
let moveSelect = beforeRow + 1;
if (moveSelect <= this.selection.focused) {
while (!this.isSelectable(moveSelect)) {
moveSelect++;
}
this.selection.select(moveSelect);
}
return beforeRow;
}
_selectAfterRowRemoval(row) {
// If last row was selected, stay on the last row
if (row >= this._rows.length) {
row = this._rows.length - 1;
};
// Make sure the selection doesn't land on a separator (e.g. deleting last feed)
while (row >= 0 && !this.isSelectable(row)) {
// move up, since we got shifted down
row--;
}
this.selection.select(row);
}
}
module.exports = CollectionTree;

View file

@ -440,6 +440,7 @@ var clearFeeds = Zotero.Promise.coroutine(function* () {
* @param {String} [params.parentKey]
* @param {Boolean} [params.synced]
* @param {Integer} [params.version]
* @param {Boolean} [params.deleted]
* @param {Integer} [params.dateAdded] - Allowed for items
* @param {Integer} [params.dateModified] - Allowed for items
*/

View file

@ -280,6 +280,67 @@ describe("Zotero.CollectionTree", function() {
assert.equal(selected, id);
});
describe(".deleted selection", function () {
for (let objectType of ['collection', 'search']) {
it(`should select next row when ${objectType} is moved to trash`, async function () {
var ran = Zotero.Utilities.randomString();
var o1 = await createDataObject(objectType, { name: ran + "AAA" });
var o2 = await createDataObject(objectType, { name: ran + "BBB" });
var o3 = await createDataObject(objectType, { name: ran + "CCC" });
await cv.selectByID(o2.treeViewID);
o2.deleted = true;
await o2.saveTx();
assert.equal(zp.getCollectionTreeRow().ref.id, o3.id);
});
it(`should maintain selection on ${objectType} when row above is moved to trash`, async function () {
var ran = Zotero.Utilities.randomString();
var o1 = await createDataObject(objectType, { name: ran + "AAA" });
var o2 = await createDataObject(objectType, { name: ran + "BBB" });
var o3 = await createDataObject(objectType, { name: ran + "CCC" });
assert.equal(zp.getCollectionTreeRow().ref.id, o3.id);
o1.deleted = true;
await o1.saveTx();
assert.equal(zp.getCollectionTreeRow().ref.id, o3.id);
});
it(`should maintain selection on trash when ${objectType} is restored`, async function () {
var o = await createDataObject(objectType, { deleted: true });
await cv.selectByID("T1");
o.deleted = false;
await o.saveTx();
assert.isTrue(zp.getCollectionTreeRow().isTrash());
// Row should have been added back
assert.isAbove(cv.getRowIndexByID(o.treeViewID), 0);
});
}
});
for (let objectType of ['collection', 'search']) {
it(`should select next row when ${objectType} is erased`, async function () {
var ran = Zotero.Utilities.randomString();
var o1 = await createDataObject(objectType, { name: ran + "AAA" });
var o2 = await createDataObject(objectType, { name: ran + "BBB" });
var o3 = await createDataObject(objectType, { name: ran + "CCC" });
await cv.selectByID(o2.treeViewID);
await o2.eraseTx();
assert.equal(zp.getCollectionTreeRow().ref.id, o3.id);
});
}
it("should update the editability of the current view", function* () {
var group = yield createGroup({
editable: false,