From 90da2563c857d0f499bcf2a66354ee5cb8a18d0f Mon Sep 17 00:00:00 2001 From: Tom Najdek Date: Wed, 2 Feb 2022 23:15:57 +0100 Subject: [PATCH] Decouple progress queue table from progress window (#2318) * Tweaked ProgressQueue to accept multiple listeners per event. Removing listeners now requires specifying which one to remove. * Decoupled progress queue table into new ProgressQueueTable component --- .../zotero/components/progressQueueTable.jsx | 123 ++++++++++++++++++ chrome/content/zotero/progressQueueDialog.jsx | 78 ++--------- chrome/content/zotero/xpcom/attachments.js | 10 +- chrome/content/zotero/xpcom/progressQueue.js | 67 +++++----- .../zotero/xpcom/progressQueueDialog.js | 23 +--- 5 files changed, 177 insertions(+), 124 deletions(-) create mode 100644 chrome/content/zotero/components/progressQueueTable.jsx diff --git a/chrome/content/zotero/components/progressQueueTable.jsx b/chrome/content/zotero/components/progressQueueTable.jsx new file mode 100644 index 0000000000..4406080979 --- /dev/null +++ b/chrome/content/zotero/components/progressQueueTable.jsx @@ -0,0 +1,123 @@ +/* + ***** BEGIN LICENSE BLOCK ***** + + Copyright © 2022 Corporation for Digital Scholarship + Vienna, Virginia, USA + https://digitalscholar.org + + This file is part of Zotero. + + Zotero is free software: you can redistribute it and/or modify + it under the terms of the GNU Affero General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + Zotero is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Affero General Public License for more details. + + You should have received a copy of the GNU Affero General Public License + along with Zotero. If not, see . + + ***** END LICENSE BLOCK ***** +*/ +import React, { memo, useCallback, useEffect, useRef } from 'react'; +import PropTypes from 'prop-types'; +import { getDOMElement } from 'components/icons'; +import { IntlProvider } from 'react-intl'; + +import VirtualizedTable, { renderCell } from 'components/virtualized-table'; +import { noop } from './utils'; + + +function getImageByStatus(status) { + if (status === Zotero.ProgressQueue.ROW_PROCESSING) { + return getDOMElement('IconArrowRefresh'); + } + else if (status === Zotero.ProgressQueue.ROW_FAILED) { + return getDOMElement('IconCross'); + } + else if (status === Zotero.ProgressQueue.ROW_SUCCEEDED) { + return getDOMElement('IconTick'); + } + return document.createElementNS("http://www.w3.org/1999/xhtml", 'span'); +} + +const ProgressQueueTable = ({ onActivate = noop, progressQueue }) => { + const treeRef = useRef(null); + + const getRowCount = useCallback(() => progressQueue.getRows().length, [progressQueue]); + + const rowToTreeItem = useCallback((index, selection, oldDiv = null, columns) => { + let rows = progressQueue.getRows(); + let row = rows[index]; + + let div; + if (oldDiv) { + div = oldDiv; + div.innerHTML = ""; + } + else { + div = document.createElementNS("http://www.w3.org/1999/xhtml", 'div'); + div.className = "row"; + } + + div.classList.toggle('selected', selection.isSelected(index)); + + for (let column of columns) { + if (column.dataKey === 'success') { + let span = document.createElementNS("http://www.w3.org/1999/xhtml", 'span'); + span.className = `cell icon ${column.className}`; + span.appendChild(getImageByStatus(row.status)); + div.appendChild(span); + } + else { + div.appendChild(renderCell(index, row[column.dataKey], column)); + } + } + return div; + }, [progressQueue]); + + const columns = progressQueue.getColumns(); + + const tableColumns = [ + { dataKey: 'success', fixedWidth: true, width: "26" }, + { dataKey: 'fileName', label: Zotero.getString(columns[0]) }, + { dataKey: 'message', label: Zotero.getString(columns[1]) }, + ]; + + const refreshTree = useCallback(() => treeRef.current.invalidate(), []); + + useEffect(() => { + progressQueue.addListener('rowadded', refreshTree); + progressQueue.addListener('rowupdated', refreshTree); + progressQueue.addListener('rowdeleted', refreshTree); + return () => { + progressQueue.removeListener('rowadded', refreshTree); + progressQueue.removeListener('rowupdated', refreshTree); + progressQueue.removeListener('rowdeleted', refreshTree); + }; + }, []); // eslint-disable-line react-hooks/exhaustive-deps + + return ( + + + + ); +}; + +ProgressQueueTable.propTypes = { + onActivate: PropTypes.func, + progressQueue: PropTypes.object.isRequired +}; + +export default memo(ProgressQueueTable); diff --git a/chrome/content/zotero/progressQueueDialog.jsx b/chrome/content/zotero/progressQueueDialog.jsx index 006a77a98c..0e42fd23d8 100644 --- a/chrome/content/zotero/progressQueueDialog.jsx +++ b/chrome/content/zotero/progressQueueDialog.jsx @@ -26,86 +26,24 @@ Components.utils.import("resource://gre/modules/Services.jsm"); import React from 'react'; import ReactDOM from 'react-dom'; -import VirtualizedTable from 'components/virtualized-table'; -const { IntlProvider } = require('react-intl'); -const { getDOMElement } = require('components/icons'); -const { renderCell } = VirtualizedTable; +import ProgressQueueTable from 'components/progressQueueTable'; -let _progressIndicator = null; let _progressQueue; -let _tree; - -function _getImageByStatus(status) { - if (status === Zotero.ProgressQueue.ROW_PROCESSING) { - return getDOMElement('IconArrowRefresh'); - } - else if (status === Zotero.ProgressQueue.ROW_FAILED) { - return getDOMElement('IconCross'); - } - else if (status === Zotero.ProgressQueue.ROW_SUCCEEDED) { - return getDOMElement('IconTick'); - } - return document.createElementNS("http://www.w3.org/1999/xhtml", 'span'); -} - -function _rowToTreeItem(index, selection, oldDiv=null, columns) { - let rows = _progressQueue.getRows(); - let row = rows[index]; - - let div; - if (oldDiv) { - div = oldDiv; - div.innerHTML = ""; - } - else { - div = document.createElementNS("http://www.w3.org/1999/xhtml", 'div'); - div.className = "row"; - } - - div.classList.toggle('selected', selection.isSelected(index)); - - for (let column of columns) { - if (column.dataKey === 'success') { - let span = document.createElementNS("http://www.w3.org/1999/xhtml", 'span'); - span.className = `cell icon ${column.className}`; - span.appendChild(_getImageByStatus(row.status)); - div.appendChild(span); - } - else { - div.appendChild(renderCell(index, row[column.dataKey], column)); - } - } - return div; -} function _init() { var io = window.arguments[0]; _progressQueue = io.progressQueue; document.title = Zotero.getString(_progressQueue.getTitle()); - - let columns = _progressQueue.getColumns(); - - const tableColumns = [ - { dataKey: 'success', fixedWidth: true, width: "26" }, - { dataKey: 'fileName', label: Zotero.getString(columns[0]) }, - { dataKey: 'message', label: Zotero.getString(columns[1]) }, - ]; const domEl = document.querySelector('#tree'); - let elem = ( - - _progressQueue.getRows().length} - id="locateManager-table" - ref={ref => io.tree = _tree = ref} - renderItem={_rowToTreeItem} - showHeader={true} - columns={tableColumns} - onActivate={_handleActivate} - /> - + + ReactDOM.render( + , + domEl ); - ReactDOM.render(elem, domEl); } /** diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js index b5fb48e1f2..433cfc11e2 100644 --- a/chrome/content/zotero/xpcom/attachments.js +++ b/chrome/content/zotero/xpcom/attachments.js @@ -1478,6 +1478,7 @@ Zotero.Attachments = new function(){ this.addAvailablePDFs = async function (items, options = {}) { const MAX_CONSECUTIVE_DOMAIN_FAILURES = 5; const SAME_DOMAIN_REQUEST_DELAY = options.sameDomainRequestDelay || 1000; + var queue; var domains = new Map(); function getDomainInfo(domain) { @@ -1502,8 +1503,10 @@ Zotero.Attachments = new function(){ 'general.pdf' ] }); + progressQueue.addListener('cancel', () => queue = []); } - var queue = _findPDFQueue; + + queue = _findPDFQueue; for (let item of items) { // Skip items that aren't eligible. This is sort of weird, because it means some @@ -1572,11 +1575,6 @@ Zotero.Attachments = new function(){ queueResolve = resolve; }); - // Only one listener can be added, so we just add each time - progressQueue.addListener('cancel', () => { - queue = []; - }); - // // Process items in the queue // diff --git a/chrome/content/zotero/xpcom/progressQueue.js b/chrome/content/zotero/xpcom/progressQueue.js index 91461044d0..e39b370860 100644 --- a/chrome/content/zotero/xpcom/progressQueue.js +++ b/chrome/content/zotero/xpcom/progressQueue.js @@ -28,7 +28,14 @@ Zotero.ProgressQueue = function (options) { let _title = options.title; let _columns = options.columns; - let _listeners = {}; + let _listeners = { + empty: [], + cancel: [], + rowadded: [], + nonempty: [], + rowupdated: [], + rowdeleted: [], + }; let _rows = []; let _dialog = null; @@ -75,7 +82,10 @@ Zotero.ProgressQueue = function (options) { * @param callback */ this.addListener = function (name, callback) { - _listeners[name] = callback; + if (!(name in _listeners)) { + throw new Error(`Invalid event listener "${name}"`); + } + _listeners[name].push(callback); }; @@ -83,8 +93,17 @@ Zotero.ProgressQueue = function (options) { * Remove listener * @param {String} name Event name */ - this.removeListener = function (name) { - delete _listeners[name]; + this.removeListener = function (name, callback) { + if (!(name in _listeners)) { + throw new Error(`Invalid event listener "${name}"`); + } + if (!callback) { + Zotero.debug(`Calling "removeListener" without specifying which callback to remove is deprecated`); + _listeners[name] = []; // remove all callbacks to simulate previous behaviour + } + else { + _listeners[name] = _listeners[name].filter(existingCallback => existingCallback !== callback); + } }; @@ -120,13 +139,8 @@ Zotero.ProgressQueue = function (options) { */ this.cancel = function () { _rows = []; - if (_listeners.empty) { - _listeners.empty(); - } - - if (_listeners.cancel) { - _listeners.cancel(); - } + _listeners.empty.forEach(listener => listener()); + _listeners.cancel.forEach(listener => listener()); }; @@ -146,13 +160,8 @@ Zotero.ProgressQueue = function (options) { _rows.push(row); - if (_listeners.rowadded) { - _listeners.rowadded(row); - } - - if (_listeners.nonempty) { - _listeners.nonempty(); - } + _listeners.rowadded.forEach(listener => listener(row)); + _listeners.nonempty.forEach(listener => listener()); }; @@ -168,13 +177,11 @@ Zotero.ProgressQueue = function (options) { if (row.id === itemID) { row.status = status; row.message = message; - if (_listeners.rowupdated) { - _listeners.rowupdated({ - id: row.id, - status, - message: message || '' - }); - } + _listeners.rowupdated.forEach(listener => listener({ + id: row.id, + status, + message: message || '' + })); return; } } @@ -187,13 +194,11 @@ Zotero.ProgressQueue = function (options) { */ this.deleteRow = function(itemID) { let row = _rows.find(x => x.id === itemID); - if(row) { + if (row) { _rows.splice(_rows.indexOf(row), 1); - if (_listeners.rowdeleted) { - _listeners.rowdeleted({ - id: row.id - }); - } + _listeners.rowdeleted.forEach(listener => listener({ + id: row.id + })); } }; }; diff --git a/chrome/content/zotero/xpcom/progressQueueDialog.js b/chrome/content/zotero/xpcom/progressQueueDialog.js index 816fbed825..91d538d3c4 100644 --- a/chrome/content/zotero/xpcom/progressQueueDialog.js +++ b/chrome/content/zotero/xpcom/progressQueueDialog.js @@ -113,29 +113,18 @@ Zotero.ProgressQueueDialog = function (progressQueue) { }); _progressWindow.addEventListener('unload', function () { - _progressQueue.removeListener('rowadded'); - _progressQueue.removeListener('rowupdated'); - _progressQueue.removeListener('rowdeleted'); + _progressQueue.removeListener('rowadded', _updateProgress); + _progressQueue.removeListener('rowupdated', _updateProgress); + _progressQueue.removeListener('rowdeleted', _updateProgress); _progressWindow = null; _progressIndicator = null; _status = null; _showMinimize = true; }); - _progressQueue.addListener('rowadded', function (row) { - _io.tree.invalidate(); - _updateProgress(); - }); - - _progressQueue.addListener('rowupdated', function (row) { - _io.tree.invalidate(); - _updateProgress(); - }); - - _progressQueue.addListener('rowdeleted', function (row) { - _io.tree.invalidate(); - _updateProgress(); - }); + _progressQueue.addListener('rowadded', _updateProgress); + _progressQueue.addListener('rowupdated', _updateProgress); + _progressQueue.addListener('rowdeleted', _updateProgress); _updateProgress(); }