Citation dialog: do not wait for cited items to load (#5127)

Before making the dialog interactable.

Initialize loading cited items via io.getItems when the dialog appears.
Once they are loaded, itemsList is refreshed to include
cited matching items and all subsequent searches will include
them too. During the refresh immediately after cited items load,
try to preserve focused/selected state of items.

Added IOManager.preInit that is ran as soon as the dialog
is loaded. Move accept/cancel initialization there.
IOManager.init runs at the very end of onLoad because
it relies on most data and layouts being loaded,
It means if there is some delay or error during loading,
the dialog cannot be closed. To avoid it, setup
cancel/accept buttons separately in the beginning.

Also, minor refactoring of SearchHandler to handle
refreshing of cited items separately from selected and
open items.

Fixes: #5121

chrome/content/zotero/integration/citationDialog.js

@@ -67,6 +67,9 @@ async function onLoad() {
 	libraryLayout = new LibraryLayout();
 	listLayout = new ListLayout();
 
+	// initialize most essential IO functionality (e.g. accept/cancel)
+	// remaining listeners that rely on layouts being loaded are added later in IOManager.init
+	IOManager.preInit();
 	// top-level keypress handling and focus navigation across the dialog
 	// keypresses for lower-level bubble-specific behavior are handled in bubbleInput.js
 	doc.addEventListener("keydown", event => KeyboardHandler.handleKeydown(event));
@@ -80,9 +83,9 @@ async function onLoad() {
 	// init library layout after bubble input is built since bubble-input's height is a factor
 	// determining initial library layout height
 	await libraryLayout.init();
-	// fetch opened/selected/cited items so they are known
+	// fetch selected items so they are known
 	// before refreshing items list after dialog mode setting
-	await SearchHandler.refreshNonLibraryItems();
+	SearchHandler.refreshSelectedAndOpenItems();
 	// some nodes (e.g. item-tree-menu-bar) are expected to be present to switch modes
 	// so this has to go after all layouts are loaded
 	IOManager.setInitialDialogMode();
@@ -90,6 +93,12 @@ async function onLoad() {
 	IOManager.init();
 	// explicitly focus bubble input so one can begin typing right away
 	_id("bubble-input").refocusInput();
+	// loading cited items can take a long time - start loading them now
+	// and add new nodes when cited items are ready
+	SearchHandler.loadCitedItemsPromise.then(() => {
+		SearchHandler.refreshCitedItems();
+		currentLayout.refreshItemsList({ retainItemsState: true });
+	});
 
 	// Disabled all multiselect when citing notes
 	if (isCitingNotes) {
@@ -131,6 +140,7 @@ function cancel() {
 }
 
 function cleanupBeforeDialogClosing() {
+	if (!currentLayout || !libraryLayout) return;
 	Zotero.Prefs.set("integration.citationDialogLastClosedMode", currentLayout.type);
 	if (currentLayout.type == "library") {
 		Zotero.Prefs.set("integration.citationDialogCollectionLastSelected", libraryLayout.collectionsView.selectedTreeRow.id);
@@ -157,8 +167,9 @@ class Layout {
 		this._searchDebouncePromise = null;
 	}
 
-	// Re-render the items based on search rersults
-	async refreshItemsList() {
+	// Re-render the items based on search results
+	// @param {Boolean} options.retainItemsState: try to restore focused and selected status of item nodes.
+	async refreshItemsList({ retainItemsState } = {}) {
 		let sections = [];
 
 		// Tell SearchHandler which currently cited items are so they are not included in results
@@ -196,6 +207,14 @@ class Layout {
 				items.push(itemNode);
 				index++;
 			}
+			// if cited group is present but has no items, cited items must be
+			// still loading, so show a placeholder item card
+			if (group.length === 0 && key == "cited") {
+				let placeholder = Helpers.createCitedItemPlaceholder();
+				items = [placeholder];
+				let spinner = Helpers.createNode("image", { id: "cited-items-spinner", status: "animate" }, "zotero-spinner-16");
+				section.querySelector(".header .header-btn-group").prepend(spinner);
+			}
 			itemContainer.replaceChildren(...items);
 			sections.push(section);
 			if (isGroupCollapsible) {
@@ -215,14 +234,30 @@ class Layout {
 				}
 			}
 		}
-		let selectedNow = doc.activeElement;
+		let previouslyFocused = doc.activeElement;
+		let previouslySelected = doc.querySelectorAll(".item.selected");
 		_id(`${this.type}-layout`).querySelector(".search-items").replaceChildren(...sections);
 		// Update which bubbles need to be highlighted
 		this.updateSelectedItems();
+
+		// Keep focus and selection on the same item nodes if specified.
+		// This should only be applicable to refresh after SearchHandler.loadCitedItemsPromise.
+		if (retainItemsState) {
+			doc.getElementById(previouslyFocused.id)?.focus();
+			// Try to retain selected status of items, in case if multiselection was in progress
+			for (let oldNote of previouslySelected) {
+				let itemNode = doc.getElementById(oldNote.id);
+				if (!itemNode) continue;
+				itemNode.classList.add("selected");
+				itemNode.classList.toggle("current", oldNote.classList.contains("current"));
+			}
+		}
 		// Pre-select the item to be added on Enter of an input
-		this.markPreSelected();
-		// If the previously focused node is no longer a part of the DOM, try to restore focus
-		if (!doc.contains(selectedNow) || doc.activeElement.tagName == "body") {
+		else {
+			this.markPreSelected();
+		}
+		// Ensure focus is never lost
+		if (doc.activeElement.tagName == "body") {
 			IOManager._restorePreClickFocus();
 		}
 	}
@@ -238,10 +273,13 @@ class Layout {
 		_id("loading-spinner").setAttribute("status", "animate");
 		_id("accept-button").hidden = true;
 		SearchHandler.searching = true;
-		// search for selected/opened/cited items
+		// search for selected/opened items
 		// only enforce min query length in list mode
 		SearchHandler.setSearchValue(value, this.type == "list");
-		await SearchHandler.refreshNonLibraryItems();
+		SearchHandler.refreshSelectedAndOpenItems();
+		// noop if cited items are not yet loaded
+		SearchHandler.refreshCitedItems();
+		
 		// Never resize window of list layout here to avoid flickering
 		// The window will always be resized after the second items list update below
 		await this.refreshItemsList({ skipWindowResize: true });
@@ -301,7 +339,7 @@ class Layout {
 			itemNode.classList.remove("selected");
 			itemNode.classList.remove("current");
 		}
-		let firstItemNode = _id(`${currentLayout.type}-layout`).querySelector(`.item`);
+		let firstItemNode = _id(`${currentLayout.type}-layout`).querySelector(`.item:not([disabled])`);
 		if (!firstItemNode) return;
 		let activeSearch = SearchHandler.searchValue.length > 0;
 		let noBubbles = !CitationDataManager.items.length;
@@ -354,8 +392,8 @@ class LibraryLayout extends Layout {
 		return itemNode;
 	}
 
-	async refreshItemsList() {
-		await super.refreshItemsList();
+	async refreshItemsList(options) {
+		await super.refreshItemsList(options);
 		_id("library-other-items").querySelector(".search-items").hidden = !_id("library-layout").querySelector(".section:not([hidden])");
 		_id("library-no-suggested-items-message").hidden = !_id("library-other-items").querySelector(".search-items").hidden;
 		// When there are no matches, show a message
@@ -720,7 +758,7 @@ class ListLayout extends Layout {
 			"data-tabindex": 30,
 			"data-arrow-nav-enabled": true,
 			draggable: true
-		}, "item vbox keyboard-clickable");
+		}, "item keyboard-clickable");
 		let id = item.cslItemID || item.id;
 		itemNode.setAttribute("itemID", id);
 		itemNode.setAttribute("role", "option");
@@ -744,7 +782,7 @@ class ListLayout extends Layout {
 	}
 
 	async refreshItemsList(options = {}) {
-		await super.refreshItemsList();
+		await super.refreshItemsList(options);
 
 		// Hide padding of list layout if there is not a single item to show
 		let isEmpty = !_id("list-layout").querySelector(".section:not([hidden])");
@@ -848,6 +886,15 @@ class ListLayout extends Layout {
 const IOManager = {
 	sectionExpandedStatus: {},
 
+	// most essential IO functionality that is added immediately on load
+	preInit() {
+		_id("accept-button").addEventListener("click", accept);
+		_id("cancel-button").addEventListener("click", cancel);
+
+		doc.addEventListener("dialog-accepted", accept);
+		doc.addEventListener("dialog-cancelled", cancel);
+	},
+
 	init() {
 		// handle input receiving focus or something being typed
 		doc.addEventListener("handle-input", ({ detail: { query, eventType } }) => this._handleInput({ query, eventType }));
@@ -864,10 +911,6 @@ const IOManager = {
 		doc.addEventListener("select-items", ({ detail: { startNode, endNode } }) => this.selectItemNodesRange(startNode, endNode));
 		// update bubbles after citation item is updated by itemDetails popup
 		doc.addEventListener("item-details-updated", () => this.updateBubbleInput());
-		
-		// accept/cancel events emitted by keyboardHandler
-		doc.addEventListener("dialog-accepted", accept);
-		doc.addEventListener("dialog-cancelled", cancel);
 
 		doc.addEventListener("DOMMenuBarActive", () => this._handleMenuBarAppearance());
 
@@ -878,9 +921,6 @@ const IOManager = {
 
 		// open settings popup on btn click
 		_id("settings-button").addEventListener("click", event => _id("settings-popup").openPopup(event.target, "before_end"));
-		// handle accept/cancel buttons
-		_id("accept-button").addEventListener("click", accept);
-		_id("cancel-button").addEventListener("click", cancel);
 
 		// some additional logic to keep focus on relevant nodes during mouse interactions
 		this._initFocusRetention();

chrome/content/zotero/integration/citationDialog/helpers.mjs

@@ -137,15 +137,16 @@ export class CitationDialogHelpers {
 		let divider = this.createNode("div", {}, "divider");
 		headerSpan.innerText = headerText;
 		header.append(headerSpan);
-		let itemContainer = this.createNode("div", { role: "group", "aria-label": headerText }, "itemsContainer");
+		let itemContainer = this.createNode("div", { id: `${id}_container`, role: "group", "aria-label": headerText }, "itemsContainer");
 		section.append(header, itemContainer, divider);
 
+		let buttonGroup = this.createNode("div", {}, "header-btn-group");
+		header.append(buttonGroup);
+
 		if (isCollapsible) {
 			headerSpan.id = `header_${id}`;
 			section.classList.add("expandable");
 			section.style.setProperty('--deck-length', deckLength);
-			let buttonGroup = this.createNode("div", { }, "header-btn-group");
-			header.append(buttonGroup);
 
 			let addAllBtn = this.createNode("span", { tabindex: -1, 'data-tabindex': 22, role: "button", "aria-describedby": headerSpan.id }, "add-all keyboard-clickable");
 			buttonGroup.append(addAllBtn);
@@ -168,6 +169,20 @@ export class CitationDialogHelpers {
 		return section;
 	}
 
+	// Create mock item node to use as a the placeholder for cited items that are loading
+	createCitedItemPlaceholder() {
+		let itemNode = this.createNode("div", {
+			role: "option",
+			disabled: true
+		}, "item cited-placeholder");
+		let title = this.createNode("div", {}, "title");
+		let description = this.createNode("div", {}, "description");
+		title.textContent = Zotero.getString("general.loading");
+		description.textContent = " ";
+		itemNode.append(title, description);
+		return itemNode;
+	}
+
 	// Extract locator from a string and return an object: { label: string, page: string, onlyLocator: bool}
 	// to identify the locator and pass that info to the dialog
 	extractLocator(string) {

chrome/content/zotero/integration/citationDialog/keyboardHandler.mjs

@@ -65,7 +65,7 @@ export class CitationDialogKeyboardHandler {
 			let rowIndex = focusedRow.id.split("-")[4];
 			if (rowIndex !== "0") return;
 			// if there are suggested items, focus them
-			if (this._id("library-other-items").querySelector(".item")) {
+			if (this._id("library-other-items").querySelector(".item:not([disabled])")) {
 				let current = this.doc.querySelector(".selected.current");
 				if (current) {
 					current.focus();
@@ -128,7 +128,7 @@ export class CitationDialogKeyboardHandler {
 			if (current) {
 				current.focus();
 			}
-			else if (group.querySelector(".item")) {
+			else if (group.querySelector(".item:not([disabled])")) {
 				this._navigateGroup({ group, current: null, forward: true, shouldSelect: true, shouldFocus: true, multiSelect: false });
 			}
 			else if (this._id("zotero-items-tree").querySelector(".row")) {
@@ -148,7 +148,7 @@ export class CitationDialogKeyboardHandler {
 			// on arrowUp from the first row, clear selection
 			if (current === firstRow && event.key == "ArrowUp" && !event.shiftKey) {
 				this._selectItems(null);
-				firstRow.classList.remove("current");
+				firstRow?.classList.remove("current");
 				group.scrollTo(0, 0);
 				this._multiselectStart = null;
 			}

chrome/content/zotero/integration/citationDialog/searchHandler.mjs

@@ -45,7 +45,15 @@ export class CitationDialogSearchHandler {
 		this.minQueryLengthEnforced = false;
 		this.searching = false;
 		this.searchResultIDs = [];
-		this._nonLibraryItems = {};
+
+		// cache selected/open/cited items
+		this.selectedItems = null;
+		this.openItems = null;
+		this.citedItems = null;
+
+		this.loadCitedItemsPromise = this._getCitedItems().then((citedItems) => {
+			this.citedItems = citedItems;
+		});
 	}
 
 	setSearchValue(str, enforceMinQueryLength) {
@@ -69,10 +77,10 @@ export class CitationDialogSearchHandler {
 
 	// how many selected items there are without applying the filter
 	allSelectedItemsCount() {
-		if (this._nonLibraryItems.selected !== undefined) {
-			return this._nonLibraryItems.selected.length;
+		if (this.selectedItems === null) {
+			this.selectedItems = this._getSelectedLibraryItems();
 		}
-		return this._getSelectedLibraryItems().length;
+		return this.selectedItems.length;
 	}
 
 	// Return results in a more helpful formatfor rendering.
@@ -99,6 +107,12 @@ export class CitationDialogSearchHandler {
 			if (groupItems.length) {
 				result.push({ key: groupKey, group: groupItems });
 			}
+			// if cited items are being loaded, add their group with no items to indicate
+			// that a placeholder should be displayed
+			let loadingCitedItemsGroup = this.citedItems === null && groupKey === "cited";
+			if (loadingCitedItemsGroup && this.searchValue) {
+				result.push({ key: "cited", group: [] });
+			}
 		}
 		// library items go after
 		let libraryItems = Object.values(this.results.found.reduce((acc, item) => {
@@ -122,34 +136,23 @@ export class CitationDialogSearchHandler {
 		return result;
 	}
 
-	// Refresh selected/opened/cited items.
+	// Refresh selected/opened items.
 	// These items are searched for separately from actual library matches
 	// because it is much faster for large libraries, so we don't have to wait
 	// for the library search to complete to show these results.
-	async refreshNonLibraryItems() {
-		// Use cached selected/cited/open items if available to not
-		// re-fetch them every time
-		if (!Object.keys(this._nonLibraryItems).length) {
-			this._nonLibraryItems = {
-				open: this._getReaderOpenItems(),
-				cited: await this._getCitedItems(),
-				selected: this._getSelectedLibraryItems(),
-			};
+	refreshSelectedAndOpenItems() {
+		if (this.openItems === null) {
+			this.openItems = this._getReaderOpenItems();
+		}
+		if (this.selectedItems === null) {
+			this.selectedItems = this._getSelectedLibraryItems();
 		}
-		let { open, cited, selected } = this._nonLibraryItems;
 		
 		// apply filtering to item groups
-		this.results.open = this.searchValue ? this._filterNonMatchingItems(open) : open;
-		this.results.selected = this.searchValue ? this._filterNonMatchingItems(selected) : selected;
+		this.results.open = this.searchValue ? this._filterNonMatchingItems(this.openItems) : this.openItems;
+		this.results.selected = this.searchValue ? this._filterNonMatchingItems(this.selectedItems) : this.selectedItems;
 		// clear matching library items to make sure items stale results are not showing
 		this.results.found = [];
-		// if "ibid" is typed, return all cited items
-		if (this.searchValue.toLowerCase() === Zotero.getString("integration.ibid").toLowerCase()) {
-			this.results.cited = cited;
-		}
-		else {
-			this.results.cited = this.searchValue ? this._filterNonMatchingItems(cited) : [];
-		}
 		// Ensure duplicates across groups before library items are found
 		this._deduplicate();
 	}
@@ -165,10 +168,24 @@ export class CitationDialogSearchHandler {
 		this._deduplicate();
 	}
 
-	// clear selected/open/cited items cache to re-fetch those items
+	async refreshCitedItems() {
+		if (this.citedItems === null) {
+			return;
+		}
+		// if "ibid" is typed, return all cited items
+		if (this.searchValue.toLowerCase() === Zotero.getString("integration.ibid").toLowerCase()) {
+			this.results.cited = this.citedItems;
+		}
+		else {
+			this.results.cited = this.searchValue ? this._filterNonMatchingItems(this.citedItems) : [];
+		}
+	}
+
+	// clear selected/open items cache to re-fetch those items
 	// after they may have changed
 	clearNonLibraryItemsCache() {
-		this._nonLibraryItems = {};
+		this.selectedItems = null;
+		this.openItems = null;
 	}
 
 	cleanSearchQuery(str) {
@@ -183,17 +200,14 @@ export class CitationDialogSearchHandler {
 	}
 
 	// make sure that each item appears only in one group.
-	// Items that are selected are removed from opened and cited.
-	// Items that are opened are removed from cited.
-	// Items that are selected or opened or cited are removed from library results.
+	// Items that are selected are removed from opened.
+	// Items that are selected or opened are removed from library results.
 	_deduplicate() {
 		let selectedIDs = new Set(this.results.selected.map(item => item.id));
 		let openIDs = new Set(this.results.open.map(item => item.id));
-		let citedIDs = new Set(this.results.cited.filter(item => item.id).map(item => item.id));
 
 		this.results.open = this.results.open.filter(item => !selectedIDs.has(item.id));
-		this.results.cited = this.results.cited.filter(item => !selectedIDs.has(item.id) && !openIDs.has(item.id));
-		this.results.found = this.results.found.filter(item => !selectedIDs.has(item.id) && !openIDs.has(item.id) && !citedIDs.has(item.id));
+		this.results.found = this.results.found.filter(item => !selectedIDs.has(item.id) && !openIDs.has(item.id));
 	}
 		
 	// Run the actual search query and find all items matching query across all libraries

chrome/locale/en-US/zotero/integration.ftl

@@ -23,7 +23,10 @@ integration-quickFormatDialog-window =
 integration-citationDialog = Citation Dialog
 integration-citationDialog-section-open = Open Documents ({ $count })
 integration-citationDialog-section-selected = Selected Items ({ $count }/{ $total })
-integration-citationDialog-section-cited = Cited Items ({ $count })
+integration-citationDialog-section-cited = { $count ->
+    [0] Cited Items
+    *[other] Cited Items ({ $count })
+}
 integration-citationDialog-details-suffix = Suffix
 integration-citationDialog-details-prefix = Prefix
 integration-citationDialog-details-suppressAuthor = Omit Author

scss/components/_citationDialog.scss

@@ -81,12 +81,6 @@
 			#loading-spinner {
 				width: 28px;
 				height: 28px;
-				background-size: 60%;
-				background-repeat: no-repeat;
-				display: none;
-				&[status="animate"] {
-					display: inline-block;
-				}
 			}
 			.vertical-separator {
 				border-inline-end: 1px solid var(--fill-quarternary);
@@ -112,6 +106,20 @@
 		}
 	}
 
+	.zotero-spinner-16 {
+		background-size: 60%;
+		background-repeat: no-repeat;
+		display: none;
+		&[status="animate"] {
+			display: inline-block;
+		}
+	}
+
+	#cited-items-spinner {
+		width: 20px;
+		height: 20px;
+	}
+
 	// alternative to var(--accent-blue10) without opacity
 	// which causes issues with stacked item cards in library mode
 	--selected-item-background: #EBF0FC;
@@ -168,7 +176,11 @@
 						text-wrap: nowrap;
 						text-overflow: ellipsis;
 						overflow: hidden;
-						display: block;
+						display: flex;
+						.header-btn-group {
+							display: flex;
+							align-items: center;
+						}
 					}
 			
 					.itemsContainer {
@@ -230,8 +242,6 @@
 						width: calc((var(--item-horizontal-size) * var(--deck-length)) + (var(--item-margin) * (var(--deck-length) - 1)));
 	
 						.header {
-							display: flex;
-							justify-content: space-between;
 							.header-label {
 								// make sure header text does not overlap with buttons
 								max-width: 160px;
@@ -240,9 +250,6 @@
 							}
 
 							.header-btn-group {
-								display: flex;
-								flex-direction: row;
-								align-items: center;
 								gap: 4px;
 								flex: 1;
 								margin-inline-start: 2px;
@@ -294,7 +301,7 @@
 					// separate items get a hover effect, unless they are in
 					// a collapsed expandable group, in which case the whole group is hovered
 					&:not(.expandable), &.expandable.expanded {
-						.item:hover {
+						.item:not([disabled]):hover {
 							background-color: var(--color-quinary-on-background);
 						}
 					}
@@ -420,6 +427,12 @@
 					font-size: 13px;
 					color: var(--fill-secondary);
 					padding: 4px 8px 4px 16px;
+					display: flex;
+					.header-btn-group {
+						display: flex;
+						align-items: center;
+						height: 17px;
+					}
 				}
 				.item {
 					@include focus-ring(true);
@@ -429,7 +442,7 @@
 					color: var(--fill-primary);
 					cursor: default;
 					-moz-window-dragging: no-drag;
-					&:hover {
+					&:not([disabled]):hover {
 						background-color: var(--fill-quinary);
 					}
 					&.selected {
@@ -479,6 +492,7 @@
 						.header-label {
 							@include focus-ring;
 							border-radius: 5px;
+							-moz-window-dragging: no-drag;
 							&:hover {
 								cursor: pointer;
 								text-decoration: underline;