diff --git a/chrome/content/zotero/elements/itemDetails.js b/chrome/content/zotero/elements/itemDetails.js index 57eb4d63ce..e791f970cc 100644 --- a/chrome/content/zotero/elements/itemDetails.js +++ b/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 = () => { diff --git a/chrome/content/zotero/elements/itemPane.js b/chrome/content/zotero/elements/itemPane.js index 0dbc865b67..f95af15acf 100644 --- a/chrome/content/zotero/elements/itemPane.js +++ b/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"; diff --git a/chrome/content/zotero/elements/itemPaneSidenav.js b/chrome/content/zotero/elements/itemPaneSidenav.js index 89b918f6ff..345fb15497 100644 --- a/chrome/content/zotero/elements/itemPaneSidenav.js +++ b/chrome/content/zotero/elements/itemPaneSidenav.js @@ -129,8 +129,6 @@ _contextMenuTarget = null; - _disableScrollHandler = false; - get container() { return this._container; } diff --git a/test/content/support.js b/test/content/support.js index 07f3f7281c..2d1aa5a9fb 100644 --- a/test/content/support.js +++ b/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); } diff --git a/test/tests/itemPaneTest.js b/test/tests/itemPaneTest.js index f828b9f2ce..c680d28d67 100644 --- a/test/tests/itemPaneTest.js +++ b/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 = ""; + }); + }); });