Add result-level controls to the Advanced Search builder (#5962)

- Reword the header as one sentence with a result-level menu ("Find
  [attachments] matching [all] of the following:")
- Provide a per-group menu to bind the group's descendant conditions to
  the same attachment, note, or annotation (e.g., one annotation that is
  both red and contains a given word, not two different ones)
- Show a hint that offers to group ungrouped sibling conditions (e.g.,
  two annotation conditions at the top level, to bind them to one
  annotation)
- Show a warning when conditions can't combine at the chosen result
  level (e.g., an annotation condition with a note result level)
- Remove the two legacy checkboxes:
  - "Show top-level items" becomes result level = top-level item and is
    migrated on save
  - "Include parent and child items", which has no result-level
    equivalent, keeps working, stays editable, and round-trips on
    searches that already have it, but it isn't offered on new searches
    and is removed on save if unchecked
This commit is contained in:
Dan Stillman 2026-06-23 11:47:58 -04:00 committed by Dan Stillman
parent 8b5a77a75e
commit 092295da22
8 changed files with 931 additions and 16 deletions

View file

@ -127,6 +127,9 @@
}
else {
this._search = new Zotero.Search();
// Default a fresh search to top-level items, so a condition on a child
// (e.g. attachment content) maps up to its item without any grouping
this._search.addCondition('resultLevel', 'item');
this._search.addCondition('title', 'contains', '');
}
this._searchElem.search = this._search;

View file

@ -38,9 +38,11 @@
class ZoteroSearch extends SearchElementBase {
content = MozXULElement.parseXULToFragment(`
<search-condition-group root="true"/>
<vbox id="search-binding-hint" hidden="true"/>
<hbox id="search-option-checkboxes">
<checkbox id="recursiveCheckbox" label="&zotero.search.recursive.label;" native="true"/>
<checkbox id="noChildrenCheckbox" label="&zotero.search.noChildren;" native="true"/>
</hbox>
<hbox id="search-legacy-options" align="center" hidden="true">
<checkbox id="includeParentsAndChildrenCheckbox" label="&zotero.search.includeParentsAndChildren;" native="true"/>
</hbox>
`, ['chrome://zotero/locale/zotero.dtd', 'chrome://zotero/locale/searchbox.dtd']);
@ -65,9 +67,20 @@
init() {
this.rootGroup = this.querySelector('search-condition-group[root]');
this.addEventListener('keypress', event => this.handleKeyPress(event));
// Re-evaluate which remove buttons are enabled as the conditions change
this.addEventListener('input', () => this.updateRemoveButtons());
this.addEventListener('command', () => this.updateRemoveButtons());
// Re-evaluate which remove buttons are enabled and which groups can bind to the
// same descendant as the conditions change
this.addEventListener('input', () => {
this.updateRemoveButtons();
this.updateBindingMenus();
this.updateBindingHint();
this.updateLevelWarning();
});
this.addEventListener('command', () => {
this.updateRemoveButtons();
this.updateBindingMenus();
this.updateBindingHint();
this.updateLevelWarning();
});
}
// Build the condition tree (root group, nested groups, and search-global
@ -75,9 +88,11 @@
renderConditions() {
var root = this.rootGroup;
for (let name of ['recursive', 'noChildren', 'includeParentsAndChildren']) {
this.querySelector('#' + name + 'Checkbox').checked = false;
}
this.querySelector('#recursiveCheckbox').checked = false;
// 'Include parent and child items' is a legacy hack subsumed by result levels;
// its checkbox is shown only for an existing search that still carries the flag
this.querySelector('#includeParentsAndChildrenCheckbox').checked = false;
this.querySelector('#search-legacy-options').hidden = true;
root.clear();
@ -90,16 +105,35 @@
let condition = conditions[id];
switch (condition.condition) {
case 'recursive':
this.querySelector('#recursiveCheckbox').checked = condition.operator == 'true';
continue;
// Legacy "show only top-level items" is exactly result level = item, so
// fold it into the root result level rather than a checkbox
case 'noChildren':
if (condition.operator == 'true') {
root.resultLevel = 'item';
}
continue;
// Legacy include-parents-and-children: keep it editable for searches that
// have it, but don't offer it on new ones
case 'includeParentsAndChildren':
this.querySelector('#' + condition.condition + 'Checkbox').checked
this.querySelector('#includeParentsAndChildrenCheckbox').checked
= condition.operator == 'true';
if (condition.operator == 'true') {
this.querySelector('#search-legacy-options').hidden = false;
}
continue;
case 'joinMode':
stack[stack.length - 1].joinMode = condition.operator;
continue;
case 'resultLevel':
stack[stack.length - 1].resultLevel = condition.operator;
continue;
case 'groupStart': {
let group = document.createXULElement('search-condition-group');
stack[stack.length - 1].conditionsContainer.appendChild(group);
@ -124,6 +158,73 @@
}
this.updateRemoveButtons();
this.updateBindingMenus();
this.updateBindingHint();
this.updateLevelWarning();
}
// Refresh each nested group's same-entity binding menu (visibility and options)
updateBindingMenus() {
for (let group of this.querySelectorAll('search-condition-group')) {
group.updateBindingMenu();
}
}
// Refresh every group's level warning. Each group flags only its own conditions, so the
// message sits on the group whose conditions actually conflict and speaks to that group's
// controls (see SearchConditionGroup.updateLevelWarning).
updateLevelWarning() {
for (let group of this.querySelectorAll('search-condition-group')) {
group.updateLevelWarning();
}
}
// Offer to bind ungrouped sibling conditions at the root. The root can't bind itself
// (it returns the result level), so 2+ of its conditions sharing a level below the
// result level are wrappable into a "same attachment" group -- surfaced as a hint
// with a button per such level. Nested groups use their own binding menu instead.
updateBindingHint() {
var hint = this.querySelector('#search-binding-hint');
var resultLevel = this.rootGroup.resultLevel;
var counts = {};
for (let row of this.rootGroup.conditionsContainer.children) {
// Skip rows the user hasn't filled in yet, so a freshly added (or just-retyped)
// condition doesn't trigger the hint until it actually has a value
if (row.localName != 'zoterosearchcondition' || !row.isPopulated()) {
continue;
}
let level = row.conditionLevel;
if (Zotero.Search._isAncestorLevel(resultLevel, level)) {
counts[level] = (counts[level] || 0) + 1;
}
}
var levels = ['attachment', 'note', 'annotation'].filter(l => counts[l] >= 2);
// Only rebuild when the set of bindable levels changes, so re-running this on every
// keystroke doesn't recreate the rows and make the hint flicker.
let key = levels.join(',');
if (key === this._bindingHintKey) {
return;
}
this._bindingHintKey = key;
hint.replaceChildren();
if (!levels.length) {
hint.hidden = true;
return;
}
// One self-contained line per level: a statement naming that level and a button to
// group its conditions into one entity. Separate lines when 2+ levels each qualify.
for (let level of levels) {
let row = document.createXULElement('hbox');
row.setAttribute('align', 'center');
let label = document.createXULElement('label');
label.setAttribute('data-l10n-id', 'advanced-search-binding-hint-' + level);
let button = document.createXULElement('button');
button.setAttribute('data-l10n-id', 'advanced-search-bind-same-' + level);
button.addEventListener('command', () => this.rootGroup.bindSameEntity(level));
row.append(label, button);
hint.append(row);
}
hint.hidden = false;
}
// Regenerate the search's flat condition list from the current tree. The DOM is
@ -137,14 +238,23 @@
var flat = [];
this.collectGroup(this.rootGroup, flat, true);
// Search-global options
for (let name of ['recursive', 'noChildren', 'includeParentsAndChildren']) {
if (this.querySelector('#' + name + 'Checkbox').checked) {
flat.push({ condition: name, operator: 'true', value: null });
}
// Search-global options. noChildren is no longer emitted here -- it's carried by
// the result level (resultLevel = item). includeParentsAndChildren is emitted only when
// its legacy checkbox is present and still checked, so unchecking it drops it.
if (this.querySelector('#recursiveCheckbox').checked) {
flat.push({ condition: 'recursive', operator: 'true', value: null });
}
if (this.querySelector('#includeParentsAndChildrenCheckbox').checked) {
flat.push({ condition: 'includeParentsAndChildren', operator: 'true', value: null });
}
this.rebuildConditions(flat);
// Any mutation runs through here (including paths whose menus stopPropagation, like
// changing or removing a condition), so refresh the derived UI from one place
this.updateBindingMenus();
this.updateBindingHint();
this.updateLevelWarning();
}
// Append a group's serialized form to `flat`. The root contributes its
@ -157,6 +267,11 @@
if (group.joinMode == 'any') {
flat.push({ condition: 'joinMode', operator: 'any', value: null });
}
// A concrete result level is emitted as a marker inside the group, like joinMode; 'any'
// (the default) is omitted
if (group.resultLevel && group.resultLevel != 'any') {
flat.push({ condition: 'resultLevel', operator: group.resultLevel, value: null });
}
for (let child of group.conditionsContainer.children) {
if (child.localName == 'zoterosearchcondition') {
let data = child.getConditionData();
@ -299,6 +414,16 @@
content = MozXULElement.parseXULToFragment(`
<groupbox class="search-condition-group">
<caption align="center">
<label class="result-level-prefix"/>
<menulist class="result-level-menu" native="true" data-l10n-id="advanced-search-result-level-menu">
<menupopup>
<menuitem value="any" data-l10n-id="advanced-search-result-level-any" selected="true"/>
<menuitem value="item" data-l10n-id="advanced-search-result-level-item"/>
<menuitem value="attachment" data-l10n-id="advanced-search-result-level-attachment"/>
<menuitem value="note" data-l10n-id="advanced-search-result-level-note"/>
<menuitem value="annotation" data-l10n-id="advanced-search-result-level-annotation"/>
</menupopup>
</menulist>
<label class="join-mode-prefix" value="&zotero.search.joinMode.prefix;"/>
<menulist class="join-mode-menu" native="true" aria-label="&zotero.search.joinMode.prefix;">
<menupopup>
@ -306,6 +431,10 @@
<menuitem label="&zotero.search.joinMode.all;" value="all" selected="true"/>
</menupopup>
</menulist>
<label class="join-mode-following" data-l10n-id="advanced-search-of-the-following" hidden="true"/>
<menulist class="binding-menu" native="true" hidden="true" data-l10n-id="advanced-search-binding-menu">
<menupopup/>
</menulist>
<label class="join-mode-suffix" value="&zotero.search.joinMode.suffix;"/>
<spacer flex="1"/>
<hbox class="group-actions">
@ -315,12 +444,53 @@
</hbox>
</caption>
<vbox class="conditions"/>
<hbox class="level-warning" hidden="true">
<description/>
</hbox>
</groupbox>
`, ['chrome://zotero/locale/zotero.dtd', 'chrome://zotero/locale/searchbox.dtd']);
init() {
this.joinMenu = this.querySelector('.join-mode-menu');
this.resultLevelMenu = this.querySelector('.result-level-menu');
this.bindingMenu = this.querySelector('.binding-menu');
this.conditionsContainer = this.querySelector('.conditions');
// The group's own warning element, stashed at init to avoid re-querying.
this.levelWarning = this.querySelector('.level-warning');
// The result level is tracked here and reflected to whichever control is active: the root's
// result-level menu ("Find ..."), or a nested group's binding menu ("... in the
// same attachment"). collectGroup/renderConditions read and write `resultLevel`.
this._resultLevel = 'any';
// The root surfaces the result-level menu; a nested group hides it and instead
// shows a binding menu (built on demand by updateBindingMenu) when it holds
// conditions that can be bound to the same descendant.
this.resultLevelMenu.value = 'any';
var scopePrefix = this.querySelector('.result-level-prefix');
if (this.isRoot) {
// Read as one sentence: "Find [Top-level items] matching [all] of the following:"
scopePrefix.setAttribute('data-l10n-id', 'advanced-search-result-level-prefix-root');
this.querySelector('.join-mode-prefix').setAttribute('data-l10n-id', 'advanced-search-join-prefix-root');
this.resultLevelControl = this.resultLevelMenu;
}
else {
// Nested: "Match [all] of the following:" -- the result level lives on the
// root. The binding menu (and its hiding of the suffix) is set up in
// updateBindingMenu().
this.resultLevelMenu.hidden = true;
scopePrefix.hidden = true;
this.resultLevelControl = this.bindingMenu;
}
// Keep the stored result level in sync when the user changes the control (the
// command target may be the menulist or a menuitem inside it), so a nested
// binding that later hides still round-trips its last value
this.addEventListener('command', (event) => {
if (this.resultLevelControl && this.resultLevelControl.contains(event.target)) {
this._resultLevel = this.resultLevelControl.value || 'any';
}
});
// At init the group has no nested groups yet, so these resolve to its own
// caption buttons
this.addConditionButton = this.querySelector('.add-condition');
@ -357,8 +527,221 @@
this.joinMenu.value = val;
}
// The group's result level: 'any' (no level constraint -- mixed result for the root,
// plain grouping for a nested group) or a concrete 'item'/'attachment'/'note'/
// 'annotation' level for cross-level mapping. The active control's current
// selection is the source of truth; fall back to the stored value for a nested
// binding menu that's hidden (it has no options to read).
get resultLevel() {
if (this.resultLevelControl && !this.resultLevelControl.hidden) {
return this.resultLevelControl.value || 'any';
}
return this._resultLevel;
}
set resultLevel(val) {
this._resultLevel = val || 'any';
// Reflect to the active control if it currently offers a matching option; the
// binding menu's options are (re)built by updateBindingMenu()
let popup = this.resultLevelControl && this.resultLevelControl.querySelector('menupopup');
if (popup && [...popup.children].some(item => item.value == this._resultLevel)) {
this.resultLevelControl.value = this._resultLevel;
}
}
// Build the nested-group binding menu ("... in the same attachment"), shown only
// when binding is meaningful: 2+ conditions sharing a level below the result level.
updateBindingMenu() {
if (this.isRoot) {
return;
}
let resultLevel = 'any';
if (this.searchElement && this.searchElement.rootGroup) {
resultLevel = this.searchElement.rootGroup.resultLevel;
}
// Count this group's direct condition rows by level, keeping only levels below the
// result level -- those are what a group can bind to the same entity
let counts = {};
for (let row of this.conditionsContainer.children) {
// Skip rows the user hasn't filled in yet, so a freshly added (or just-retyped)
// condition doesn't trigger the hint until it actually has a value
if (row.localName != 'zoterosearchcondition' || !row.isPopulated()) {
continue;
}
let level = row.conditionLevel;
if (Zotero.Search._isAncestorLevel(resultLevel, level)) {
counts[level] = (counts[level] || 0) + 1;
}
}
let levels = Object.keys(counts);
// Binding is only meaningful when some level has 2+ conditions. When it isn't,
// hide the menu but preserve any stored level (a single descendant condition
// maps the same way bound or not, so it round-trips losslessly).
if (!levels.some(l => counts[l] >= 2)) {
this.bindingMenu.hidden = true;
// Plain group: "Match [all] of the following:" (the suffix carries the colon)
this.querySelector('.join-mode-suffix').hidden = false;
this.querySelector('.join-mode-following').hidden = true;
this._bindingMenuKey = null;
return;
}
// Drop a stored binding whose level is no longer offered
if (this._resultLevel != 'any' && !levels.includes(this._resultLevel)) {
this._resultLevel = 'any';
}
// Rebuild the popup only when its option set changes. Rebuilding it on every refresh
// would replace the menuitems mid-selection -- when the change came from this menu
// itself -- and wedge the drop-down.
let optionLevels = ['attachment', 'note', 'annotation'].filter(l => levels.includes(l));
let key = optionLevels.join(',');
if (key !== this._bindingMenuKey) {
this._bindingMenuKey = key;
let popup = this.bindingMenu.querySelector('menupopup');
popup.replaceChildren();
let separate = document.createXULElement('menuitem');
separate.setAttribute('value', 'any');
separate.setAttribute('data-l10n-id', 'advanced-search-binding-separate');
popup.append(separate);
for (let level of optionLevels) {
let item = document.createXULElement('menuitem');
item.setAttribute('value', level);
item.setAttribute('data-l10n-id', 'advanced-search-binding-same-' + level);
popup.append(item);
}
}
this.bindingMenu.hidden = false;
// Bound group: "Match [all] of the following in the same attachment". The binding
// phrase ends the caption, so swap the legacy "of the following:" (with its colon)
// for the colon-less "of the following" that precedes the binding menu.
this.querySelector('.join-mode-suffix').hidden = true;
this.querySelector('.join-mode-following').hidden = false;
this.bindingMenu.value = this._resultLevel;
}
// The level this group's conditions are actually matched at: its own result level (the
// result type for the root, the binding for a nested group) if set, otherwise the level
// it inherits from its enclosing group. Mirrors the engine, where an unbound
// ("separately") group maps its conditions to the parent's level rather than
// combining them at no level.
effectiveLevel() {
if (this.resultLevel != 'any') {
return this.resultLevel;
}
let parent = this.parentElement && this.parentElement.closest('search-condition-group');
return parent ? parent.effectiveLevel() : this.resultLevel;
}
// Warn when this group's own conditions can never combine: a child whose level can't
// reach the group's effective level, or -- with no level anywhere up the chain (a mixed
// result type) -- an "all" of children on different item-hierarchy branches. Each group
// flags only its own conditions, so the message sits where the problem is.
updateLevelWarning() {
let ownLevel = this.resultLevel;
let level = this.effectiveLevel();
// This group's direct children's levels: a populated condition row's own level (an
// empty row doesn't warn until it's filled in) or a nested group's binding. 'any'
// combines with anything, so drop it.
let childLevels = [...this.conditionsContainer.children].map((child) => {
if (child.localName == 'zoterosearchcondition') {
return child.isPopulated() ? child.conditionLevel : null;
}
if (child.localName == 'search-condition-group') {
return child.resultLevel;
}
return null;
}).filter(l => l && l != 'any');
let messageID = null;
let args = null;
let resultTypeArgs = () => {
let item = this.resultLevelMenu.querySelector('menuitem[value="item"]');
return { topLevelItems: item ? item.getAttribute('label') : 'top-level items' };
};
if (level != 'any') {
// A child that can't reach the effective level can never match here
if (childLevels.some(l => !this.levelsCombine(l, level))) {
if (!this.isRoot && ownLevel != 'any') {
// This group's own binding is the constraint, so "match separately" fixes it
messageID = 'advanced-search-group-warning-unreachable';
args = { entity: ownLevel };
}
else {
// The result type (this group's, or one it inherits) is the constraint
messageID = 'advanced-search-level-warning-unreachable';
args = resultTypeArgs();
}
}
}
else if (this.joinMode == 'all' && new Set(childLevels).size >= 2) {
// No level anywhere up the chain (mixed result type): ANDing conditions on
// different branches can never all match
let anyItem = this.joinMenu.querySelector('menuitem[value="any"]');
let matchAny = anyItem ? anyItem.getAttribute('label') : 'any';
if (this.isRoot) {
messageID = 'advanced-search-level-warning-mixed';
args = { matchAny, ...resultTypeArgs() };
}
else {
// Reachable only when the result type is "any", so setting one fixes it too
messageID = 'advanced-search-group-warning-mixed';
args = { matchAny, ...resultTypeArgs() };
}
}
// Only touch the DOM when the message changes, so re-running on every keystroke
// doesn't re-translate the string and make the warning flicker.
let key = messageID ? messageID + '\n' + JSON.stringify(args) : '';
if (key === this._levelWarningKey) {
return;
}
this._levelWarningKey = key;
if (messageID) {
document.l10n.setAttributes(this.levelWarning.querySelector('description'), messageID, args);
}
this.levelWarning.hidden = !messageID;
}
// Two levels combine if one is an ancestor of the other (or equal); 'any' matches any.
levelsCombine(a, b) {
return a == 'any' || b == 'any' || a == b
|| Zotero.Search._isAncestorLevel(a, b) || Zotero.Search._isAncestorLevel(b, a);
}
// Wrap this group's direct condition rows that match `level` into a new child group
// bound to that level ("the same attachment"). Used by the discoverability hint.
// Rows are rebuilt from their data rather than moved, since detaching a custom element
// wipes its contents.
bindSameEntity(level) {
let rows = [...this.conditionsContainer.children].filter(
row => row.localName == 'zoterosearchcondition' && row.conditionLevel == level);
if (rows.length < 2) {
return;
}
let newGroup = document.createXULElement('search-condition-group');
this.conditionsContainer.insertBefore(newGroup, rows[0]);
for (let row of rows) {
let data = row.getConditionData();
let ref;
if (data) {
let [condition, mode] = Zotero.SearchConditions.parseCondition(data.condition);
ref = { id: undefined, condition, mode, operator: data.operator, value: data.value, required: false };
}
newGroup.addCondition(ref);
row.remove();
}
newGroup.resultLevel = level;
let search = this.searchElement;
search.updateSearch();
search.updateRemoveButtons();
search.updateBindingMenus();
search.updateBindingHint();
}
clear() {
this.joinMode = 'all';
this.resultLevel = 'any';
while (this.conditionsContainer.firstChild) {
this.conditionsContainer.removeChild(this.conditionsContainer.firstChild);
}
@ -757,6 +1140,14 @@
document.l10n.setAttributes(valueMenu, 'advanced-search-condition-input', { label: valueMenu.label });
}
this.updateMenuCheckboxesRecursive(operatorsList, operatorsList.selectedItem.getAttribute('value'));
// Changing the condition or operator is a mutation like add/remove, so rebuild the
// search and refresh the derived UI (binding hint, level warning) right away. The
// condition/operator menus stopPropagation, so this won't happen via event bubbling.
// (updateSearch no-ops during the initial render via its own guard.)
if (this.parent) {
this.parent.updateSearch();
}
}
createValueMenu(rows) {
@ -1079,6 +1470,13 @@
newGroup.conditionsContainer.firstElementChild.querySelector('#conditionsmenu').focus();
}
// The item level this condition matches at ('item' by default), used to decide
// cross-level binding in a group
get conditionLevel() {
let data = this.selectedCondition && Zotero.SearchConditions.get(this.selectedCondition);
return (data && data.level) || 'item';
}
// Whether a value has been entered, used to decide whether the last
// remaining condition can be cleared back to the default state
isPopulated() {

View file

@ -909,6 +909,57 @@ advanced-search-group-btn =
.tooltiptext = Add Condition Group
advanced-search-remove-group-btn =
.tooltiptext = Remove Group
advanced-search-result-level-menu =
.aria-label = Result type
advanced-search-result-level-prefix-root =
.value = Find
advanced-search-join-prefix-root =
.value = matching
advanced-search-result-level-any =
.label = any items
advanced-search-result-level-item =
.label = top-level items
advanced-search-result-level-attachment =
.label = attachments
advanced-search-result-level-note =
.label = notes
advanced-search-result-level-annotation =
.label = annotations
advanced-search-binding-menu =
.aria-label = Match against the same item
advanced-search-binding-separate =
.label = separately
advanced-search-binding-same-attachment =
.label = in the same attachment
advanced-search-binding-same-note =
.label = in the same note
advanced-search-binding-same-annotation =
.label = in the same annotation
# Shown before the binding menu so a bound group reads "Match all of the following in the same
# annotation" (the legacy "of the following:" -- with the colon -- is kept for a plain group)
advanced-search-of-the-following =
.value = of the following
advanced-search-binding-hint-attachment =
.value = These conditions can match separate attachments.
advanced-search-binding-hint-note =
.value = These conditions can match separate notes.
advanced-search-binding-hint-annotation =
.value = These conditions can match separate annotations.
advanced-search-level-warning-mixed = These conditions cannot all match the same item, so this search will never return results. Try matching “{ $matchAny }” of them, or set the result type to “{ $topLevelItems }”.
advanced-search-level-warning-unreachable = This search has a condition that cannot apply to the chosen result type. Set the result type to “{ $topLevelItems }” or remove the incompatible condition.
advanced-search-group-warning-unreachable =
A condition here cannot be in the same { $entity ->
[attachment] attachment
[note] note
*[annotation] annotation
}. Match these separately or remove the incompatible condition.
advanced-search-group-warning-mixed = These conditions cannot all match the same item, so this group will never match. Try matching “{ $matchAny }” of them, or set the result type to “{ $topLevelItems }”.
advanced-search-bind-same-attachment =
.label = Match the same attachment
advanced-search-bind-same-note =
.label = Match the same note
advanced-search-bind-same-annotation =
.label = Match the same annotation
advanced-search-conditions-menu =
.aria-label = Search condition
.label = { $label }

View file

@ -12,7 +12,7 @@ advanced-search-deck {
advanced-search-pane {
@include inactive-opacity;
flex-direction: column;
// Inline padding is further down the tree so focus rings work with overflow: auto
padding-block: 8px;

View file

@ -10,6 +10,9 @@ zoterosearch {
gap: 8px;
overflow: auto;
max-height: 33vh;
// Keep the whole builder a comfortable width on a wide window (left-aligned) rather
// than stretching the rows across the full width. The value field flexes within this.
max-width: 64em;
@include macOS-normalize-controls;
// Use the full pane width
@ -23,6 +26,9 @@ zoterosearch {
// Zero the toolkit groupbox margin/padding
margin: 0;
padding: 0;
// The XUL groupbox shrink-wraps to content by default; fill its parent so the
// conditions box (and the value field) can use the available width
width: -moz-available;
.conditions {
min-width: fit-content; // Grow with content, don't overflow
@ -118,6 +124,76 @@ zoterosearch {
}
}
.level-warning {
// Each group's own warning, in a contained box below its conditions (the root's sits at
// the bottom; a nested group's sits inside its box) rather than as loose text. A light
// red callout mirrors the binding hint's light blue one, but at full size (vs the hint's
// smaller text) since this is a hard error -- the search can never match -- not a hint.
color: var(--accent-red);
// 10px inline is a small visual nudge to bring the text slightly in (it doesn't line up
// with anything exactly); 8px block keeps the two-line message from cramping.
padding: 8px 10px;
background: var(--accent-red10);
border-radius: 6px;
// Fill the pane width but contribute no intrinsic (max-content) width of its own, so
// showing/hiding this notice can't widen the search pane and shrink the collections
// pane. (Its long text would otherwise push the pane to its max-width.)
box-sizing: border-box;
width: 0;
min-width: 100%;
description {
// Wrap to the available width instead of overflowing the pane; min-width: 0 lets
// it shrink within the flex row rather than contributing an intrinsic width
flex: 1;
min-width: 0;
margin: 0;
white-space: normal;
}
}
// A nested group's warning sits inside the outer box, so leave room below it rather than
// letting it butt against the box's bottom edge. (The root's warning is below the box.)
search-condition-group:not([root]) .level-warning {
margin-block-end: 6px;
}
#search-binding-hint {
// A distinct suggestion callout under the conditions (not plain body text): a light
// blue, rounded block with slightly smaller text, one "[statement] [button]" line per
// bindable level. Fills the pane width but contributes no intrinsic width (like the
// warning) so its line can't widen the search pane.
display: flex;
flex-direction: column;
// Each row hugs its content (so the button sits right next to the text) while the
// box itself still fills the pane width
align-items: flex-start;
gap: 8px;
padding: 8px;
font-size: 0.9em;
background: var(--accent-blue10);
border-radius: 6px;
box-sizing: border-box;
width: 0;
min-width: 100%;
hbox {
display: flex;
align-items: center;
gap: 5px;
}
button {
margin: 0;
}
// The explicit `display: flex` above overrides the `hidden` attribute's display:none,
// so re-assert it -- otherwise the empty box shows when there's nothing to suggest
&[hidden] {
display: none;
}
}
#search-option-checkboxes {
flex-direction: row;
flex-wrap: wrap;
@ -144,6 +220,13 @@ zoterosearch {
label {
margin: 0;
}
// Native menulists sit a couple pixels high relative to the inline label text
// ("Find ... matching ... of the following:"); baseline/align-self alignment is
// locked by the platform, so nudge them down to sit on the text
menulist {
margin-block-start: 2px;
}
}
label:first-child:not(tooltip label), checkbox:first-child {
@ -180,9 +263,25 @@ zoterosearch {
position: absolute;
}
.valuefield, .valuemenu, .value-date-age {
.valuemenu, .value-date-age {
flex: 1;
}
// The text value field grows to use the available row width (the overall cap is on
// the builder, not here). It's a flex container itself so the inner stack/input
// fills the host.
.valuefield {
display: flex;
flex: 1;
min-width: 12em;
// The `display: flex` above overrides the `hidden` attribute's display:none, so
// re-assert it -- otherwise the textbox shows alongside the value menu for
// menu-based conditions (Collection, Item Type, Attachment File Type)
&[hidden] {
display: none;
}
}
}
.join-mode-menu {
@ -193,9 +292,15 @@ zoterosearch {
width: 15em;
}
#operatorsmenu, #valuemenu, #valuefield, .search-in-the-last {
#operatorsmenu, #valuemenu, .search-in-the-last {
width: 12em;
}
// Fill the flexed value field (host -> stack -> input)
#search-textbox {
flex: 1;
width: 100%;
}
#valuemenu::part(icon) {
max-height: 16px;

View file

@ -8,6 +8,7 @@ $-colors: (
accent-green: #39bf68d9,
accent-orange: #ff794cd9,
accent-red: #db2c3ae5,
accent-red10: #db2c3a4d,
accent-teal: #59adc4e5,
accent-white: #fff,
accent-wood-dark: #996b6f,

View file

@ -8,6 +8,7 @@ $-colors: (
accent-green: #39bf68,
accent-orange: #ff794c,
accent-red: #db2c3a,
accent-red10: #db2c3a1a,
accent-teal: #59adc4,
accent-white: #fff,
accent-wood-dark: #996b6f,

View file

@ -52,6 +52,56 @@ describe("Advanced Search", function () {
await otherItem.eraseTx();
});
it("should run a cross-level search across a multi-collection selection", async function () {
var word = 'zmc' + Zotero.Utilities.randomString();
var makeMatch = async function (collection) {
var item = await createDataObject('item', { collections: [collection.id] });
var attachment = await importPDFAttachment(item);
await createAnnotation('highlight', attachment, { comment: 'foo ' + word + ' bar' });
return item;
};
var c1 = await createDataObject('collection');
var c2 = await createDataObject('collection');
var c3 = await createDataObject('collection');
var itemA = await makeMatch(c1);
var itemB = await makeMatch(c2);
var itemC = await makeMatch(c3); // in an unselected collection
// Select c1 and c2, leaving c3 out
var cv = zp.collectionsView;
await cv.selectByID("C" + c1.id);
await waitForItemsLoad(win);
cv.selection.toggleSelect(cv.getRowIndexByID("C" + c2.id));
await zp.onCollectionSelected();
await zp.itemsView.waitForLoad();
// Top-level items with a descendant annotation matching the comment
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('resultLevel', 'item');
s.addCondition('annotationComment', 'contains', word);
var iv = zp.itemsView;
await iv.setFilter('advanced-search', s);
await iv.waitForLoad();
// The view merges getItems() across the selected rows (collectionViewItemTree),
// so the cross-level search runs scoped to each collection and the results union:
// both selected collections' matching items, but not the unselected one's
var rows = zp.getCollectionTreeRows();
var ids = new Set();
for (let arr of await Promise.all(rows.map(row => row.getItems()))) {
for (let item of arr) ids.add(item.id);
}
assert.sameMembers([...ids], [itemA.id, itemB.id]);
await iv.setFilter('advanced-search', null);
await selectLibrary(win);
await Zotero.Items.erase([itemA.id, itemB.id, itemC.id]);
await Zotero.Collections.erase([c1.id, c2.id, c3.id]);
});
it("shouldn't show trashed items outside the trash", async function () {
var item = await createDataObject('item', { setTitle: true });
item.deleted = true;
@ -486,6 +536,23 @@ describe("Advanced Search", function () {
// Focus moves to the new condition's drop-down
assert.equal(win.document.activeElement, newRow.querySelector('#conditionsmenu'));
});
it("should hide the value textbox for a menu-based condition", async function () {
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('title', 'is', '');
pane.search = s;
var row = conditions.firstChild;
row.onConditionSelected('fileTypeID');
// File Type uses a value menu, so the textbox must actually be hidden, not just
// sitting alongside the menu
var valuefield = row.querySelector('#valuefield');
assert.isTrue(valuefield.hidden);
assert.equal(win.getComputedStyle(valuefield).display, 'none');
assert.isFalse(row.querySelector('#valuemenu').hidden);
});
describe("Find-as-you-type", function () {
function typeInMenu(menu, str) {
@ -989,6 +1056,295 @@ describe("Advanced Search", function () {
assert.equal(conditions.firstChild.querySelector('#valuefield').value, '');
});
it("should default a fresh search to the top-level item result level", function () {
// Opening a new (unseeded) advanced search defaults the result level to
// top-level items, so child conditions map up without grouping
pane.search = null;
assert.equal(searchBox.rootGroup.resultLevel, 'item');
});
it("should render and serialize the root result level", function () {
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('resultLevel', 'annotation');
s.addCondition('annotationText', 'contains', 'foo');
pane.search = s;
assert.equal(searchBox.rootGroup.resultLevel, 'annotation');
searchBox.updateSearch();
var sequence = Object.values(searchBox.search.getConditions())
.map(c => c.condition);
assert.deepEqual(sequence, ['resultLevel', 'annotationText']);
var scopeCond = Object.values(searchBox.search.getConditions())
.find(c => c.condition == 'resultLevel');
assert.equal(scopeCond.operator, 'annotation');
});
it("should render and serialize a nested group result level", function () {
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('creator', 'contains', 'Smith');
s.addCondition('groupStart', 'true', '');
s.addCondition('resultLevel', 'annotation');
s.addCondition('annotationText', 'contains', 'foo');
s.addCondition('groupEnd', 'true', '');
pane.search = s;
var group = conditions.querySelector('search-condition-group');
assert.equal(group.resultLevel, 'annotation');
searchBox.updateSearch();
var sequence = Object.values(searchBox.search.getConditions())
.map(c => c.condition);
assert.deepEqual(sequence,
['creator', 'groupStart', 'resultLevel', 'annotationText', 'groupEnd']);
});
it("should show a binding menu for a group of same-level descendant conditions", function () {
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('resultLevel', 'item'); // result level
s.addCondition('groupStart', 'true', '');
s.addCondition('annotationText', 'contains', 'foo');
s.addCondition('annotationComment', 'contains', 'bar');
s.addCondition('groupEnd', 'true', '');
pane.search = s;
var group = conditions.querySelector('search-condition-group');
assert.isFalse(group.bindingMenu.hidden);
var values = [...group.bindingMenu.querySelectorAll('menuitem')].map(i => i.value);
assert.includeMembers(values, ['any', 'annotation']);
// No attachment conditions, so it isn't offered
assert.notInclude(values, 'attachment');
// Bind to the same annotation and confirm it serializes
group.resultLevel = 'annotation';
searchBox.updateSearch();
var seq = Object.values(searchBox.search.getConditions()).map(c => c.condition);
assert.deepEqual(seq,
['resultLevel', 'groupStart', 'resultLevel', 'annotationText', 'annotationComment', 'groupEnd']);
var groupScope = Object.values(searchBox.search.getConditions())
.filter(c => c.condition == 'resultLevel')[1];
assert.equal(groupScope.operator, 'annotation');
});
it("should not show a binding menu for a group of item-level conditions", function () {
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('resultLevel', 'item');
s.addCondition('groupStart', 'true', '');
s.addCondition('title', 'contains', 'a');
s.addCondition('title', 'contains', 'b');
s.addCondition('groupEnd', 'true', '');
pane.search = s;
var group = conditions.querySelector('search-condition-group');
assert.isTrue(group.bindingMenu.hidden);
});
it("should render a stored group binding into the menu", function () {
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('resultLevel', 'item');
s.addCondition('groupStart', 'true', '');
s.addCondition('resultLevel', 'annotation');
s.addCondition('annotationText', 'contains', 'foo');
s.addCondition('annotationComment', 'contains', 'bar');
s.addCondition('groupEnd', 'true', '');
pane.search = s;
var group = conditions.querySelector('search-condition-group');
assert.isFalse(group.bindingMenu.hidden);
assert.equal(group.resultLevel, 'annotation');
assert.equal(group.bindingMenu.value, 'annotation');
});
it("should offer a binding hint for ungrouped sibling descendant conditions", function () {
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('resultLevel', 'item');
s.addCondition('annotationText', 'contains', 'foo');
s.addCondition('annotationComment', 'contains', 'bar');
pane.search = s;
var hint = searchBox.querySelector('#search-binding-hint');
assert.isFalse(hint.hidden);
// One bindable level (annotation), so one suggestion line with one button
assert.lengthOf([...hint.querySelectorAll('button')], 1);
});
it("should not offer a binding hint for item-level sibling conditions", function () {
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('resultLevel', 'item');
s.addCondition('title', 'contains', 'a');
s.addCondition('title', 'contains', 'b');
pane.search = s;
assert.isTrue(searchBox.querySelector('#search-binding-hint').hidden);
});
it("should not offer a binding hint for an unpopulated condition", function () {
// One populated annotation condition plus an empty one: not enough to suggest
// binding until the second is actually filled in
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('resultLevel', 'item');
s.addCondition('annotationText', 'contains', 'foo');
s.addCondition('annotationComment', 'contains', '');
pane.search = s;
assert.isTrue(searchBox.querySelector('#search-binding-hint').hidden);
});
it("should wrap conditions into a bound group when the hint is taken", function () {
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('resultLevel', 'item');
s.addCondition('annotationText', 'contains', 'foo');
s.addCondition('annotationComment', 'contains', 'bar');
pane.search = s;
searchBox.rootGroup.bindSameEntity('annotation');
// The two conditions are now one group bound to annotation
var group = conditions.querySelector('search-condition-group');
assert.ok(group);
assert.equal(group.resultLevel, 'annotation');
assert.lengthOf(
[...group.conditionsContainer.children].filter(c => c.localName == 'zoterosearchcondition'),
2);
searchBox.updateSearch();
var seq = Object.values(searchBox.search.getConditions()).map(c => c.condition);
assert.deepEqual(seq,
['resultLevel', 'groupStart', 'resultLevel', 'annotationText', 'annotationComment', 'groupEnd']);
// Conditions are no longer ungrouped siblings, so the hint is gone
assert.isTrue(searchBox.querySelector('#search-binding-hint').hidden);
});
it("should fold a legacy noChildren into the result level", function () {
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('noChildren', 'true');
s.addCondition('title', 'contains', 'foo');
pane.search = s;
// Shown as result level = top-level items, no separate checkbox
assert.equal(searchBox.rootGroup.resultLevel, 'item');
searchBox.updateSearch();
var seq = Object.values(searchBox.search.getConditions()).map(c => c.condition);
assert.notInclude(seq, 'noChildren');
assert.deepEqual(seq, ['resultLevel', 'title']);
});
it("should warn when ALL conditions can't match the same item at a mixed result level", function () {
// No result type (mixed), so an item-level and an annotation-level condition
// can't both be true of one row
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('title', 'contains', 'a');
s.addCondition('annotationText', 'contains', 'b');
pane.search = s;
assert.isFalse(searchBox.querySelector('.level-warning').hidden);
});
it("should not warn when a result type lets the conditions combine", function () {
// Result type item: the annotation condition maps up, so it's satisfiable
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('resultLevel', 'item');
s.addCondition('title', 'contains', 'a');
s.addCondition('annotationText', 'contains', 'b');
pane.search = s;
assert.isTrue(searchBox.querySelector('.level-warning').hidden);
});
it("should warn when a condition can't reach the result type", function () {
// A note can never be (or be under) an attachment
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('resultLevel', 'attachment');
s.addCondition('note', 'contains', 'a');
pane.search = s;
assert.isFalse(searchBox.querySelector('.level-warning').hidden);
});
it("should warn on the group when a condition can't reach its binding", function () {
// A group bound to "same annotation" with a note condition: the conflict is the
// group's binding, so the warning belongs on the group, not at the root.
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('resultLevel', 'item');
s.addCondition('groupStart', 'true', '');
s.addCondition('resultLevel', 'annotation');
s.addCondition('annotationComment', 'contains', 'a');
s.addCondition('note', 'contains', 'b');
s.addCondition('groupEnd', 'true', '');
pane.search = s;
var group = conditions.querySelector('search-condition-group');
assert.isFalse(group.levelWarning.hidden);
assert.isTrue(searchBox.rootGroup.levelWarning.hidden);
});
it("should not warn for a \"separately\" group whose conditions roll up", function () {
// "Separately" inherits the result level (item), so annotations and a note all
// roll up independently -- satisfiable, no warning (so "match separately" really
// does clear the bound-group warning)
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('resultLevel', 'item');
s.addCondition('groupStart', 'true', '');
s.addCondition('annotationComment', 'contains', 'a');
s.addCondition('annotationComment', 'contains', 'b');
s.addCondition('note', 'contains', 'c');
s.addCondition('groupEnd', 'true', '');
pane.search = s;
assert.isTrue(conditions.querySelector('search-condition-group').levelWarning.hidden);
});
it("should not warn for an ordinary search", function () {
pane.search = null;
assert.isTrue(searchBox.querySelector('.level-warning').hidden);
});
it("should keep a legacy includeParentsAndChildren editable and round-tripping", function () {
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('title', 'contains', 'foo');
s.addCondition('includeParentsAndChildren', 'true');
pane.search = s;
assert.isFalse(searchBox.querySelector('#search-legacy-options').hidden);
assert.isTrue(searchBox.querySelector('#includeParentsAndChildrenCheckbox').checked);
searchBox.updateSearch();
var seq = Object.values(searchBox.search.getConditions()).map(c => c.condition);
assert.include(seq, 'includeParentsAndChildren');
});
it("should drop includeParentsAndChildren when its legacy checkbox is unchecked", function () {
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;
s.addCondition('title', 'contains', 'foo');
s.addCondition('includeParentsAndChildren', 'true');
pane.search = s;
searchBox.querySelector('#includeParentsAndChildrenCheckbox').checked = false;
searchBox.updateSearch();
var seq = Object.values(searchBox.search.getConditions()).map(c => c.condition);
assert.notInclude(seq, 'includeParentsAndChildren');
});
it("should add a sibling condition outside the group from the group's +", function () {
var s = new Zotero.Search();
s.libraryID = Zotero.Libraries.userLibraryID;