From c78739bf2f9d8b65e8115fec1c69d2020367a486 Mon Sep 17 00:00:00 2001
From: Abe Jellinek <jellinek@berkeley.edu>
Date: Mon, 4 Dec 2023 12:12:17 -0500
Subject: [PATCH] Update and improve attachment-row initialization

- Actually show un-deleted rows as context rows within trash
- Add all rows, hide ones that shouldn't be shown
- Check whether attachment is file before calling getAnnotations()
  - Fixes errors on update and new link attachments being shown as empty rows
---
 chrome/content/zotero/elements/attachmentRow.js | 15 ++++++++++-----
 .../content/zotero/elements/attachmentsBox.js   | 17 +++++++++--------
 scss/elements/_attachmentRow.scss               |  6 +++++-
 3 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/chrome/content/zotero/elements/attachmentRow.js b/chrome/content/zotero/elements/attachmentRow.js
index 827dcbdf4f..c74872c3ca 100644
--- a/chrome/content/zotero/elements/attachmentRow.js
+++ b/chrome/content/zotero/elements/attachmentRow.js
@@ -106,7 +106,9 @@ import { getCSSItemTypeIcon } from 'components/icons';
 		}
 		
 		get empty() {
-			return !this._attachment || !this._attachment.numAnnotations();
+			return !this._attachment
+				|| !this._attachment.isFileAttachment()
+				|| !this._attachment.numAnnotations();
 		}
 		
 		get contextRow() {
@@ -155,10 +157,13 @@ import { getCSSItemTypeIcon } from 'components/icons';
 			this._label.textContent = this._attachment.getField('title');
 			
 			this._body.replaceChildren();
-			for (let annotation of this._attachment.getAnnotations()) {
-				let row = document.createXULElement('annotation-row');
-				row.annotation = annotation;
-				this._body.append(row);
+			
+			if (this._attachment.isFileAttachment()) {
+				for (let annotation of this._attachment.getAnnotations()) {
+					let row = document.createXULElement('annotation-row');
+					row.annotation = annotation;
+					this._body.append(row);
+				}
 			}
 
 			if (!this._listenerAdded) {
diff --git a/chrome/content/zotero/elements/attachmentsBox.js b/chrome/content/zotero/elements/attachmentsBox.js
index 18a9b4c910..418d8fbf98 100644
--- a/chrome/content/zotero/elements/attachmentsBox.js
+++ b/chrome/content/zotero/elements/attachmentsBox.js
@@ -52,7 +52,7 @@
 			this._item = item;
 			this._body.replaceChildren();
 			if (item) {
-				for (let attachment of Zotero.Items.get(item.getAttachments())) {
+				for (let attachment of Zotero.Items.get(item.getAttachments(true))) {
 					this.addRow(attachment);
 				}
 				this.updateCount();
@@ -74,7 +74,7 @@
 		set inTrash(inTrash) {
 			this._inTrash = inTrash;
 			for (let row of this._body.children) {
-				row.contextRow = this._isContext(row.attachment);
+				this._updateRowAttributes(row, row.attachment);
 			}
 			this.updateCount();
 		}
@@ -124,11 +124,8 @@
 		}
 		
 		addRow(attachment) {
-			if (attachment.deleted && !this._inTrash) return;
-			
 			let row = document.createXULElement('attachment-row');
-			row.attachment = attachment;
-			row.contextRow = this._isContext(attachment);
+			this._updateRowAttributes(row, attachment);
 			
 			let inserted = false;
 			for (let existingRow of this._body.children) {
@@ -156,8 +153,12 @@
 			this._section.open = true;
 		};
 		
-		_isContext(attachment) {
-			return this._inTrash && !this._item.deleted && !attachment.deleted;
+		_updateRowAttributes(row, attachment) {
+			let hidden = !this._inTrash && attachment.deleted;
+			let context = this._inTrash && !this._item.deleted && !attachment.deleted;
+			row.attachment = attachment;
+			row.hidden = hidden;
+			row.contextRow = context;
 		}
 	}
 	customElements.define("attachments-box", AttachmentsBox);
diff --git a/scss/elements/_attachmentRow.scss b/scss/elements/_attachmentRow.scss
index 3a49c35566..3b736145e7 100644
--- a/scss/elements/_attachmentRow.scss
+++ b/scss/elements/_attachmentRow.scss
@@ -2,6 +2,10 @@ attachment-row {
 	display: flex;
 	flex-direction: column;
 	gap: 2px;
+	
+	&[hidden] {
+		display: none;
+	}
 
 	& > .head {
 		display: flex;
@@ -39,7 +43,7 @@ attachment-row {
 		fill: var(--fill-tertiary);
 	}
 	
-	&.context > .head .title {
+	&.context > .head .label {
 		// TODO This color is used in virtualized-table - probably want to change to something theme-defined
 		color: gray;
 	}