diff --git a/chrome/content/zotero/elements/itemDetails.js b/chrome/content/zotero/elements/itemDetails.js index e2ac231c2d..57eb4d63ce 100644 --- a/chrome/content/zotero/elements/itemDetails.js +++ b/chrome/content/zotero/elements/itemDetails.js @@ -36,12 +36,6 @@ } }; - const waitDOMUpdate = async (timeout = 50) => { - return new Promise((resolve) => { - requestIdleCallback(resolve, { timeout }); - }); - }; - const waitNoLongerThan = async (promise, ms = 1000) => { return Promise.race([ promise, @@ -275,11 +269,6 @@ 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; @@ -302,7 +291,7 @@ this._paneParent.style.paddingBottom = ''; if (pinnedPaneElem) { let paneID = pinnedPaneElem.dataset.pane; - await this.scrollToPane(paneID, 'instant'); + this.scrollToPane(paneID, 'instant'); this.pinnedPane = paneID; } else { @@ -321,17 +310,10 @@ 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(); } @@ -457,14 +439,9 @@ } async scrollToPane(paneID, behavior = 'smooth') { - let panes = this.getEnabledPanes(); - let paneIndex = panes.findIndex(elem => elem.dataset.pane == paneID); - let pane = panes[paneIndex]; + let pane = this.getEnabledPane(paneID); if (!pane) return null; - // Wait for the next DOM update to make sure the scroll position is updated - await waitDOMUpdate(); - let scrollPromise; // If the itemPane is collapsed, just remember which pane needs to be scrolled to @@ -473,9 +450,6 @@ 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 @@ -485,31 +459,9 @@ 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(); - 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; + return scrollPromise; } _makeSpaceForPane(pane) { @@ -576,17 +528,7 @@ this._intersectionOb.disconnect(); } this._intersectionOb = new IntersectionObserver(this._handleIntersection); - 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)); - } + this.getPanes().forEach(elem => this._intersectionOb.observe(elem)); } _handleContainerScroll = () => { diff --git a/chrome/content/zotero/elements/itemPaneSidenav.js b/chrome/content/zotero/elements/itemPaneSidenav.js index 345fb15497..89b918f6ff 100644 --- a/chrome/content/zotero/elements/itemPaneSidenav.js +++ b/chrome/content/zotero/elements/itemPaneSidenav.js @@ -129,6 +129,8 @@ _contextMenuTarget = null; + _disableScrollHandler = false; + get container() { return this._container; } diff --git a/test/content/support.js b/test/content/support.js index 2d1aa5a9fb..07f3f7281c 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; - await itemDetails.scrollToPane(paneID, "instant"); + 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 c680d28d67..f828b9f2ce 100644 --- a/test/tests/itemPaneTest.js +++ b/test/tests/itemPaneTest.js @@ -1795,47 +1795,4 @@ 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 = ""; - }); - }); });