Item pane scroll pos fix after switching from note (#4897)

Follow up to #4879
fix: #4276

chrome/content/zotero/elements/itemDetails.js

@@ -36,6 +36,12 @@
 		}
 	};
 
+	const waitDOMUpdate = async (timeout = 50) => {
+		return new Promise((resolve) => {
+			requestIdleCallback(resolve, { timeout });
+		});
+	};
+
 	const waitNoLongerThan = async (promise, ms = 1000) => {
 		return Promise.race([
 			promise,
@@ -269,6 +275,11 @@
 			this._restorePinnedPane();
 
 			let panes = this.getPanes();
+
+			// Unobserve intersection to prevent unwanted rendering.
+			// Checking flags in _handleIntersection is not reliable because it's async.
+			this._toggleIntersectionObserver(false);
+
 			for (let box of [this._header, ...panes]) {
 				box.editable = this.editable;
 				box.tabID = this.tabID;
@@ -291,7 +302,7 @@
 			this._paneParent.style.paddingBottom = '';
 			if (pinnedPaneElem) {
 				let paneID = pinnedPaneElem.dataset.pane;
-				this.scrollToPane(paneID, 'instant');
+				await this.scrollToPane(paneID, 'instant');
 				this.pinnedPane = paneID;
 			}
 			else {
@@ -310,10 +321,17 @@
 					continue;
 				}
 				await waitNoLongerThan(box.asyncRender(), 500);
+				// Make sure the layout is updated for next isPaneVisible check
+				await waitDOMUpdate();
 			}
+
 			if (this.item.id == item.id) {
 				this._isRendering = false;
 			}
+
+			// Re-enable intersection observer
+			this._toggleIntersectionObserver(true);
+
 			if (Zotero.test) {
 				resolve();
 			}
@@ -439,7 +457,9 @@
 		}
 
 		async scrollToPane(paneID, behavior = 'smooth') {
-			let pane = this.getEnabledPane(paneID);
+			let panes = this.getEnabledPanes();
+			let paneIndex = panes.findIndex(elem => elem.dataset.pane == paneID);
+			let pane = panes[paneIndex];
 			if (!pane) return null;
 
 			let scrollPromise;
@@ -450,6 +470,9 @@
 				return null;
 			}
 
+			// Temporarily disable intersection observer to prevent unwanted rendering
+			this._toggleIntersectionObserver(false);
+
 			// The pane should always be at the very top
 			// If there isn't enough stuff below it for it to be at the top, we add padding
 			// We use a ::before pseudo-element for this so that we don't need to add another level to the DOM
@@ -459,9 +482,31 @@
 				scrollPromise = this._waitForScroll();
 				scrollPromise.then(() => this._disableScrollHandler = false);
 			}
+			else {
+				// Wait for the next DOM update to make sure the height is updated before rendering
+				scrollPromise = waitDOMUpdate();
+			}
 			pane.scrollIntoView({ block: 'start', behavior });
 			pane.focus();
-			return scrollPromise;
+			await scrollPromise;
+
+			// Check current and following panes for async render
+			for (let i = paneIndex; i < panes.length; i++) {
+				let nextPane = panes[i];
+				// Stop if the pane is not visible anymore
+				if (!this.isPaneVisible(nextPane.dataset.pane)) {
+					break;
+				}
+				if (nextPane.asyncRender) {
+					await nextPane.asyncRender();
+					// Wait for the next DOM update to make sure the layout is updated
+					await waitDOMUpdate();
+				}
+			}
+
+			// Re-enable intersection observer
+			this._toggleIntersectionObserver(true);
+			return true;
 		}
 		
 		_makeSpaceForPane(pane) {
@@ -528,7 +573,17 @@
 				this._intersectionOb.disconnect();
 			}
 			this._intersectionOb = new IntersectionObserver(this._handleIntersection);
-			this.getPanes().forEach(elem => this._intersectionOb.observe(elem));
+			this._toggleIntersectionObserver(true);
+		}
+
+		_toggleIntersectionObserver(enabled) {
+			let panes = this.getPanes();
+			if (enabled) {
+				panes.forEach(elem => this._intersectionOb.observe(elem));
+			}
+			else {
+				panes.forEach(elem => this._intersectionOb.unobserve(elem));
+			}
 		}
 
 		_handleContainerScroll = () => {

chrome/content/zotero/elements/itemPane.js

@@ -151,8 +151,17 @@
 			return true;
 		}
 
-		renderItemPane(item) {
+		async renderItemPane(item) {
+			let previousMode = this.mode;
 			this.mode = "item";
+
+			// Fix https://forums.zotero.org/discussion/115450/zotero-7-beta-wrong-vertical-position-in-the-item-pane-after-switching-from-a-note
+			if (previousMode === "note") {
+				// Wait for DOM to update and then trigger item-details render
+				await new Promise((resolve) => {
+					requestIdleCallback(resolve, { timeout: 50 });
+				});
+			}
 			
 			this._itemDetails.editable = this.editable;
 			this._itemDetails.tabID = "zotero-pane";

chrome/content/zotero/elements/itemPaneSidenav.js

@@ -129,8 +129,6 @@
 		
 		_contextMenuTarget = null;
 
-		_disableScrollHandler = false;
-		
 		get container() {
 			return this._container;
 		}

test/content/support.js

@@ -446,7 +446,7 @@ async function waitNoLongerThan(promise, ms = 1000) {
 
 async function waitForScrollToPane(itemDetails, paneID) {
 	await itemDetails._renderPromise;
-	itemDetails.scrollToPane(paneID, "instant");
+	await itemDetails.scrollToPane(paneID, "instant");
 	// Wait for some frames or up to 150ms to ensure the pane is visible
 	await waitForFrames(5);
 }

test/tests/itemPaneTest.js

@@ -1795,4 +1795,47 @@ describe("Item pane", function () {
 			assert.isTrue(ZoteroContextPane.splitter.hidden);
 		});
 	});
+
+	describe("Libraries pane", function () {
+		let paneID = "libraries-collections";
+
+		beforeEach(function () {
+			Zotero.Prefs.set("panes.libraries-collections.open", true);
+			Zotero_Tabs.select("zotero-pane");
+		});
+
+		afterEach(function () {
+			Zotero_Tabs.select("zotero-pane");
+			Zotero_Tabs.closeAll();
+		});
+
+		it("should scroll to pinned pane after selection moves from note to item", async function () {
+			let itemDetails = ZoteroPane.itemPane._itemDetails;
+			let pane = itemDetails.getPane(paneID);
+
+			let item = await createDataObject('item');
+			await importPDFAttachment(item);
+
+			let note = await createDataObject('item', { itemType: 'note' });
+
+			await select(win, item);
+			await waitForScrollToPane(itemDetails, paneID);
+
+			itemDetails.pinnedPane = paneID;
+
+			await select(win, note);
+
+			await Zotero.Promise.delay(10);
+
+			await select(win, item);
+			await waitForScrollToPane(itemDetails, paneID);
+
+			// Should scroll to pinned pane
+			assert.isTrue(
+				Math.abs(pane.getBoundingClientRect().top - pane.parentElement.getBoundingClientRect().top) < 3
+			);
+
+			itemDetails.pinnedPane = "";
+		});
+	});
 });