From 47aeff8943bdb0abd654e035952318558c8ea302 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adomas=20Ven=C4=8Dkauskas?= Date: Mon, 28 Sep 2026 10:02:57 +0300 Subject: [PATCH] Do not scroll tree on attachment reparent --- chrome/content/zotero/itemTree.jsx | 20 ++++++----- test/tests/collectionViewItemTreeTest.js | 42 +++++++++--------------- 2 files changed, 27 insertions(+), 35 deletions(-) diff --git a/chrome/content/zotero/itemTree.jsx b/chrome/content/zotero/itemTree.jsx index 515ceaee17..f24da90f72 100644 --- a/chrome/content/zotero/itemTree.jsx +++ b/chrome/content/zotero/itemTree.jsx @@ -1232,7 +1232,7 @@ var ItemTree = class ItemTree extends LibraryTree { /** * Capture selection and scroll position before changing rows. Preserve the first - * visible row by default; single-item edits and reparenting preserve selection scroll. + * visible row by default; single-item edits preserve selection scroll. * The update's restoreSelection and restoreScroll flags independently apply this state. * * @param {Object} [options] @@ -1360,16 +1360,18 @@ var ItemTree = class ItemTree extends LibraryTree { return; } - // Preserve selection scroll for single-item edits and reparenting, including - // moves of multiple items. Other bulk changes preserve the first visible row. - let preserveSelectionScroll = action == 'modify' && type == 'item' - && (ids.length == 1 || ids.some(id => { - let row = this._rowMap[id]; - if (row === undefined) return false; + // Preserve selection scroll for single-item edits (e.g. if a change makes the + // item resort and move somewhere else in the tree) + // Other changes preserve the first visible row. + let preserveSelectionScroll = false; + if (action == 'modify' && type == 'item' && ids.length == 1) { + let row = this._rowMap[ids[0]]; + if (row !== undefined) { let parentIndex = this.getParentIndex(row); let oldParentID = parentIndex == -1 ? null : this.getRow(parentIndex).ref.id; - return oldParentID != (this.getRow(row).ref.parentItemID || null); - })); + preserveSelectionScroll = oldParentID == (this.getRow(row).ref.parentItemID || null); + } + } this._cacheState({ preserveSelectionScroll }); await this.rowProvider.notify(action, type, ids, extraData); diff --git a/test/tests/collectionViewItemTreeTest.js b/test/tests/collectionViewItemTreeTest.js index 8d7e1c9308..95ef347011 100644 --- a/test/tests/collectionViewItemTreeTest.js +++ b/test/tests/collectionViewItemTreeTest.js @@ -1210,34 +1210,24 @@ describe("CollectionViewItemTree", function () { assert.equal(treebox.scrollOffset, 0); }); - for (let count of [1, 2]) { - it(`should preserve selection scroll position when moving ${count} child item(s) to another parent`, async function () { - let attachments = []; - for (let i = 0; i < count; i++) { - attachments.push(await importFileAttachment('test.png', { parentItemID: items[0].id })); - } - itemsView.expandAllRows(true); - await itemsView.selectItems(attachments.map(item => item.id)); - treebox.scrollTo(7); - let firstSelected = itemsView.getRow(itemsView.getRowIndexByID(items[0].id) + 1).ref; - let offset = treebox.getRowPosition(itemsView.getRowIndexByID(firstSelected.id)) - treebox.scrollOffset; + it("shouldn't scroll when a selected child item is moved to a parent further down", async function () { + let attachment = await importFileAttachment('test.png', { parentItemID: items[0].id }); + itemsView.expandAllRows(true); + await itemsView.selectItem(attachment.id); + // Scroll partway into a row, which should be preserved as is + treebox.scrollTo(7); - await Zotero.DB.executeTransaction(async function () { - for (let attachment of attachments) { - attachment.parentItemID = items[3].id; - await attachment.save(); - } - }); - await itemsView.waitForLoad(); + attachment.parentItemID = items[3].id; + await attachment.saveTx(); + await itemsView.waitForLoad(); - assert.sameMembers(itemsView.getSelectedItems(true), attachments.map(item => item.id)); - assert.equal( - treebox.getRowPosition(itemsView.getRowIndexByID(firstSelected.id)) - treebox.scrollOffset, - offset - ); - assert.isAbove(treebox.scrollOffset, 7); - }); - } + assert.sameMembers(itemsView.getSelectedItems(true), [attachment.id]); + assert.equal( + itemsView.getRowIndexByID(attachment.id), + itemsView.getRowIndexByID(items[3].id) + 1 + ); + assert.equal(treebox.scrollOffset, 7); + }); }); it("should update search results when items are added", async function () {