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

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 <http://www.gnu.org/licenses/>.
+	
+	***** 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 (
+		<IntlProvider locale={ Zotero.locale } messages={ Zotero.Intl.strings }>
+			<VirtualizedTable
+				getRowCount={ getRowCount }
+				ref={ treeRef }
+				id="progress-queue-table"
+				renderItem={ rowToTreeItem }
+				showHeader={ true }
+				columns={ tableColumns }
+				onActivate={ onActivate }
+			/>
+		</IntlProvider>
+	);
+};
+
+ProgressQueueTable.propTypes = {
+	onActivate: PropTypes.func,
+	progressQueue: PropTypes.object.isRequired
+};
+
+export default memo(ProgressQueueTable);

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 = (
-		<IntlProvider locale={Zotero.locale} messages={Zotero.Intl.strings}>
-			<VirtualizedTable
-				getRowCount={() => _progressQueue.getRows().length}
-				id="locateManager-table"
-				ref={ref => io.tree = _tree = ref}
-				renderItem={_rowToTreeItem}
-				showHeader={true}
-				columns={tableColumns}
-				onActivate={_handleActivate}
-			/>
-		</IntlProvider>
+	
+	ReactDOM.render(
+		<ProgressQueueTable
+			onActivate={ _handleActivate }
+			progressQueue={ _progressQueue }
+		/>,
+		domEl
 	);
-	ReactDOM.render(elem, domEl);
 }
 	
 /**

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
 		//

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
+			}));
 		}
 	};
 };

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();
 	}