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

Keep fractional item tree scroll position when restoring it

Browse source 
Restoring by row index snapped the list to a row boundary after a
change, so a partly scrolled row jumped into place. Anchor on the row's
pixel offset instead, which also makes windowed-list's forceScrollToTop
unnecessary.
This commit is contained in:
Dan Stillman 2026-09-17 11:09:52 -04:00
parent 210c67ec10
commit e9ffaf0457
4 changed files with 25 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
chrome/content/zotero/components/virtualized-table.jsx
View file

@ -898,7 +898,7 @@ class VirtualizedTable extends React.Component {
&& this._getSectionHeaderIndices().some(i => i < index)) {
topOffset = this._rowHeight;
}
this._jsWindow.scrollToRow(index, false, topOffset);
this._jsWindow.scrollToRow(index, topOffset);
}
/**

21
chrome/content/zotero/components/windowed-list.js
View file

@ -203,13 +203,11 @@ module.exports = class {
/**
* Scroll the scrollbox to a specified item. No-op if already in view
* @param {Integer} index
* @param {Boolean} forceScrollToTop If true, the row will be scrolled to the top of the scrollbox
* even if it is below the current scroll window.
* @param {Integer} topOffset Amount of space reserved at the top of the scrollbox (e.g. for a
* sticky section header that overlays the rows). When scrolling a row into view from above, the
* row is positioned below this offset rather than flush with the top edge.
*/
scrollToRow(index, forceScrollToTop = false, topOffset = 0) {
scrollToRow(index, topOffset = 0) {
const { scrollOffset } = this;
const itemCount = this._getItemCount();
const height = this.getWindowHeight();
@ -217,13 +215,6 @@ module.exports = class {
index = Math.max(0, Math.min(index, itemCount - 1));
let startPosition = this._getItemPosition(index);
let endPosition = this._getItemPosition(index + 1);
// If forceScrollToTop is set, always scroll to the start position even if the row is
// already visible. This is used when restoring scroll position, where we need an exact
// first-visible-row rather than just ensuring the row is within view.
if (forceScrollToTop) {
this.scrollTo(startPosition);
return;
}
if (startPosition - topOffset < scrollOffset) {
this.scrollTo(startPosition - topOffset);
}
@ -232,6 +223,16 @@ module.exports = class {
}
}
/**
* Return the position of the top of a row relative to the top of the list
*
* @param {Integer} index
* @return {Integer}
*/
getRowPosition(index) {
return this._getItemPosition(index);
}
getFirstVisibleRow() {
const idx = this._binarySearchOffsets(this._rowOffsets, this.scrollOffset, true);
const [offsetIdx, offset] = this._rowOffsets[idx];

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

@ -2714,7 +2714,7 @@ var ItemTree = class ItemTree extends LibraryTree {
if (row === undefined) {
continue;
}
this._treebox.scrollToRow(Math.max(row - anchor.offset, 0), true);
this._treebox.scrollTo(this._treebox.getRowPosition(row) - anchor.offset);
return;
}
}
@ -2723,7 +2723,9 @@ var ItemTree = class ItemTree extends LibraryTree {
* 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, or false if there's nothing to anchor to
* 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
*/
_saveScrollPosition() {
if (!this._treebox) return false;
@ -2733,6 +2735,7 @@ var ItemTree = class ItemTree extends LibraryTree {
return false;
}
var last = treebox.getLastVisibleRow();
var scrollOffset = treebox.scrollOffset;
// If an object is selected, keep the first selected one in position
var selection = null;
@ -2742,7 +2745,7 @@ var ItemTree = class ItemTree extends LibraryTree {
if (row) {
selection = {
id: row.ref.treeViewID,
offset: i - first
offset: treebox.getRowPosition(i) - scrollOffset
};
}
break;
@ -2764,7 +2767,7 @@ var ItemTree = class ItemTree extends LibraryTree {
if (firstRow) {
viewport = {
id: firstRow.ref.treeViewID,
offset: 0
offset: treebox.getRowPosition(first) - scrollOffset
};
}

12
test/tests/collectionViewItemTreeTest.js
View file

@ -785,8 +785,8 @@ describe("CollectionViewItemTree", function () {
await waitForItemsLoad(win);
var parentRow = itemsView.getRowIndexByID(parentItem.id);
treebox.scrollToRow(parentRow);
var firstVisibleBefore = treebox.getFirstVisibleRow();
treebox.scrollTo(treebox.getRowPosition(parentRow));
var scrollOffsetBefore = treebox.scrollOffset;
// Select a visible row below the container
await itemsView.selectItem(itemsView.getRow(parentRow + 2).ref.id);
@ -796,7 +796,7 @@ describe("CollectionViewItemTree", function () {
await itemsView.waitForLoad();
assert.isTrue(itemsView.isContainerOpen(parentRow));
assert.equal(treebox.getFirstVisibleRow(), firstVisibleBefore);
assert.equal(treebox.scrollOffset, scrollOffsetBefore);
assert.isTrue(itemsView.tree.rowIsVisible(parentRow));
});
});
@ -1308,9 +1308,9 @@ describe("CollectionViewItemTree", function () {
await waitForItemsLoad(win);
itemsView.expandAllRows(true);
treebox.scrollToRow(0);
// Scroll partway into a row, which should be preserved as is
treebox.scrollTo(7);
await itemsView.selectItem(attachment.id);
var firstVisibleBefore = treebox.getFirstVisibleRow();
assert.isTrue(itemsView.tree.rowIsVisible(itemsView.getRowIndexByID(parentItem2.id)));
// Move the attachment to the parent below
@ -1322,7 +1322,7 @@ describe("CollectionViewItemTree", function () {
itemsView.getRowIndexByID(attachment.id),
itemsView.getRowIndexByID(parentItem2.id) + 1
);
assert.equal(treebox.getFirstVisibleRow(), firstVisibleBefore);
assert.equal(treebox.scrollOffset, 7);
});
it("should update search results when items are added", async function () {