diff --git a/chrome/content/zotero/integration/citationDialog.js b/chrome/content/zotero/integration/citationDialog.js index 2005cf1018..349734332f 100644 --- a/chrome/content/zotero/integration/citationDialog.js +++ b/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(); diff --git a/chrome/content/zotero/integration/citationDialog/helpers.mjs b/chrome/content/zotero/integration/citationDialog/helpers.mjs index 848e55bb1b..4e4e1339ab 100644 --- a/chrome/content/zotero/integration/citationDialog/helpers.mjs +++ b/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) { diff --git a/chrome/content/zotero/integration/citationDialog/keyboardHandler.mjs b/chrome/content/zotero/integration/citationDialog/keyboardHandler.mjs index 932643c9d9..6f37569880 100644 --- a/chrome/content/zotero/integration/citationDialog/keyboardHandler.mjs +++ b/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; } diff --git a/chrome/content/zotero/integration/citationDialog/searchHandler.mjs b/chrome/content/zotero/integration/citationDialog/searchHandler.mjs index 872f5928a0..2ffe06e88a 100644 --- a/chrome/content/zotero/integration/citationDialog/searchHandler.mjs +++ b/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 diff --git a/chrome/locale/en-US/zotero/integration.ftl b/chrome/locale/en-US/zotero/integration.ftl index e2cd3ff4cf..8f29d6b2bb 100644 --- a/chrome/locale/en-US/zotero/integration.ftl +++ b/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 diff --git a/scss/components/_citationDialog.scss b/scss/components/_citationDialog.scss index a17b2f7d50..91270158a1 100644 --- a/scss/components/_citationDialog.scss +++ b/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;