diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index c84d97ff86..c26366e288 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -3150,7 +3150,6 @@ var ZoteroPane = new function() 'sep1', 'addNote', 'createNoteFromAnnotations', - 'createNoteFromAnnotationsMenu', 'addAttachments', 'sep2', 'findPDF', @@ -3286,12 +3285,33 @@ var ZoteroPane = new function() } } - // "Add Notes from Annotation" and "Find Available PDFs" + // "Add/Create Note from Annotations" and "Find Available PDFs" if (collectionTreeRow.filesEditable && !collectionTreeRow.isDuplicates() && !collectionTreeRow.isFeed()) { - if (items.some(item => attachmentsWithExtractableAnnotations(item).length)) { + if (items.some(item => attachmentsWithExtractableAnnotations(item).length) + || items.some(item => isAttachmentWithExtractableAnnotations(item))) { + let menuitem = menu.childNodes[m.createNoteFromAnnotations]; show.add(m.createNoteFromAnnotations); + let key; + // If all from a single item, show "Add Note from Annotations" + if (Zotero.Items.getTopLevel(items).length == 1) { + key = 'addNoteFromAnnotations'; + menuitem.onclick = async () => { + return this.addNoteFromAnnotationsFromSelected(); + }; + } + // Otherwise show "Create Note from Annotations" + else { + key = 'createNoteFromAnnotations'; + menuitem.onclick = async () => { + return this.createStandaloneNoteFromAnnotationsFromSelected(); + }; + } + menuitem.setAttribute( + 'label', + Zotero.getString('pane.items.menu.' + key) + ); show.add(m.sep3); } @@ -3351,43 +3371,42 @@ var ZoteroPane = new function() show.add(m.sep1); } + // Show "Add Note from Annotations" on parent item with any extractable annotations if (item.isRegularItem() && !item.isFeedItem) { show.add(m.addNote); show.add(m.addAttachments); show.add(m.sep2); - // Create Note from Annotations - let popup = document.getElementById('create-note-from-annotations-popup'); - popup.textContent = ''; - let eligibleAttachments = Zotero.Items.get(item.getAttachments()) - .filter(item => item.isPDFAttachment()); - let attachmentsWithAnnotations = eligibleAttachments + let attachmentsWithAnnotations = Zotero.Items.get(item.getAttachments()) .filter(item => isAttachmentWithExtractableAnnotations(item)); if (attachmentsWithAnnotations.length) { - // Display submenu if there's more than one PDF attachment, even if - // there's only attachment with annotations, so it's clear which one - // the annotations are coming from - if (eligibleAttachments.length > 1) { - show.add(m.createNoteFromAnnotationsMenu); - for (let attachment of attachmentsWithAnnotations) { - let menuitem = document.createElement('menuitem'); - menuitem.setAttribute('label', attachment.getDisplayTitle()); - menuitem.onclick = () => { - ZoteroPane.createNoteFromAnnotationsForAttachment(attachment); - }; - popup.appendChild(menuitem); - } - } - // Single attachment with annotations - else { - show.add(m.createNoteFromAnnotations); - } + show.add(m.createNoteFromAnnotations); } } - else if (isAttachmentWithExtractableAnnotations(item) && !item.isTopLevelItem()) { + // Show "(Create|Add) Note from Annotations" on attachment with extractable annotations + else if (isAttachmentWithExtractableAnnotations(item)) { show.add(m.createNoteFromAnnotations); show.add(m.sep2); } + if (show.has(m.createNoteFromAnnotations)) { + let menuitem = menu.childNodes[m.createNoteFromAnnotations]; + let str; + // Show "Create" on standalone attachments + if (item.isAttachment() && item.isTopLevelItem()) { + str = 'pane.items.menu.createNoteFromAnnotations'; + menuitem.onclick = async () => { + return this.createStandaloneNoteFromAnnotationsFromSelected(); + }; + } + // And "Add" otherwise + else { + str = 'pane.items.menu.addNoteFromAnnotations'; + menuitem.onclick = async () => { + return this.addNoteFromAnnotationsFromSelected(); + }; + } + menuitem.setAttribute('label', Zotero.getString(str)); + } if (Zotero.Attachments.canFindPDFForItem(item)) { show.add(m.findPDF); @@ -3553,8 +3572,6 @@ var ZoteroPane = new function() } // Set labels, plural if necessary - menu.childNodes[m.createNoteFromAnnotations].setAttribute('label', Zotero.getString('pane.items.menu.addNoteFromAnnotations' + multiple)); - menu.childNodes[m.createNoteFromAnnotationsMenu].setAttribute('label', Zotero.getString('pane.items.menu.addNoteFromAnnotations' + multiple)); menu.childNodes[m.findPDF].setAttribute('label', Zotero.getString('pane.items.menu.findAvailablePDF' + multiple)); menu.childNodes[m.moveToTrash].setAttribute('label', Zotero.getString('pane.items.menu.moveToTrash' + multiple)); menu.childNodes[m.deleteFromLibrary].setAttribute('label', Zotero.getString('pane.items.menu.delete')); @@ -4952,7 +4969,7 @@ var ZoteroPane = new function() }; - this.createNoteFromAnnotationsForAttachment = async function (attachment, { skipSelect } = {}) { + this.addNoteFromAnnotationsForAttachment = async function (attachment, { skipSelect } = {}) { if (!this.canEdit()) { this.displayCannotEditLibraryMessage(); return; @@ -4975,7 +4992,75 @@ var ZoteroPane = new function() }; - this.createNoteFromAnnotationsFromSelected = async function () { + /** + * Add a single child note with the annotations from all selected items, including from all + * child attachments of a selected regular item + * + * Selected items must all have the same top-level item + */ + this.addNoteFromAnnotationsFromSelected = async function () { + if (!this.canEdit()) { + this.displayCannotEditLibraryMessage(); + return; + } + var items = this.getSelectedItems(); + var topLevelItems = [...new Set(Zotero.Items.getTopLevel(items))]; + if (topLevelItems.length > 1) { + throw new Error("Can't create child attachment from different top-level items"); + } + var topLevelItem = topLevelItems[0]; + if (!topLevelItem.isRegularItem()) { + throw new Error("Can't add note to standalone attachment"); + } + + // Ignore top-level item if specific child items are also selected + if (items.length > 1) { + items = items.filter(item => !item.isRegularItem()); + } + + var attachments = []; + for (let item of items) { + if (item.isRegularItem()) { + // Find all child items with extractable annotations + attachments.push( + ...Zotero.Items.get(item.getAttachments()) + .filter(item => isAttachmentWithExtractableAnnotations(item)) + ); + } + else if (isAttachmentWithExtractableAnnotations(item)) { + attachments.push(item); + } + else { + continue; + } + } + + if (!attachments.length) { + Zotero.debug("No attachments found", 2); + return; + } + + var annotations = []; + for (let attachment of attachments) { + annotations.push(...attachment.getAnnotations()); + } + var note = await Zotero.EditorInstance.createNoteFromAnnotations( + annotations, + { + parentID: topLevelItem.id + } + ); + await this.selectItem(note.id); + }; + + + /** + * Create separate child notes for each selected item, including all child attachments of + * selected regular items + * + * No longer exposed via UI + */ + this.addNotesFromAnnotationsFromSelected = async function () { if (!this.canEdit()) { this.displayCannotEditLibraryMessage(); return; @@ -5001,7 +5086,7 @@ var ZoteroPane = new function() continue; } for (let attachment of attachments) { - let note = await this.createNoteFromAnnotationsForAttachment( + let note = await this.addNoteFromAnnotationsForAttachment( attachment, { skipSelect: true } ); @@ -5013,6 +5098,59 @@ var ZoteroPane = new function() await this.selectItems(itemIDsToSelect); }; + + this.createStandaloneNoteFromAnnotationsFromSelected = async function () { + if (!this.canEdit()) { + this.displayCannotEditLibraryMessage(); + return; + } + var items = this.getSelectedItems(); + + // Ignore selected top-level items if any descendant items are also selected + var topLevelOfSelectedDescendants = new Set(); + for (let item of items) { + if (!item.isTopLevelItem()) { + topLevelOfSelectedDescendants.add(item.topLevelItem); + } + } + items = items.filter(item => !topLevelOfSelectedDescendants.has(item)); + + var annotations = []; + for (let item of items) { + let attachments = []; + if (item.isRegularItem()) { + // Find all child attachments with extractable annotations + attachments.push( + ...Zotero.Items.get(item.getAttachments()) + .filter(item => isAttachmentWithExtractableAnnotations(item)) + ); + } + else if (isAttachmentWithExtractableAnnotations(item)) { + attachments.push(item); + } + else { + continue; + } + for (let attachment of attachments) { + annotations.push(...attachment.getAnnotations()); + } + } + + if (!annotations.length) { + Zotero.debug("No annotations found", 2); + return; + } + + var note = await Zotero.EditorInstance.createNoteFromAnnotations( + annotations, + { + collectionID: this.getSelectedCollection(true) + } + ); + await this.selectItem(note.id); + }; + + this.createEmptyParent = async function (item) { await Zotero.DB.executeTransaction(async function () { // TODO: remove once there are no top-level web attachments diff --git a/chrome/content/zotero/zoteroPane.xul b/chrome/content/zotero/zoteroPane.xul index 4ca670647a..0716fec257 100644 --- a/chrome/content/zotero/zoteroPane.xul +++ b/chrome/content/zotero/zoteroPane.xul @@ -293,11 +293,7 @@ - - - - + diff --git a/chrome/locale/en-US/zotero/zotero.properties b/chrome/locale/en-US/zotero/zotero.properties index f298c24cd6..c9cf8bcfae 100644 --- a/chrome/locale/en-US/zotero/zotero.properties +++ b/chrome/locale/en-US/zotero/zotero.properties @@ -333,7 +333,7 @@ pane.items.removeRecursive.title = Remove from Collection and Subcollections pane.items.removeRecursive = Are you sure you want to remove the selected item from this collection and all subcollections? pane.items.removeRecursive.multiple = Are you sure you want to remove the selected items from this collection and all subcollections? pane.items.menu.addNoteFromAnnotations = Add Note from Annotations -pane.items.menu.addNoteFromAnnotations.multiple = Add Notes from Annotations +pane.items.menu.createNoteFromAnnotations = Create Note from Annotations pane.items.menu.findAvailablePDF = Find Available PDF pane.items.menu.findAvailablePDF.multiple = Find Available PDFs pane.items.menu.addToCollection = Add to Collection diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index c88b8b9a75..88b3c9de9b 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -338,6 +338,204 @@ describe("ZoteroPane", function() { }) + describe("#addNoteFromAnnotationsFromSelected()", function () { + it("should create a single note within a selected regular item for all child attachments", async function () { + var item = await createDataObject('item'); + var attachment1 = await importPDFAttachment(item); + var attachment2 = await importPDFAttachment(item); + var annotation1 = await createAnnotation('highlight', attachment1); + var annotation2 = await createAnnotation('highlight', attachment1); + var annotation3 = await createAnnotation('highlight', attachment2); + var annotation4 = await createAnnotation('highlight', attachment2); + await zp.selectItems([item.id]); + await zp.addNoteFromAnnotationsFromSelected(); + var newItems = zp.getSelectedItems(); + assert.lengthOf(newItems, 1); + var note = newItems[0]; + assert.equal(note.itemType, 'note'); + assert.equal(note.parentID, item.id); + var dp = new DOMParser(); + var doc = dp.parseFromString(note.getNote(), 'text/html'); + assert.sameMembers( + [...doc.querySelectorAll('h3')].map(x => x.textContent), + [attachment1.attachmentFilename, attachment2.attachmentFilename] + ); + assert.lengthOf([...doc.querySelectorAll('h3 + p')], 2); + assert.lengthOf([...doc.querySelectorAll('span.highlight')], 4); + }); + + it("should create a single note within the parent for all selected sibling attachments", async function () { + var item = await createDataObject('item'); + var attachment1 = await importPDFAttachment(item); + var attachment2 = await importPDFAttachment(item); + var annotation1 = await createAnnotation('highlight', attachment1); + var annotation2 = await createAnnotation('highlight', attachment1); + var annotation3 = await createAnnotation('highlight', attachment2); + var annotation4 = await createAnnotation('highlight', attachment2); + await zp.selectItems([attachment1.id, attachment2.id]); + await zp.addNoteFromAnnotationsFromSelected(); + var newItems = zp.getSelectedItems(); + assert.lengthOf(newItems, 1); + var note = newItems[0]; + assert.equal(note.parentID, item.id); + var dp = new DOMParser(); + var doc = dp.parseFromString(note.getNote(), 'text/html'); + assert.sameMembers( + [...doc.querySelectorAll('h3')].map(x => x.textContent), + [attachment1.attachmentFilename, attachment2.attachmentFilename] + ); + // No item titles + assert.lengthOf([...doc.querySelectorAll('h2 + p')], 0); + // Just attachment titles + assert.lengthOf([...doc.querySelectorAll('h3 + p')], 2); + assert.lengthOf([...doc.querySelectorAll('span.highlight')], 4); + }); + + it("should ignore top-level item if child attachment is also selected", async function () { + var item = await createDataObject('item'); + var attachment1 = await importPDFAttachment(item); + var attachment2 = await importPDFAttachment(item); + await createAnnotation('highlight', attachment1); + await createAnnotation('highlight', attachment1); + await createAnnotation('highlight', attachment2); + await zp.selectItems([item.id, attachment1.id]); + await zp.addNoteFromAnnotationsFromSelected(); + var newItems = zp.getSelectedItems(); + assert.lengthOf(newItems, 1); + var note = newItems[0]; + var dp = new DOMParser(); + var doc = dp.parseFromString(note.getNote(), 'text/html'); + // No titles + assert.lengthOf([...doc.querySelectorAll('h2 + p')], 0); + assert.lengthOf([...doc.querySelectorAll('h3 + p')], 0); + assert.lengthOf([...doc.querySelectorAll('span.highlight')], 2); + }); + + it("shouldn't do anything if parent item and child note is selected", async function () { + var item = await createDataObject('item'); + var attachment = await importPDFAttachment(item); + var note = await createDataObject('item', { itemType: 'note', parentID: item.id }); + await createAnnotation('highlight', attachment); + await zp.selectItems([item.id, note.id]); + await zp.addNoteFromAnnotationsFromSelected(); + var selectedItems = zp.getSelectedItems(); + assert.lengthOf(selectedItems, 2); + assert.sameMembers(selectedItems, [item, note]); + }); + }); + + + describe("#createStandaloneNoteFromAnnotationsFromSelected()", function () { + it("should create a single standalone note for all child attachments of selected regular items", async function () { + var collection = await createDataObject('collection'); + var item1 = await createDataObject('item', { setTitle: true, collections: [collection.id] }); + var item2 = await createDataObject('item', { setTitle: true, collections: [collection.id] }); + var attachment1 = await importPDFAttachment(item1); + var attachment2 = await importPDFAttachment(item1); + var attachment3 = await importPDFAttachment(item2); + var attachment4 = await importPDFAttachment(item2); + await createAnnotation('highlight', attachment1); + await createAnnotation('highlight', attachment1); + await createAnnotation('highlight', attachment2); + await createAnnotation('highlight', attachment2); + await createAnnotation('highlight', attachment3); + await createAnnotation('highlight', attachment3); + await createAnnotation('highlight', attachment4); + await createAnnotation('highlight', attachment4); + await zp.selectItems([item1.id, item2.id]); + await zp.createStandaloneNoteFromAnnotationsFromSelected(); + var newItems = zp.getSelectedItems(); + assert.lengthOf(newItems, 1); + var note = newItems[0]; + assert.equal(note.itemType, 'note'); + assert.isFalse(note.parentID); + assert.isTrue(collection.hasItem(note)); + var dp = new DOMParser(); + var doc = dp.parseFromString(note.getNote(), 'text/html'); + assert.sameMembers( + [...doc.querySelectorAll('h2')].map(x => x.textContent), + [item1.getDisplayTitle(), item2.getDisplayTitle()] + ); + assert.sameMembers( + [...doc.querySelectorAll('h3')].map(x => x.textContent), + [ + attachment1.attachmentFilename, + attachment2.attachmentFilename, + attachment3.attachmentFilename, + attachment4.attachmentFilename + ] + ); + assert.lengthOf([...doc.querySelectorAll('h3 + p')], 4); + assert.lengthOf([...doc.querySelectorAll('span.highlight')], 8); + }); + + it("should create a single standalone note for all selected attachments", async function () { + var collection = await createDataObject('collection'); + var item1 = await createDataObject('item', { setTitle: true, collections: [collection.id] }); + var item2 = await createDataObject('item', { setTitle: true, collections: [collection.id] }); + var attachment1 = await importPDFAttachment(item1); + var attachment2 = await importPDFAttachment(item1); + var attachment3 = await importPDFAttachment(item2); + var attachment4 = await importPDFAttachment(item2); + await createAnnotation('highlight', attachment1); + await createAnnotation('highlight', attachment1); + await createAnnotation('highlight', attachment2); + await createAnnotation('highlight', attachment2); + await createAnnotation('highlight', attachment3); + await createAnnotation('highlight', attachment3); + await createAnnotation('highlight', attachment4); + await createAnnotation('highlight', attachment4); + await zp.selectItems([attachment1.id, attachment3.id]); + await zp.createStandaloneNoteFromAnnotationsFromSelected(); + var newItems = zp.getSelectedItems(); + assert.lengthOf(newItems, 1); + var note = newItems[0]; + assert.isFalse(note.parentID); + assert.isTrue(collection.hasItem(note)); + var dp = new DOMParser(); + var doc = dp.parseFromString(note.getNote(), 'text/html'); + assert.sameMembers( + [...doc.querySelectorAll('h2')].map(x => x.textContent), + [item1.getDisplayTitle(), item2.getDisplayTitle()] + ); + assert.lengthOf([...doc.querySelectorAll('h2 + p')], 2); + assert.lengthOf([...doc.querySelectorAll('h3')], 0); + assert.lengthOf([...doc.querySelectorAll('span.highlight')], 4); + }); + + it("should ignore top-level item if child attachment is also selected", async function () { + var item1 = await createDataObject('item', { setTitle: true }); + var item2 = await createDataObject('item', { setTitle: true }); + var attachment1 = await importPDFAttachment(item1); + var attachment2 = await importPDFAttachment(item1); + var attachment3 = await importPDFAttachment(item2); + var attachment4 = await importPDFAttachment(item2); + await createAnnotation('highlight', attachment1); + await createAnnotation('highlight', attachment1); + await createAnnotation('highlight', attachment2); + await createAnnotation('highlight', attachment2); + await createAnnotation('highlight', attachment3); + await createAnnotation('highlight', attachment3); + await createAnnotation('highlight', attachment4); + await createAnnotation('highlight', attachment4); + await zp.selectItems([item1.id, attachment1.id, attachment3.id]); + await zp.createStandaloneNoteFromAnnotationsFromSelected(); + var newItems = zp.getSelectedItems(); + assert.lengthOf(newItems, 1); + var note = newItems[0]; + var dp = new DOMParser(); + var doc = dp.parseFromString(note.getNote(), 'text/html'); + assert.sameMembers( + [...doc.querySelectorAll('h2')].map(x => x.textContent), + [item1.getDisplayTitle(), item2.getDisplayTitle()] + ); + assert.lengthOf([...doc.querySelectorAll('h2 + p')], 2); + assert.lengthOf([...doc.querySelectorAll('h3')], 0); + assert.lengthOf([...doc.querySelectorAll('span.highlight')], 4); + }); + }); + + describe("#renameSelectedAttachmentsFromParents()", function () { it("should rename a linked file", async function () { var oldFilename = 'old.png';