scriptoriumadmin/zotero
1
0
Fork 0
mirror of https://github.com/zotero/zotero.git synced 2026-10-03 02:21:49 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Make tree scroll behavior more consistent.

Browse source 
Default to preserving viewport position (top item), only preserve
selection position in tree viewport when editing the
item metadata (which might change sort position) or moving attachments.
This commit is contained in:
Adomas Venčkauskas 2026-09-22 15:38:01 +03:00
parent 0a7e4d26b4
commit 4a3566eb30
3 changed files with 169 additions and 273 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
chrome/content/zotero/collectionViewItemTree.jsx
View file

@ -679,7 +679,6 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider {
let selectInActiveWindow = false;
let restoreSelection = true;
let restoreScroll = true;
let preserveViewport = false;
let rowsToSelect = null;
let items = null;
@ -860,7 +859,6 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider {
// Close container to remove any sub-items
this._closeContainer(row, true);
this._removeRow(row);
preserveViewport = true;
}
// If moved from under another item to top level, remove old row and add new one
else if (parentIndex != -1 && !parentItemID) {
@ -871,7 +869,6 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider {
this._addRow(this.createRow(item, 0, false), beforeRow);
sort = id;
preserveViewport = true;
}
// If moved from one parent to another, remove from old parent
else if (parentItemID && parentIndex != -1 && this._rowMap[parentItemID] != parentIndex) {
@ -881,7 +878,6 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider {
if (newParentIndex !== undefined) {
this._refreshContainer(newParentIndex);
}
preserveViewport = true;
}
// If Unfiled Items and item was added to a collection, remove from view
else if (this.itemTree.isContainer(row) && this.viewMode == 'unfiled' && item.getCollections().length) {
@ -1105,7 +1101,6 @@ class CollectionViewItemTreeRowProvider extends ItemTreeRowProvider {
await this.runListeners('update', true, {
restoreSelection,
restoreScroll,
preserveViewport,
selectInActiveWindow,
selection: rowsToSelect
});

122
chrome/content/zotero/itemTree.jsx
View file

@ -302,7 +302,6 @@ class ItemTreeRowProvider {
restoreSelection: !preserveDetachedFocus,
expandCollapsedParents: false,
restoreScroll: true,
preserveViewport: true,
});
if (preserveDetachedFocus) {
// Collapsing a container with selected descendants moves their selection to the
@ -361,7 +360,6 @@ class ItemTreeRowProvider {
restoreSelection: true,
expandCollapsedParents: false,
restoreScroll: true,
preserveViewport: true,
});
}
@ -396,7 +394,6 @@ class ItemTreeRowProvider {
restoreSelection: true,
expandCollapsedParents: false,
restoreScroll: true,
preserveViewport: true,
});
}
@ -1233,9 +1230,17 @@ var ItemTree = class ItemTree extends LibraryTree {
return this.rowProvider.refresh(options);
})
_cacheState() {
/**
* Capture selection and scroll position before changing rows. Preserve the first
* visible row by default; single-item edits and reparenting preserve selection scroll.
* The update's restoreSelection and restoreScroll flags independently apply this state.
*
* @param {Object} [options]
* @param {boolean} [options.preserveSelectionScroll=false]
*/
_cacheState({ preserveSelectionScroll = false } = {}) {
this._cachedSelection = this.getSelectedObjects();
this._cachedScrollPosition = this._saveScrollPosition();
this._cachedScrollPosition = this._saveScrollPosition({ preserveSelectionScroll });
}
/**
@ -1249,8 +1254,6 @@ var ItemTree = class ItemTree extends LibraryTree {
* @param {boolean} options.restoreSelection - Whether to restore the cached selection.
* @param {boolean} options.ensureRowsAreVisible - Whether to ensure selected rows are visible.
* @param {boolean} options.restoreScroll - Whether to restore the cached scroll position.
* @param {boolean} options.preserveViewport - Whether to restore the scroll position by
* keeping the first visible row in place rather than the selected row.
* @param {boolean} options.loading - Whether to show loading state (hides tree, shows message).
* @param {string} options.message - Optional message to display (for loading, errors, intro text).
*/
@ -1261,7 +1264,6 @@ var ItemTree = class ItemTree extends LibraryTree {
expandCollapsedParents: true,
ensureRowsAreVisible: true,
restoreScroll: false,
preserveViewport: false,
loading: false,
message: null,
}) {
@ -1310,7 +1312,7 @@ var ItemTree = class ItemTree extends LibraryTree {
}
if (options.restoreScroll) {
this._restoreScrollPosition(null, options.preserveViewport);
this._restoreScrollPosition();
}
// Allow selection events to propagate and redraw the needed rows
@ -1358,7 +1360,17 @@ var ItemTree = class ItemTree extends LibraryTree {
return;
}
this._cacheState();
// 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;
let parentIndex = this.getParentIndex(row);
let oldParentID = parentIndex == -1 ? null : this.getRow(parentIndex).ref.id;
return oldParentID != (this.getRow(row).ref.parentItemID || null);
}));
this._cacheState({ preserveSelectionScroll });
await this.rowProvider.notify(action, type, ids, extraData);
}
@ -2690,11 +2702,8 @@ var ItemTree = class ItemTree extends LibraryTree {
* If scrollPosition is provided, restores from it without touching the cache.
*
* @param {Object|null} scrollPosition - Scroll position to restore, or null to use cached
* @param {Boolean} preserveViewport - Keep the first visible row in position rather than the
* selected row, for changes the user made in this view (e.g. expanding a container or
* dragging an attachment to another item), which shouldn't shift what's on screen
*/
_restoreScrollPosition(scrollPosition = null, preserveViewport = false) {
_restoreScrollPosition(scrollPosition = null) {
if (scrollPosition === null) {
scrollPosition = this._cachedScrollPosition;
this._cachedScrollPosition = null;
@ -2702,79 +2711,56 @@ var ItemTree = class ItemTree extends LibraryTree {
if (!scrollPosition || !this._treebox) {
return;
}
let anchors = preserveViewport
? [scrollPosition.viewport, scrollPosition.selection]
: [scrollPosition.selection, scrollPosition.viewport];
// Use the first anchor that's still in the view
for (let anchor of anchors) {
if (!anchor) {
continue;
}
let row = this._rowMap[anchor.id];
if (row === undefined) {
continue;
}
this._treebox.scrollTo(this._treebox.getRowPosition(row) - anchor.offset);
if (scrollPosition.id === undefined) {
this._treebox.scrollTo(scrollPosition.offset);
return;
}
let row = this._rowMap[scrollPosition.id];
if (row === undefined) {
return;
}
this._treebox.scrollTo(this._treebox.getRowPosition(row) - scrollPosition.offset);
}
/**
* Return an object describing the current scroll position to restore after changes
*
* @return {Object|Boolean} - Object with .selection and .viewport anchors, each with .id (a
* treeViewID) and .offset (pixels between the top of the view and the top of the row,
* so that a partly scrolled row is restored where it was), or false if there's nothing
* to anchor to
* Anchors to top visible item (viewport) scroll by default, can override to anchor to
* selected item instead. This ensures that collapse/expand doesn't move the row from under
* cursor.
*
* @param {Object} [options]
* @param {boolean} [options.preserveSelectionScroll=false] - Prefer a visible selected row over the viewport
* @return {Object|false} - A row ID and relative pixel offset
*/
_saveScrollPosition() {
_saveScrollPosition({ preserveSelectionScroll = false } = {}) {
if (!this._treebox) return false;
var treebox = this._treebox;
var first = treebox.getFirstVisibleRow();
if (first === undefined || first === null) {
return false;
}
var last = treebox.getLastVisibleRow();
var scrollOffset = treebox.scrollOffset;
let scrollOffset = treebox.scrollOffset;
if (scrollOffset == 0) {
return { offset: 0 };
}
// If an object is selected, keep the first selected one in position
var selection = null;
for (let i = first; i <= last; i++) {
if (this.selection.isSelected(i)) {
let row = this.getRow(i);
if (row) {
selection = {
id: row.ref.treeViewID,
offset: treebox.getRowPosition(i) - scrollOffset
};
let anchorIndex = first;
if (preserveSelectionScroll) {
for (let i = first; i <= treebox.getLastVisibleRow(); i++) {
if (this.selection.isSelected(i)) {
anchorIndex = i;
break;
}
break;
}
}
// With no selection to anchor to, don't save a scroll position when the
// view is already at the top of the list. Otherwise restoring after an
// insertion would pin the previously-top row in place (pushing the view
// down) instead of leaving the list scrolled to its new top.
if (!selection && !first) {
return false;
}
// Keep the first visible row in position, for changes that shouldn't shift what's on
// screen even when the selected row moves
var viewport = null;
let firstRow = this.getRow(first);
if (firstRow) {
viewport = {
id: firstRow.ref.treeViewID,
offset: treebox.getRowPosition(first) - scrollOffset
};
}
if (!selection && !viewport) {
return false;
}
return { selection, viewport };
let row = this.getRow(anchorIndex);
if (!row) return false;
return {
id: row.ref.treeViewID,
offset: treebox.getRowPosition(anchorIndex) - scrollOffset
};
}
/**

315
test/tests/collectionViewItemTreeTest.js
View file

@ -1121,210 +1121,125 @@ describe("CollectionViewItemTree", function () {
assert.sameMembers(zp.itemsView.getSelectedItems(true), [item.id]);
});
it("should keep first visible item in view when other items are added with skipSelect and nothing in view is selected", async function () {
var collection = await createDataObject('collection');
await waitForItemsLoad(win);
itemsView = zp.itemsView;
var treebox = itemsView._treebox;
var numVisibleRows = treebox.getLastVisibleRow() - treebox.getFirstVisibleRow();
// Get a numeric string left-padded with zeroes
function getTitle(i, max) {
return new String(new Array(max + 1).join(0) + i).slice(-1 * max);
}
var num = numVisibleRows + 10;
await Zotero.DB.executeTransaction(async function () {
for (let i = 0; i < num; i++) {
let title = getTitle(i, num);
let item = createUnsavedDataObject('item', { title });
item.addToCollection(collection.id);
await item.save();
}
}.bind(this));
// Scroll halfway
treebox.scrollToRow(Math.round(num / 2) - Math.round(numVisibleRows / 2));
var firstVisibleItemID = itemsView.getRow(treebox.getFirstVisibleRow()).ref.id;
// Add one item at the beginning
var item = createUnsavedDataObject(
'item', { title: getTitle(0, num), collections: [collection.id] }
);
await item.saveTx({
skipSelect: true
describe("scroll position", function () {
var collection, items, treebox;
beforeEach(async function () {
collection = await createDataObject('collection');
await select(win, collection);
itemsView = zp.itemsView;
treebox = itemsView._treebox;
items = [];
let count = treebox.getLastVisibleRow() - treebox.getFirstVisibleRow() + 30;
await Zotero.DB.executeTransaction(async function () {
for (let i = 0; i < count; i++) {
let item = createUnsavedDataObject('item', {
title: String(i).padStart(4, '0'),
collections: [collection.id],
});
await item.save({ skipSelect: true });
items.push(item);
}
});
await itemsView.selectItem(items[8].id);
treebox.scrollTo(treebox.getRowPosition(4) + 7);
});
// Then add a few more in a transaction
await Zotero.DB.executeTransaction(async function () {
for (let i = 0; i < 3; i++) {
var item = createUnsavedDataObject(
'item', { title: getTitle(0, num), collections: [collection.id] }
it("should preserve the selected item's position in viewport when an edit changes its sort position", async function () {
let offset = treebox.getRowPosition(itemsView.getRowIndexByID(items[8].id)) - treebox.scrollOffset;
items[8].setField('title', '0012a');
await items[8].saveTx();
assert.equal(
treebox.getRowPosition(itemsView.getRowIndexByID(items[8].id)) - treebox.scrollOffset,
offset
);
});
it("should preserve scroll position for multi-item edits", async function () {
let offset = treebox.scrollOffset;
await Zotero.DB.executeTransaction(async function () {
for (let i of [8, 9]) {
items[i].setField('title', '0012' + i);
await items[i].save();
}
});
assert.equal(treebox.scrollOffset, offset);
});
it("should preserve scroll position when items are added above the visible rows with no selection", async function () {
itemsView.selection.clearSelection();
let firstItem = itemsView.getRow(treebox.getFirstVisibleRow()).ref;
let offset = treebox.getRowPosition(itemsView.getRowIndexByID(firstItem.id)) - treebox.scrollOffset;
await Zotero.DB.executeTransaction(async function () {
for (let i = 0; i < 3; i++) {
let item = createUnsavedDataObject('item', {
title: '0000a', collections: [collection.id]
});
await item.save({ skipSelect: true });
}
});
assert.equal(itemsView.getRow(treebox.getFirstVisibleRow()).ref.id, firstItem.id);
assert.equal(treebox.getRowPosition(itemsView.getRowIndexByID(firstItem.id)) - treebox.scrollOffset, offset);
});
it("shouldn't scroll when items are added within the visible rows with skipSelect", async function () {
let offset = treebox.scrollOffset;
await Zotero.DB.executeTransaction(async function () {
for (let i = 0; i < 3; i++) {
let item = createUnsavedDataObject('item', {
title: '0005a', collections: [collection.id]
});
await item.save({ skipSelect: true });
}
});
assert.sameMembers(itemsView.getSelectedItems(true), [items[8].id]);
assert.equal(treebox.scrollOffset, offset);
});
it("shouldn't scroll when at the top and items are added with skipSelect", async function () {
await itemsView.selectItem(items[2].id);
treebox.scrollTo(0);
await Zotero.DB.executeTransaction(async function () {
for (let i = 0; i < 3; i++) {
let item = createUnsavedDataObject('item', {
title: '000', collections: [collection.id]
});
await item.save({ skipSelect: true });
}
});
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;
await Zotero.DB.executeTransaction(async function () {
for (let attachment of attachments) {
attachment.parentItemID = items[3].id;
await attachment.save();
}
});
await itemsView.waitForLoad();
assert.sameMembers(itemsView.getSelectedItems(true), attachments.map(item => item.id));
assert.equal(
treebox.getRowPosition(itemsView.getRowIndexByID(firstSelected.id)) - treebox.scrollOffset,
offset
);
await item.save({
skipSelect: true
});
}
}.bind(this));
// Make sure the same item is still in the first visible row
assert.equal(itemsView.getRow(treebox.getFirstVisibleRow()).ref.id, firstVisibleItemID);
});
it("should keep first visible selected item in position when other items are added with skipSelect", async function () {
var collection = await createDataObject('collection');
await select(win, collection);
itemsView = zp.itemsView;
var treebox = itemsView._treebox;
var numVisibleRows = treebox.getLastVisibleRow() - treebox.getFirstVisibleRow();
// Get a numeric string left-padded with zeroes
function getTitle(i, max) {
return new String(new Array(max + 1).join(0) + i).slice(-1 * max);
assert.isAbove(treebox.scrollOffset, 7);
});
}
var num = numVisibleRows + 10;
await Zotero.DB.executeTransaction(async function () {
for (let i = 0; i < num; i++) {
let title = getTitle(i, num);
let item = createUnsavedDataObject('item', { title });
item.addToCollection(collection.id);
await item.save();
}
});
// Scroll halfway
treebox.scrollToRow(Math.round(num / 2) - Math.round(numVisibleRows / 2));
// Select an item
itemsView.selection.select(Math.round(num / 2));
var selectedItem = itemsView.getSelectedItems()[0];
var offset = itemsView.getRowIndexByID(selectedItem.treeViewID) - treebox.getFirstVisibleRow();
// Add one item at the beginning
var item = createUnsavedDataObject(
'item', { title: getTitle(0, num), collections: [collection.id] }
);
await item.saveTx({
skipSelect: true
});
// Then add a few more in a transaction
await Zotero.DB.executeTransaction(async function () {
for (let i = 0; i < 3; i++) {
var item = createUnsavedDataObject(
'item', { title: getTitle(0, num), collections: [collection.id] }
);
await item.save({
skipSelect: true
});
}
});
// Make sure the selected item is still at the same position
assert.equal(itemsView.getSelectedItems()[0], selectedItem);
var newOffset = itemsView.getRowIndexByID(selectedItem.treeViewID) - treebox.getFirstVisibleRow();
assert.equal(newOffset, offset);
});
it("shouldn't scroll items list if at top when other items are added with skipSelect", async function () {
var collection = await createDataObject('collection');
await select(win, collection);
itemsView = zp.itemsView;
var treebox = itemsView._treebox;
var numVisibleRows = treebox.getLastVisibleRow() - treebox.getFirstVisibleRow();
// Get a numeric string left-padded with zeroes
function getTitle(i, max) {
return new String(new Array(max + 1).join(0) + i).slice(-1 * max);
}
var num = numVisibleRows + 10;
await Zotero.DB.executeTransaction(async function () {
// Start at "*1" so we can add items before
for (let i = 1; i < num; i++) {
let title = getTitle(i, num);
let item = createUnsavedDataObject('item', { title });
item.addToCollection(collection.id);
await item.save();
}
}.bind(this));
// Scroll to top
treebox.scrollToRow(0);
// Add one item at the beginning
var item = createUnsavedDataObject(
'item', { title: getTitle(0, num), collections: [collection.id] }
);
await item.saveTx({
skipSelect: true
});
// Then add a few more in a transaction
await Zotero.DB.executeTransaction(async function () {
for (let i = 0; i < 3; i++) {
var item = createUnsavedDataObject(
'item', { title: getTitle(0, num), collections: [collection.id] }
);
await item.save({
skipSelect: true
});
}
}.bind(this));
// Make sure the first row is still at the top
assert.equal(treebox.getFirstVisibleRow(), 0);
});
it("shouldn't scroll items list when a child item is moved to a parent further down", async function () {
var collection = await createDataObject('collection');
await select(win, collection);
itemsView = zp.itemsView;
var treebox = itemsView._treebox;
var numVisibleRows = treebox.getLastVisibleRow() - treebox.getFirstVisibleRow();
var num = numVisibleRows + 10;
var parentItem1 = await createDataObject('item', {
title: String(0).padStart(num, '0'),
collections: [collection.id]
});
var parentItem2 = await createDataObject('item', {
title: String(3).padStart(num, '0'),
collections: [collection.id]
});
await Zotero.DB.executeTransaction(async function () {
for (let i = 4; i < num; i++) {
let item = createUnsavedDataObject('item', {
title: String(i).padStart(num, '0'),
collections: [collection.id]
});
await item.save();
}
});
var attachment = await importFileAttachment('test.png', { parentItemID: parentItem1.id });
await waitForItemsLoad(win);
itemsView.expandAllRows(true);
// Scroll partway into a row, which should be preserved as is
treebox.scrollTo(7);
await itemsView.selectItem(attachment.id);
assert.isTrue(itemsView.tree.rowIsVisible(itemsView.getRowIndexByID(parentItem2.id)));
// Move the attachment to the parent below
attachment.parentItemID = parentItem2.id;
await attachment.saveTx();
await itemsView.waitForLoad();
assert.equal(
itemsView.getRowIndexByID(attachment.id),
itemsView.getRowIndexByID(parentItem2.id) + 1
);
assert.equal(treebox.scrollOffset, 7);
});
it("should update search results when items are added", async function () {
var search = await createDataObject('search');
await select(win, search);