From b0ba7e464602f713ea2e9c3a1d768b9b089b68f2 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Wed, 22 Dec 2021 03:38:12 -0500 Subject: [PATCH] Show small PDF icon in Attachments column when there's a PDF Closes #2265 There's also new code for showing a different icon for snapshots, files, and DOI/URL links, like the web library and iOS app, but it's commented out for now. The bitmap icons create too much visual noise with the greater information density and hierarchical tree of the desktop app (not to mention many more low-DPI displays). We can revisit after switching to SVGs across the board. --- chrome/content/zotero/components/icons.jsx | 1 + chrome/content/zotero/itemTree.jsx | 69 ++++++++++++++++---- chrome/content/zotero/itemTreeColumns.jsx | 2 +- chrome/content/zotero/xpcom/data/item.js | 44 ++++++++++--- chrome/locale/en-US/zotero/zotero.properties | 5 +- scss/components/_item-tree.scss | 17 +++++ 6 files changed, 114 insertions(+), 24 deletions(-) diff --git a/chrome/content/zotero/components/icons.jsx b/chrome/content/zotero/components/icons.jsx index b6c1118eca..70c85f6904 100644 --- a/chrome/content/zotero/components/icons.jsx +++ b/chrome/content/zotero/components/icons.jsx @@ -86,6 +86,7 @@ i('Twisty', ( i('Cross', "chrome://zotero/skin/cross.png"); i('Tick', "chrome://zotero/skin/tick.png"); i('ArrowRefresh', "chrome://zotero/skin/arrow_refresh.png"); +//i('Link', "chrome://zotero/skin/link.png"); i('RTFScanAccept', "chrome://zotero/skin/rtfscan-accept.png"); i('RTFScanLink', "chrome://zotero/skin/rtfscan-link.png"); diff --git a/chrome/content/zotero/itemTree.jsx b/chrome/content/zotero/itemTree.jsx index 9864245778..f5709c6e93 100644 --- a/chrome/content/zotero/itemTree.jsx +++ b/chrome/content/zotero/itemTree.jsx @@ -2644,28 +2644,57 @@ var ItemTree = class ItemTree extends LibraryTree { return span; } + // TEMP: For now, we use the blue bullet for all non-PDF attachments, but there's + // commented-out code for showing different icons for snapshots, files, and URL/DOI links if (this.isContainer(index)) { if (item.isRegularItem()) { - const state = item.getBestAttachmentStateCached(); + const { type, exists } = item.getBestAttachmentStateCached(); let icon = ""; - if (state === 1) { - icon = getDOMElement('IconBulletBlue'); + let ariaLabel; + // If the item has a child attachment + if (type !== null && type != 'none') { + if (type == 'pdf') { + icon = getDOMElement('IconTreeitemAttachmentPDF'); + ariaLabel = Zotero.getString('pane.item.attachments.hasPDF'); + if (!exists) { + icon.classList.add('icon-missing-file'); + } + } + else if (type == 'snapshot') { + //icon = getDOMElement('IconTreeitemAttachmentSnapshot'); + icon = exists ? getDOMElement('IconBulletBlue') : getDOMElement('IconBulletBlueEmpty'); + ariaLabel = Zotero.getString('pane.item.attachments.hasSnapshot'); + } + else { + //icon = getDOMElement('IconTreeitem'); + icon = exists ? getDOMElement('IconBulletBlue') : getDOMElement('IconBulletBlueEmpty'); + ariaLabel = Zotero.getString('pane.item.attachments.has'); + } icon.classList.add('cell-icon'); + //if (!exists) { + // icon.classList.add('icon-missing-file'); + //} } - else if (state === -1) { - icon = getDOMElement('IconBulletBlueEmpty'); - icon.classList.add('cell-icon'); - } - - if (icon.setAttribute) { - icon.setAttribute('aria-label', Zotero.getString('pane.item.attachments.has') + '.'); + //else if (type == 'none') { + // if (item.getField('url') || item.getField('DOI')) { + // icon = getDOMElement('IconLink'); + // ariaLabel = Zotero.getString('pane.item.attachments.hasLink'); + // icon.classList.add('cell-icon'); + // } + //} + if (ariaLabel) { + icon.setAttribute('aria-label', ariaLabel + '.'); } span.append(icon); item.getBestAttachmentState() // TODO: With no cell refreshing this is possibly somewhat inefficient // Refresh cell when promise is fulfilled - .then(bestState => bestState != state && this.tree.invalidateRow(index)); + .then(({ type: newType, exists: newExists }) => { + if (newType !== type || newExists !== exists) { + this.tree.invalidateRow(index); + } + }); } } @@ -2673,8 +2702,24 @@ var ItemTree = class ItemTree extends LibraryTree { const exists = item.fileExistsCached(); let icon = ""; if (exists !== null) { - icon = exists ? getDOMElement('IconBulletBlue') : getDOMElement('IconBulletBlueEmpty'); + if (item.isPDFAttachment()) { + icon = getDOMElement('IconTreeitemAttachmentPDF'); + if (!exists) { + icon.classList.add('icon-missing-file'); + } + } + else if (item.isSnapshotAttachment()) { + //icon = getDOMElement('IconTreeitemAttachmentSnapshot'); + icon = exists ? getDOMElement('IconBulletBlue') : getDOMElement('IconBulletBlueEmpty'); + } + else { + //icon = getDOMElement('IconTreeitem'); + icon = exists ? getDOMElement('IconBulletBlue') : getDOMElement('IconBulletBlueEmpty'); + } icon.classList.add('cell-icon'); + //if (!exists) { + // icon.classList.add('icon-missing-file'); + //} } span.append(icon); diff --git a/chrome/content/zotero/itemTreeColumns.jsx b/chrome/content/zotero/itemTreeColumns.jsx index 770c38350d..966fb7968a 100644 --- a/chrome/content/zotero/itemTreeColumns.jsx +++ b/chrome/content/zotero/itemTreeColumns.jsx @@ -275,7 +275,7 @@ const COLUMNS = [ label: "zotero.tabs.attachments.label", iconLabel: , fixedWidth: true, - width: "14", + width: "16", zoteroPersist: new Set(["hidden", "sortDirection"]) }, { diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index b86b8a4d82..982bce8684 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -2315,6 +2315,16 @@ Zotero.Item.prototype.isEmbeddedImageAttachment = function() { } +/** + * @return {Boolean} - Returns true if item is a snapshot + */ +Zotero.Item.prototype.isSnapshotAttachment = function () { + return this.attachmentLinkMode == Zotero.Attachments.LINK_MODE_IMPORTED_URL + && this.attachmentContentType == 'text/html'; +}; + + + /** * @return {Boolean} - Returns true if item is a stored or linked PDF attachment */ @@ -3631,28 +3641,42 @@ Zotero.Item.prototype.getBestAttachments = Zotero.Promise.coroutine(function* () /** * Return state of best attachment * - * @return {Promise} Promise for 0 (none), 1 (present), -1 (missing) + * @return {Promise} - Promise for object with string 'type' ('none'|'pdf'|'snapshot'|'other') + * and boolean 'exists' */ -Zotero.Item.prototype.getBestAttachmentState = Zotero.Promise.coroutine(function* () { +Zotero.Item.prototype.getBestAttachmentState = async function () { if (this._bestAttachmentState !== null) { return this._bestAttachmentState; } - var item = yield this.getBestAttachment(); - if (item) { - let exists = yield item.fileExists(); - return this._bestAttachmentState = exists ? 1 : -1; + var item = await this.getBestAttachment(); + if (!item) { + return this._bestAttachmentState = { + type: 'none' + }; } - return this._bestAttachmentState = 0; -}); + var type; + if (item.isPDFAttachment()) { + type = 'pdf'; + } + else if (item.isSnapshotAttachment()) { + type = 'snapshot'; + } + else { + type = 'other'; + } + var exists = await item.fileExists(); + return this._bestAttachmentState = { type, exists }; +}; /** * Return cached state of best attachment for use in items view * - * @return {Integer|null} 0 (none), 1 (present), -1 (missing), null (unavailable) + * @return {Object|null} - Resolved value from getBestAttachmentState() or { type: null } if + * unavailable */ Zotero.Item.prototype.getBestAttachmentStateCached = function () { - return this._bestAttachmentState; + return this._bestAttachmentState || { type: null }; } diff --git a/chrome/locale/en-US/zotero/zotero.properties b/chrome/locale/en-US/zotero/zotero.properties index cf5b524035..6f6cae8d0f 100644 --- a/chrome/locale/en-US/zotero/zotero.properties +++ b/chrome/locale/en-US/zotero/zotero.properties @@ -405,7 +405,10 @@ pane.item.attachments.delete.confirm = Are you sure you want to delete this att pane.item.attachments.count.zero = %S attachments: pane.item.attachments.count.singular = %S attachment: pane.item.attachments.count.plural = %S attachments: -pane.item.attachments.has = Has attachments +pane.item.attachments.has = Has attachment +pane.item.attachments.hasPDF = Has PDF attachment +pane.item.attachments.hasSnapshot = Has snapshot +pane.item.attachments.hasLink = Has link pane.item.attachments.select = Select a File pane.item.attachments.PDF.installTools.title = PDF Tools Not Installed pane.item.attachments.PDF.installTools.text = To use this feature, you must first install the PDF tools in the Search pane of the Zotero preferences. diff --git a/scss/components/_item-tree.scss b/scss/components/_item-tree.scss index 59401818e9..b7019a4b03 100644 --- a/scss/components/_item-tree.scss +++ b/scss/components/_item-tree.scss @@ -29,6 +29,23 @@ .cell.hasAttachment { box-sizing: content-box; padding: 0 4px; + height: 100%; + + .icon-treeitemattachmentpdf { + min-width: 10px; + max-width: 10px; + margin: 3px; + } + + .icon-link { + min-width: 14px; + max-width: 14px; + margin: 2px 1px; + } + + .icon-missing-file { + opacity: 0.4; + } } .cell.numNotes {