From 85cade3fb295bd2f0b63cdab5f78a327818e3c55 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Sat, 26 Aug 2023 05:57:38 -0400 Subject: [PATCH] Make preferences less janky, preload panes on hover, allow panes to delay visibility until promise resolves (#3363) Prevents flashes of unlocalized labels and controls without values set. Makes switching panes feel speedier overall because of preloading. I thought there was an issue for the flashes of uninitialized content but can't find it now. --- .../content/zotero/preferences/preferences.js | 113 +++++++++++++----- .../zotero/preferences/preferences_cite.xhtml | 2 +- .../zotero/preferences/preferences_export.jsx | 8 +- .../preferences/preferences_export.xhtml | 2 +- test/content/support.js | 9 +- 5 files changed, 90 insertions(+), 44 deletions(-) diff --git a/chrome/content/zotero/preferences/preferences.js b/chrome/content/zotero/preferences/preferences.js index da999465a5..b78ea3e3ac 100644 --- a/chrome/content/zotero/preferences/preferences.js +++ b/chrome/content/zotero/preferences/preferences.js @@ -37,7 +37,8 @@ var Zotero_Preferences = { this.content = document.getElementById('prefs-content'); this.helpContainer = document.getElementById('prefs-help-container'); - this.navigation.addEventListener('select', () => this._onNavigationSelect()); + this.navigation.addEventListener('mouseover', event => this._handleNavigationMouseOver(event)); + this.navigation.addEventListener('select', () => this._handleNavigationSelect()); document.getElementById('prefs-search').addEventListener('command', event => this._search(event.target.value)); @@ -124,18 +125,28 @@ var Zotero_Preferences = { } }, - _onNavigationSelect() { - for (let child of this.content.children) { - if (child !== this.helpContainer) { - child.setAttribute('hidden', true); - } + async _handleNavigationMouseOver(event) { + if (event.target.tagName === 'richlistitem') { + await this._loadPane(event.target.value); } + }, + + async _handleNavigationSelect() { let paneID = this.navigation.value; if (paneID) { - this.content.scrollTop = 0; + let pane = this.panes.get(paneID); document.getElementById('prefs-search').value = ''; - this._search(''); - this._loadAndDisplayPane(paneID); + await this._search(''); + await this._showPane(paneID); + this.content.scrollTop = 0; + for (let child of this.content.children) { + if (child !== this.helpContainer && child !== pane.container) { + child.hidden = true; + } + } + + this.helpContainer.hidden = !pane.helpURL; + document.getElementById('prefs-subpane-back-button').hidden = !pane.parent; } Zotero.Prefs.set('lastSelectedPrefPane', paneID); }, @@ -201,20 +212,31 @@ var Zotero_Preferences = { this.panes.set(id, { ...options, - imported: false, + loaded: false, container, }); }, /** - * Display a pane's content, alongside any other panes already showing. - * If the pane is not yet loaded, it will be loaded first. + * Load a pane if not already loaded. * * @param {String} id + * @return {Promise} */ - _loadAndDisplayPane(id) { + async _loadPane(id) { let pane = this.panes.get(id); - if (!pane.imported) { + if (pane.loaded) { + return; + } + if (pane.loadPromise) { + await pane.loadPromise; + return; + } + + let rest = async () => { + // Hack - make sure the following code does not run synchronously so we can set loadPromise immediately + await Zotero.Promise.delay(); + if (pane.scripts) { for (let script of pane.scripts) { Services.scriptloader.loadSubScript(script, window); @@ -237,17 +259,34 @@ var Zotero_Preferences = { ? MozXULElement.parseXULToFragment(markup, dtdFiles) : this._parseXHTMLToFragment(markup, dtdFiles); contentFragment = document.importNode(contentFragment, true); + this._initImportedNodesPreInsert(contentFragment); pane.container.append(contentFragment); - pane.imported = true; - this._initImportedNodesPostInsert(pane.container); - } + + await document.l10n.ready; + await document.l10n.translateFragment(pane.container); + await this._initImportedNodesPostInsert(pane.container); + + pane.loaded = true; + }; + pane.loadPromise = rest(); + await pane.loadPromise; + }, + + /** + * Display a pane's content, alongside any other panes already showing. + * If the pane is not yet loaded, it will be loaded first. + * + * @param {String} id + * @return {Promise} + */ + async _showPane(id) { + await this._loadPane(id); + + let pane = this.panes.get(id); pane.container.hidden = false; pane.container.children[0].dispatchEvent(new Event('showing')); - - let backButton = document.getElementById('prefs-subpane-back-button'); - backButton.hidden = !pane.parent; }, _parseXHTMLToFragment(str, entities = []) { @@ -334,10 +373,10 @@ ${str} * @param {Element} container * @private */ - _initImportedNodesPostInsert(container) { + async _initImportedNodesPostInsert(container) { let attachToPreference = (elem) => { if (this._observerSymbols.has(elem)) { - return; + return Promise.resolve(); } let preference = elem.getAttribute('preference'); @@ -382,9 +421,10 @@ ${str} elem.addEventListener('change', this._syncToPrefOnModify.bind(this)); // Set timeout before populating the value so the pane can add listeners first - setTimeout(() => { + return new Promise(resolve => setTimeout(() => { this._syncFromPref(elem, elem.getAttribute('preference')); - }); + resolve(); + })); }; let detachFromPreference = (elem) => { @@ -395,9 +435,13 @@ ${str} } }; + let awaitBeforeShowing = []; + // Activate `preference` attributes + // Do not await anything between here and the 'load' event dispatch below! That would cause 'syncfrompreference' + // events to be fired before 'load'! for (let elem of container.querySelectorAll('[preference]')) { - attachToPreference(elem); + awaitBeforeShowing.push(attachToPreference(elem)); } new MutationObserver((mutations) => { @@ -406,6 +450,7 @@ ${str} let target = mutation.target; detachFromPreference(target); if (target.hasAttribute('preference')) { + // Don't bother awaiting these attachToPreference(target); } } @@ -443,9 +488,15 @@ ${str} } for (let child of container.children) { - child.dispatchEvent(new Event('load')); + let event = new Event('load'); + event.waitUntil = (promise) => { + awaitBeforeShowing.push(promise); + }; + child.dispatchEvent(event); } + await Promise.allSettled(awaitBeforeShowing); + // If this is the first pane to be loaded, notify anyone waiting // (for tests) this._firstPaneLoadDeferred.resolve(); @@ -461,7 +512,7 @@ ${str} * * @param {String} [term] */ - _search(term) { + _search: Zotero.Utilities.Internal.serial(async function (term) { // Initial housekeeping: // Clear existing highlights @@ -480,11 +531,13 @@ ${str} hidden.ariaHidden = false; } + // Hide help button by default - _handleNavigationSelect() will show it when appropriate + this.helpContainer.hidden = true; + if (!term) { if (this.navigation.selectedIndex == -1) { this.navigation.selectedIndex = 0; } - this.helpContainer.hidden = !this.panes.get(this.navigation.value)?.helpURL; return; } @@ -500,7 +553,7 @@ ${str} pane.container.hidden = true; } else { - this._loadAndDisplayPane(id); + await this._showPane(id); } } @@ -580,7 +633,7 @@ ${str} } } } - }, + }), /** * Search for the given term (case-insensitive) in the tree. diff --git a/chrome/content/zotero/preferences/preferences_cite.xhtml b/chrome/content/zotero/preferences/preferences_cite.xhtml index aa0f9dac68..3d1b28353a 100644 --- a/chrome/content/zotero/preferences/preferences_cite.xhtml +++ b/chrome/content/zotero/preferences/preferences_cite.xhtml @@ -23,7 +23,7 @@ ***** END LICENSE BLOCK ***** --> - + &zotero.preferences.cite.styles; diff --git a/chrome/content/zotero/preferences/preferences_export.jsx b/chrome/content/zotero/preferences/preferences_export.jsx index 60ccff9d78..2a6db92ebf 100644 --- a/chrome/content/zotero/preferences/preferences_export.jsx +++ b/chrome/content/zotero/preferences/preferences_export.jsx @@ -31,11 +31,11 @@ var VirtualizedTable = require('components/virtualized-table'); var { makeRowRenderer } = VirtualizedTable; Zotero_Preferences.Export = { - init: Zotero.Promise.coroutine(function* () { + init: async function () { this.updateQuickCopyInstructions(); - yield this.populateQuickCopyList(); - yield this.populateNoteQuickCopyList(); - }), + await this.populateQuickCopyList(); + await this.populateNoteQuickCopyList(); + }, getQuickCopyTranslators: async function () { diff --git a/chrome/content/zotero/preferences/preferences_export.xhtml b/chrome/content/zotero/preferences/preferences_export.xhtml index c11d360ba8..33285579d6 100644 --- a/chrome/content/zotero/preferences/preferences_export.xhtml +++ b/chrome/content/zotero/preferences/preferences_export.xhtml @@ -23,7 +23,7 @@ ***** END LICENSE BLOCK ***** --> - + diff --git a/test/content/support.js b/test/content/support.js index ca523c96c8..e6c8ae75f3 100644 --- a/test/content/support.js +++ b/test/content/support.js @@ -113,14 +113,7 @@ var loadPrefPane = async function (paneName) { var win = await loadWindow("chrome://zotero/content/preferences/preferences.xhtml", { pane: id }); - var doc = win.document; - while (true) { - var pane = doc.getElementById(id); - if (pane) { - break; - } - await delay(1); - } + await win.Zotero_Preferences.waitForFirstPaneLoad(); return win; };