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

Merge 4a3566eb30 into 53ded5eb44

Browse source
This commit is contained in:
Dan Stillman 2026-09-25 14:13:44 -04:00 • committed by GitHub
commit 17889ae653
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 230 additions and 200 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
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);
}
/**
@ -1557,7 +1557,7 @@ class VirtualizedTable extends React.Component {
let currentIndex = -1;
let nextIndex = -1;
for (let index of headerIndices) {
if (this._jsWindow._getItemPosition(index) <= scrollTop) {
if (this._jsWindow.getRowPosition(index) <= scrollTop) {
currentIndex = index;
}
else {
@ -1568,7 +1568,7 @@ class VirtualizedTable extends React.Component {
// Show the pinned copy only once the header row has scrolled up past the top edge of
// the view
let stuck = currentIndex != -1
&& scrollTop > this._jsWindow._getItemPosition(currentIndex);
&& scrollTop > this._jsWindow.getRowPosition(currentIndex);
if (!stuck) {
clip.style.display = 'none';
this._stickyHeaderIndex = null;
@ -1597,7 +1597,7 @@ class VirtualizedTable extends React.Component {
// Push the pinned header up as the next section's header approaches the top
let translateY = 0;
if (nextIndex != -1) {
let nextTop = this._jsWindow._getItemPosition(nextIndex) - scrollTop;
let nextTop = this._jsWindow.getRowPosition(nextIndex) - scrollTop;
if (nextTop < this._rowHeight) {
translateY = nextTop - this._rowHeight;
}

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

@ -102,7 +102,7 @@ module.exports = class {
if (!this._renderedRows.has(index)) return;
let oldElem = this._renderedRows.get(index);
let elem = this.renderItem(index, oldElem);
elem.style.top = this._getItemPosition(index) + "px";
elem.style.top = this.getRowPosition(index) + "px";
elem.style.position = "absolute";
if (elem == oldElem) return;
this.innerElem.replaceChild(elem, this._renderedRows.get(index));
@ -138,7 +138,7 @@ module.exports = class {
for (let index = startIndex; index < stopIndex; index++) {
if (this._renderedRows.has(index)) continue;
let elem = renderItem(index);
elem.style.top = this._getItemPosition(index) + "px";
elem.style.top = this.getRowPosition(index) + "px";
elem.style.position = "absolute";
innerElem.appendChild(elem);
this._renderedRows.set(index, elem);
@ -203,27 +203,18 @@ 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();
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;
}
let startPosition = this.getRowPosition(index);
let endPosition = this.getRowPosition(index + 1);
if (startPosition - topOffset < scrollOffset) {
this.scrollTo(startPosition - topOffset);
}
@ -245,12 +236,26 @@ module.exports = class {
return Math.max(1, offsetIdx + Math.ceil(((this.scrollOffset + height + 1) - offset) / this.itemHeight)) - 1;
}
_getItemPosition = (index) => {
/**
* Return the position of the top of a row relative to the top of the list
*
* @param {Integer} index
* @return {Integer}
*/
getRowPosition = (index) => {
const idx = this._binarySearchOffsets(this._rowOffsets, index);
const [offsetIdx, offset] = this._rowOffsets[idx];
return offset + (this.itemHeight * (index - offsetIdx));
};
/**
* @deprecated Use getRowPosition()
*/
_getItemPosition = (index) => {
Zotero.warn('windowed-list _getItemPosition() is deprecated -- use getRowPosition()');
return this.getRowPosition(index);
};
_getRangeToRender() {
const { overscanCount, scrollDirection } = this;
const itemCount = this._getItemCount();

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

@ -1230,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 });
}
/**
@ -1352,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,55 +2708,58 @@ var ItemTree = class ItemTree extends LibraryTree {
scrollPosition = this._cachedScrollPosition;
this._cachedScrollPosition = null;
}
if (!scrollPosition || !scrollPosition.id || !this._treebox) {
if (!scrollPosition || !this._treebox) {
return;
}
var row = this._rowMap[scrollPosition.id];
if (scrollPosition.id === undefined) {
this._treebox.scrollTo(scrollPosition.offset);
return;
}
let row = this._rowMap[scrollPosition.id];
if (row === undefined) {
return;
}
this._treebox.scrollToRow(Math.max(row - scrollPosition.offset, 0), true);
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 .id (a treeViewID) and .offset, or false if no rows
* 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();
for (let i = first; i <= last; i++) {
// If an object is selected, keep the first selected one in position
if (this.selection.isSelected(i)) {
let row = this.getRow(i);
if (!row) return false;
return {
id: row.ref.treeViewID,
offset: i - first
};
let scrollOffset = treebox.scrollOffset;
if (scrollOffset == 0) {
return { offset: 0 };
}
let anchorIndex = first;
if (preserveSelectionScroll) {
for (let i = first; i <= treebox.getLastVisibleRow(); i++) {
if (this.selection.isSelected(i)) {
anchorIndex = i;
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 (!first) {
return false;
}
// Otherwise keep the first visible row in position
let row = this.getRow(first);
let row = this.getRow(anchorIndex);
if (!row) return false;
return {
id: row.ref.treeViewID,
offset: 0
offset: treebox.getRowPosition(anchorIndex) - scrollOffset
};
}

308
test/tests/collectionViewItemTreeTest.js
View file

@ -757,6 +757,48 @@ describe("CollectionViewItemTree", function () {
assert.equal(treebox.getFirstVisibleRow(), firstVisibleBefore);
assert.isFalse(itemsView.tree.rowIsVisible(itemsView.getRowIndexByID(selectedItemID)));
});
it("shouldn't scroll when opening a container above the selected row", async function () {
var collection = await createDataObject('collection');
await select(win, collection);
itemsView = zp.itemsView;
var treebox = itemsView._treebox;
var numVisibleRows = treebox.getLastVisibleRow() - treebox.getFirstVisibleRow();
// Sort the container to the top, with more rows below than fit in the view
var num = numVisibleRows + 10;
var parentItem = await createDataObject('item', {
title: String(0).padStart(num, '0'),
collections: [collection.id]
});
await importFileAttachment('test.png', { parentItemID: parentItem.id });
await Zotero.DB.executeTransaction(async function () {
for (let i = 1; i < num; i++) {
let item = createUnsavedDataObject('item', {
title: String(i).padStart(num, '0'),
collections: [collection.id]
});
await item.save();
}
});
await waitForItemsLoad(win);
var parentRow = itemsView.getRowIndexByID(parentItem.id);
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);
assert.isFalse(itemsView.isContainerOpen(parentRow));
await itemsView.toggleOpenState(parentRow);
await itemsView.waitForLoad();
assert.isTrue(itemsView.isContainerOpen(parentRow));
assert.equal(treebox.scrollOffset, scrollOffsetBefore);
assert.isTrue(itemsView.tree.rowIsVisible(parentRow));
});
});
describe("#sort()", function () {
@ -1079,163 +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] }
);
await item.save({
skipSelect: true
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();
}
});
}
}.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);
}
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 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
});
}
});
// 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);
assert.isAbove(treebox.scrollOffset, 7);
});
}
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("should update search results when items are added", async function () {
var search = await createDataObject('search');
await select(win, search);