diff --git a/chrome/content/zotero/elements/zoteroSearch.js b/chrome/content/zotero/elements/zoteroSearch.js index 7a978f7901..426b9f3ec0 100644 --- a/chrome/content/zotero/elements/zoteroSearch.js +++ b/chrome/content/zotero/elements/zoteroSearch.js @@ -884,7 +884,13 @@ - + + + + + + + @@ -930,6 +936,8 @@ // Build conditions menu var conditionsMenu = this.querySelector('#conditionsmenu'); var moreConditionsMenu = this.querySelector('#more-conditions-menu'); + var attachmentConditionsMenu = this.querySelector('#attachment-conditions-menu'); + var annotationConditionsMenu = this.querySelector('#annotation-conditions-menu'); var conditions = Zotero.SearchConditions.getStandardConditions(); // Cache the (alphabetically sorted) condition list and set up @@ -939,20 +947,45 @@ this._typeAheadTime = 0; conditionsMenu.addEventListener('keydown', event => this.handleConditionKeyDown(event), true); + // Label the submenus and seed the top-level entries with them, so the + // headings sort alphabetically alongside the primary conditions + attachmentConditionsMenu.setAttribute( + 'label', Zotero.getString('search-conditions-submenu-attachment') + ); + annotationConditionsMenu.setAttribute( + 'label', Zotero.getString('search-conditions-submenu-annotation') + ); + let topLevelEntries = [ + { label: attachmentConditionsMenu.getAttribute('label'), node: attachmentConditionsMenu }, + { label: annotationConditionsMenu.getAttribute('label'), node: annotationConditionsMenu }, + ]; + for (let condition of conditions) { let menuitem; - if (this.isPrimaryCondition(condition.name)) { + let submenu = this.getConditionSubmenu(condition.name); + // Attachment- and annotation-level conditions go in their own submenus, + // with a short label since the submenu heading supplies the context + if (submenu == 'attachment' || submenu == 'annotation') { + let parentMenu = submenu == 'attachment' + ? attachmentConditionsMenu + : annotationConditionsMenu; + menuitem = parentMenu.appendItem( + Zotero.getString('search-conditions-short-' + condition.name), + condition.name + ); + } + else if (this.isPrimaryCondition(condition.name)) { menuitem = document.createXULElement('menuitem'); menuitem.setAttribute('label', condition.localized); menuitem.setAttribute('value', condition.name); - moreConditionsMenu.before(menuitem); + topLevelEntries.push({ label: condition.localized, node: menuitem }); } else { menuitem = moreConditionsMenu.appendItem( condition.localized, condition.name ); } - + var baseFields = null; try { baseFields = Zotero.ItemFields.getTypeFieldsFromBase(condition.name); @@ -1011,6 +1044,15 @@ menuitem.setAttribute('tooltip', condition.name + '-tooltip'); } } + + // Insert the top-level conditions and the two submenus alphabetically, + // before the catch-all "More" submenu + let collation = Zotero.getLocaleCollation(); + topLevelEntries.sort((a, b) => collation.compareString(1, a.label, b.label)); + for (let entry of topLevelEntries) { + moreConditionsMenu.before(entry.node); + } + conditionsMenu.selectedIndex = 0; } @@ -1024,22 +1066,34 @@ case 'date': case 'dateAdded': case 'dateModified': - case 'lastRead': case 'itemType': - case 'fileTypeID': case 'publicationTitle': case 'tag': case 'note': + return true; + } + + return false; + } + + // Conditions that live in the Attachment or Annotation submenu rather than + // at the top level of the conditions menu. Returns 'attachment', 'annotation', + // or null. + getConditionSubmenu(condition) { + switch (condition) { case 'fulltextContent': + case 'fileTypeID': + case 'lastRead': + return 'attachment'; case 'annotationText': case 'annotationComment': case 'annotationType': case 'annotationColor': case 'annotationAuthor': - return true; + return 'annotation'; } - - return false; + + return null; } async onConditionSelected(conditionName, reload) { diff --git a/chrome/locale/en-US/zotero/zotero.ftl b/chrome/locale/en-US/zotero/zotero.ftl index ff1bd652f8..6ef4cb42af 100644 --- a/chrome/locale/en-US/zotero/zotero.ftl +++ b/chrome/locale/en-US/zotero/zotero.ftl @@ -1004,6 +1004,22 @@ search-conditions-annotationAuthor = Annotation Author search-conditions-anyField = Any Field search-conditions-titleCreatorYear = Title, Creator, Year +# Submenu headings grouping the attachment- and annotation-level conditions +search-conditions-submenu-attachment = Attachment +search-conditions-submenu-annotation = Annotation + +# Labels for conditions shown within the Attachment and Annotation submenus, +# where the submenu heading already supplies the "Attachment"/"Annotation" +# context. The full names above are still shown once a condition is selected. +search-conditions-short-fulltextContent = Content +search-conditions-short-fileTypeID = File Type +search-conditions-short-lastRead = Last Read +search-conditions-short-annotationText = Text +search-conditions-short-annotationComment = Comment +search-conditions-short-annotationType = Type +search-conditions-short-annotationColor = Color +search-conditions-short-annotationAuthor = Author + find-pdf-files-added = { $count -> [one] { $count } file added *[other] { $count } files added diff --git a/test/tests/advancedSearchTest.js b/test/tests/advancedSearchTest.js index 49c32b98a6..5f0105e001 100644 --- a/test/tests/advancedSearchTest.js +++ b/test/tests/advancedSearchTest.js @@ -629,6 +629,54 @@ describe("Advanced Search", function () { assert.isFalse(row.querySelector('#valuemenu').hidden); }); + it("should place attachment and annotation conditions in their submenus", function () { + var s = new Zotero.Search(); + s.libraryID = Zotero.Libraries.userLibraryID; + s.addCondition('title', 'is', ''); + pane.search = s; + + var row = conditions.firstChild; + var attachmentMenu = row.querySelector('#attachment-conditions-menu'); + var annotationMenu = row.querySelector('#annotation-conditions-menu'); + + // An attachment-level condition is in the Attachment submenu rather than at + // the top level of the conditions menu + assert.isFalse(row.isPrimaryCondition('fileTypeID')); + assert.ok(attachmentMenu.querySelector('menuitem[value="fileTypeID"]')); + + // An annotation-level condition is in the Annotation submenu + assert.isFalse(row.isPrimaryCondition('annotationColor')); + assert.ok(annotationMenu.querySelector('menuitem[value="annotationColor"]')); + + // Selecting one shows its full name as the closed-menu label + row.onConditionSelected('annotationColor'); + assert.equal(row.selectedCondition, 'annotationColor'); + assert.equal( + row.querySelector('#conditionsmenu').getAttribute('label'), + Zotero.SearchConditions.getLocalizedName('annotationColor') + ); + }); + + it("should sort the submenus alphabetically among the top-level conditions", function () { + var s = new Zotero.Search(); + s.libraryID = Zotero.Libraries.userLibraryID; + s.addCondition('title', 'is', ''); + pane.search = s; + + var row = conditions.firstChild; + var popup = row.querySelector('#conditionsmenu > menupopup'); + + // Top-level entries (including the Attachment/Annotation submenus) are in + // alphabetical order, with the catch-all "More" submenu kept last + var labels = [...popup.children] + .filter(node => node.id != 'more-conditions-menu') + .map(node => node.getAttribute('label')); + var collation = Zotero.getLocaleCollation(); + var sorted = [...labels].sort((a, b) => collation.compareString(1, a, b)); + assert.deepEqual(labels, sorted); + assert.equal(popup.lastElementChild.id, 'more-conditions-menu'); + }); + it("should keep an edited value when switching to another text condition", async function () { var s = new Zotero.Search(); s.libraryID = Zotero.Libraries.userLibraryID;