Implement #2786: Allow selection of multiple groups

CHANGELOG.md

@@ -11,11 +11,15 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
 ## [Unreleased]
 
 ### Changed
-- Continued to redesign the user interface: this time the editor got a fresh coat of paint:
+- We continued to improve the new groups interface:
+  - You can now again select multiple groups (and a few related settings were added to the preferences) [#2786](https://github.com/JabRef/jabref/issues/2786).
+- The entry editor got a fresh coat of paint:
+  - Homogenize the size of text fields.
   - The buttons were changed to icons.
+  - Completely new interface to add or modify linked files.
   - Removed the hidden feature that a double click in the editor inserted the current date.
 - All authors and editors are separated using semicolons when exporting to csv. [#2762](https://github.com/JabRef/jabref/issues/2762)
-- Improved wording of "Show recommendationns: into "Show 'Related Articles' tab" in the preferences
+- Improved wording of "Show recommendations: into "Show 'Related Articles' tab" in the preferences
 
 ### Fixed
 - We fixed the IEEE Xplore web search functionality [#2789](https://github.com/JabRef/jabref/issues/2789)

src/main/java/org/jabref/gui/BasePanel.java

@@ -2151,7 +2151,7 @@ public void listen(EntryAddedEvent addedEntryEvent) {
             if (Globals.prefs.getBoolean(JabRefPreferences.AUTO_ASSIGN_GROUP)
                     && frame.getGroupSelector().getToggleAction().isSelected()) {
                 final List<BibEntry> entries = Collections.singletonList(addedEntryEvent.getBibEntry());
-                Globals.stateManager.getSelectedGroup(bibDatabaseContext).ifPresent(
+                Globals.stateManager.getSelectedGroup(bibDatabaseContext).forEach(
                         selectedGroup -> selectedGroup.addEntriesToGroup(entries));
                 SwingUtilities.invokeLater(() -> BasePanel.this.getGroupSelector().valueChanged(null));
             }

src/main/java/org/jabref/gui/StateManager.java

@@ -7,8 +7,8 @@
 
 import javafx.beans.binding.Bindings;
 import javafx.beans.property.ObjectProperty;
-import javafx.beans.property.ReadOnlyObjectProperty;
-import javafx.beans.property.ReadOnlyObjectWrapper;
+import javafx.beans.property.ReadOnlyListProperty;
+import javafx.beans.property.ReadOnlyListWrapper;
 import javafx.beans.property.SimpleObjectProperty;
 import javafx.collections.FXCollections;
 import javafx.collections.ObservableList;
@@ -33,21 +33,21 @@
 public class StateManager {
 
     private final ObjectProperty<Optional<BibDatabaseContext>> activeDatabase = new SimpleObjectProperty<>(Optional.empty());
-    private final ReadOnlyObjectWrapper<Optional<GroupTreeNode>> activeGroup = new ReadOnlyObjectWrapper<>(Optional.empty());
+    private final ReadOnlyListWrapper<GroupTreeNode> activeGroups = new ReadOnlyListWrapper<>(FXCollections.observableArrayList());
     private final ObservableList<BibEntry> selectedEntries = FXCollections.observableArrayList();
-    private final ObservableMap<BibDatabaseContext, GroupTreeNode> selectedGroups = FXCollections.observableHashMap();
+    private final ObservableMap<BibDatabaseContext, ObservableList<GroupTreeNode>> selectedGroups = FXCollections.observableHashMap();
 
     public StateManager() {
         MonadicBinding<BibDatabaseContext> currentDatabase = EasyBind.map(activeDatabase, database -> database.orElse(null));
-        activeGroup.bind(EasyBind.map(Bindings.valueAt(selectedGroups, currentDatabase), Optional::ofNullable));
+        activeGroups.bind(Bindings.valueAt(selectedGroups, currentDatabase));
     }
 
     public ObjectProperty<Optional<BibDatabaseContext>> activeDatabaseProperty() {
         return activeDatabase;
     }
 
-    public ReadOnlyObjectProperty<Optional<GroupTreeNode>> activeGroupProperty() {
-        return activeGroup.getReadOnlyProperty();
+    public ReadOnlyListProperty<GroupTreeNode> activeGroupProperty() {
+        return activeGroups.getReadOnlyProperty();
     }
 
     public ObservableList<BibEntry> getSelectedEntries() {
@@ -58,16 +58,17 @@ public void setSelectedEntries(List<BibEntry> newSelectedEntries) {
         selectedEntries.setAll(newSelectedEntries);
     }
 
-    public void setSelectedGroup(BibDatabaseContext database, GroupTreeNode newSelectedGroup) {
-        Objects.requireNonNull(newSelectedGroup);
-        selectedGroups.put(database, newSelectedGroup);
+    public void setSelectedGroups(BibDatabaseContext database, List<GroupTreeNode> newSelectedGroups) {
+        Objects.requireNonNull(newSelectedGroups);
+        selectedGroups.put(database, FXCollections.observableArrayList(newSelectedGroups));
     }
 
-    public Optional<GroupTreeNode> getSelectedGroup(BibDatabaseContext database) {
-        return Optional.ofNullable(selectedGroups.get(database));
+    public ObservableList<GroupTreeNode> getSelectedGroup(BibDatabaseContext database) {
+        ObservableList<GroupTreeNode> selectedGroupsForDatabase = selectedGroups.get(database);
+        return selectedGroupsForDatabase != null ? selectedGroupsForDatabase : FXCollections.observableArrayList();
     }
 
-    public void clearSelectedGroup(BibDatabaseContext database) {
+    public void clearSelectedGroups(BibDatabaseContext database) {
         selectedGroups.remove(database);
     }
 

src/main/java/org/jabref/gui/groups/GroupSelector.java

@@ -9,7 +9,6 @@
 import java.awt.event.MouseEvent;
 import java.util.Enumeration;
 import java.util.List;
-import java.util.Optional;
 
 import javax.swing.AbstractAction;
 import javax.swing.BorderFactory;
@@ -55,6 +54,8 @@
 import org.jabref.model.groups.event.GroupUpdatedEvent;
 import org.jabref.model.metadata.MetaData;
 import org.jabref.model.search.SearchMatcher;
+import org.jabref.model.search.matchers.MatcherSet;
+import org.jabref.model.search.matchers.MatcherSets;
 import org.jabref.preferences.JabRefPreferences;
 
 import com.google.common.eventbus.Subscribe;
@@ -329,27 +330,22 @@ public void valueChanged(TreeSelectionEvent e) {
 
     private void updateShownEntriesAccordingToSelectedGroups() {
         updateShownEntriesAccordingToSelectedGroups(Globals.stateManager.activeGroupProperty().get());
-        /*final MatcherSet searchRules = MatcherSets
-                .build(andCb.isSelected() ? MatcherSets.MatcherType.AND : MatcherSets.MatcherType.OR);
-
-        for (GroupTreeNodeViewModel node : getLeafsOfSelection()) {
-            SearchMatcher searchRule = node.getNode().getSearchMatcher();
-            searchRules.addRule(searchRule);
-        }
-        SearchMatcher searchRule = invCb.isSelected() ? new NotMatcher(searchRules) : searchRules;
-        GroupingWorker worker = new GroupingWorker(searchRule);
-        worker.getWorker().run();
-        worker.getCallBack().update();
-        */
     }
 
-    private void updateShownEntriesAccordingToSelectedGroups(Optional<GroupTreeNode> selectedGroup) {
-        if (!selectedGroup.isPresent()) {
+    private void updateShownEntriesAccordingToSelectedGroups(List<GroupTreeNode> selectedGroups) {
+        if (selectedGroups == null || selectedGroups.isEmpty()) {
             // No selected group, nothing to do
             return;
         }
-        SearchMatcher searchRule = selectedGroup.get().getSearchMatcher();
-        GroupingWorker worker = new GroupingWorker(searchRule);
+
+        final MatcherSet searchRules = MatcherSets.build(
+                Globals.prefs.getBoolean(JabRefPreferences.GROUP_INTERSECT_SELECTIONS) ? MatcherSets.MatcherType.AND : MatcherSets.MatcherType.OR);
+
+        for (GroupTreeNode node : selectedGroups) {
+            searchRules.addRule(node.getSearchMatcher());
+        }
+
+        GroupingWorker worker = new GroupingWorker(searchRules);
         worker.run();
         worker.update();
     }

src/main/java/org/jabref/gui/groups/GroupTreeController.java

@@ -2,17 +2,22 @@
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.util.List;
 import java.util.Optional;
+import java.util.function.Consumer;
+import java.util.stream.Collectors;
 
 import javax.inject.Inject;
 
 import javafx.beans.property.ObjectProperty;
+import javafx.collections.ObservableList;
 import javafx.css.PseudoClass;
 import javafx.event.ActionEvent;
 import javafx.fxml.FXML;
 import javafx.scene.control.ContextMenu;
 import javafx.scene.control.Control;
 import javafx.scene.control.MenuItem;
+import javafx.scene.control.SelectionMode;
 import javafx.scene.control.SeparatorMenuItem;
 import javafx.scene.control.TextField;
 import javafx.scene.control.TreeItem;
@@ -59,11 +64,27 @@ public class GroupTreeController extends AbstractController<GroupTreeViewModel>
     public void initialize() {
         viewModel = new GroupTreeViewModel(stateManager, dialogService, taskExecutor);
 
+        // Set-up groups tree
+        groupTree.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
+
         // Set-up bindings
-        groupTree.getSelectionModel().selectedItemProperty().addListener((observable, oldValue, newValue) -> viewModel
-                .selectedGroupProperty().setValue(newValue != null ? newValue.getValue() : null));
-        viewModel.selectedGroupProperty().addListener((observable, oldValue, newValue) -> getTreeItemByValue(newValue)
-                .ifPresent(treeItem -> groupTree.getSelectionModel().select(treeItem)));
+        Consumer<ObservableList<GroupNodeViewModel>> updateSelectedGroups =
+                (newSelectedGroups) -> newSelectedGroups.forEach(this::selectNode);
+        Consumer<List<TreeItem<GroupNodeViewModel>>> updateViewModel =
+                (newSelectedGroups) -> {
+                    if (newSelectedGroups == null) {
+                        viewModel.selectedGroupsProperty().clear();
+                    } else {
+                        viewModel.selectedGroupsProperty().setAll(newSelectedGroups.stream().map(TreeItem::getValue).collect(Collectors.toList()));
+                    }
+                };
+        BindingsHelper.bindContentBidirectional(
+                groupTree.getSelectionModel().getSelectedItems(),
+                viewModel.selectedGroupsProperty(),
+                updateSelectedGroups,
+                updateViewModel
+        );
+
         viewModel.filterTextProperty().bind(searchField.textProperty());
 
         groupTree.rootProperty().bind(
@@ -196,6 +217,11 @@ public void initialize() {
         setupClearButtonField(searchField);
     }
 
+    private void selectNode(GroupNodeViewModel value) {
+        getTreeItemByValue(value)
+                .ifPresent(treeItem -> groupTree.getSelectionModel().select(treeItem));
+    }
+
     private Optional<TreeItem<GroupNodeViewModel>> getTreeItemByValue(GroupNodeViewModel value) {
         return getTreeItemByValue(groupTree.getRoot(), value);
     }

src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java

@@ -5,15 +5,20 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.function.Predicate;
+import java.util.stream.Collectors;
 
 import javax.swing.SwingUtilities;
 
 import javafx.application.Platform;
 import javafx.beans.binding.Bindings;
+import javafx.beans.property.ListProperty;
 import javafx.beans.property.ObjectProperty;
+import javafx.beans.property.SimpleListProperty;
 import javafx.beans.property.SimpleObjectProperty;
 import javafx.beans.property.SimpleStringProperty;
 import javafx.beans.property.StringProperty;
+import javafx.collections.FXCollections;
+import javafx.collections.ObservableList;
 
 import org.jabref.gui.AbstractViewModel;
 import org.jabref.gui.DialogService;
@@ -30,16 +35,16 @@
 public class GroupTreeViewModel extends AbstractViewModel {
 
     private final ObjectProperty<GroupNodeViewModel> rootGroup = new SimpleObjectProperty<>();
-    private final ObjectProperty<GroupNodeViewModel> selectedGroup = new SimpleObjectProperty<>();
+    private final ListProperty<GroupNodeViewModel> selectedGroups = new SimpleListProperty<>(FXCollections.observableArrayList());
     private final StateManager stateManager;
     private final DialogService dialogService;
     private final TaskExecutor taskExecutor;
     private final ObjectProperty<Predicate<GroupNodeViewModel>> filterPredicate = new SimpleObjectProperty<>();
     private final StringProperty filterText = new SimpleStringProperty();
-    private Optional<BibDatabaseContext> currentDatabase;
     private final Comparator<GroupTreeNode> compAlphabetIgnoreCase = (GroupTreeNode v1, GroupTreeNode v2) -> v1
             .getName()
             .compareToIgnoreCase(v2.getName());
+    private Optional<BibDatabaseContext> currentDatabase;
 
     public GroupTreeViewModel(StateManager stateManager, DialogService dialogService, TaskExecutor taskExecutor) {
         this.stateManager = Objects.requireNonNull(stateManager);
@@ -49,7 +54,7 @@ public GroupTreeViewModel(StateManager stateManager, DialogService dialogService
         // Register listener
         stateManager.activeDatabaseProperty()
                 .addListener((observable, oldValue, newValue) -> onActiveDatabaseChanged(newValue));
-        selectedGroup.addListener((observable, oldValue, newValue) -> onSelectedGroupChanged(newValue));
+        selectedGroups.addListener((observable, oldValue, newValue) -> onSelectedGroupChanged(newValue));
 
         // Set-up bindings
         filterPredicate
@@ -63,8 +68,8 @@ public ObjectProperty<GroupNodeViewModel> rootGroupProperty() {
         return rootGroup;
     }
 
-    public ObjectProperty<GroupNodeViewModel> selectedGroupProperty() {
-        return selectedGroup;
+    public ListProperty<GroupNodeViewModel> selectedGroupsProperty() {
+        return selectedGroups;
     }
 
     public ObjectProperty<Predicate<GroupNodeViewModel>> filterPredicateProperty() {
@@ -79,17 +84,17 @@ public StringProperty filterTextProperty() {
      * Gets invoked if the user selects a different group.
      * We need to notify the {@link StateManager} about this change so that the main table gets updated.
      */
-    private void onSelectedGroupChanged(GroupNodeViewModel newValue) {
+    private void onSelectedGroupChanged(ObservableList<GroupNodeViewModel> newValue) {
         if (!currentDatabase.equals(stateManager.activeDatabaseProperty().getValue())) {
             // Switch of database occurred -> do nothing
             return;
         }
 
         currentDatabase.ifPresent(database -> {
             if (newValue == null) {
-                stateManager.clearSelectedGroup(database);
+                stateManager.clearSelectedGroups(database);
             } else {
-                stateManager.setSelectedGroup(database, newValue.getGroupNode());
+                stateManager.setSelectedGroups(database, newValue.stream().map(GroupNodeViewModel::getGroupNode).collect(Collectors.toList()));
             }
         });
     }
@@ -114,9 +119,10 @@ private void onActiveDatabaseChanged(Optional<BibDatabaseContext> newDatabase) {
                     .orElse(GroupNodeViewModel.getAllEntriesGroup(newDatabase.get(), stateManager, taskExecutor));
 
             rootGroup.setValue(newRoot);
-            stateManager.getSelectedGroup(newDatabase.get()).ifPresent(
-                    selectedGroup -> this.selectedGroup.setValue(
-                            new GroupNodeViewModel(newDatabase.get(), stateManager, taskExecutor, selectedGroup)));
+            this.selectedGroups.setAll(
+                    stateManager.getSelectedGroup(newDatabase.get()).stream()
+                            .map(selectedGroup -> new GroupNodeViewModel(newDatabase.get(), stateManager, taskExecutor, selectedGroup))
+                            .collect(Collectors.toList()));
         }
 
         currentDatabase = newDatabase;

src/main/java/org/jabref/gui/util/BindingsHelper.java

@@ -18,6 +18,7 @@
 import javafx.css.PseudoClass;
 import javafx.scene.Node;
 
+
 /**
  * Helper methods for javafx binding.
  * Some methods are taken from https://bugs.openjdk.java.net/browse/JDK-8134679
@@ -86,14 +87,33 @@ public static <A, B> void bindBidirectional(ObservableValue<A> propertyA, Observ
         propertyB.addListener(binding.getChangeListenerB());
     }
 
-    public static <A, B> void bindContentBidirectional(ListProperty<A> listProperty, Property<B> property, Function<List<A>, B> mapToB, Function<B, List<A>> mapToList) {
-        final BidirectionalListBinding<A, B> binding = new BidirectionalListBinding<>(listProperty, property, mapToB, mapToList);
+    public static <A, B> void bindContentBidirectional(ObservableList<A> propertyA, ListProperty<B> propertyB, Consumer<ObservableList<B>> updateA, Consumer<List<A>> updateB) {
+        bindContentBidirectional(
+                propertyA,
+                (ObservableValue<ObservableList<B>>) propertyB,
+                updateA,
+                updateB);
+    }
+
+    public static <A, B> void bindContentBidirectional(ObservableList<A> propertyA, ObservableValue<B> propertyB, Consumer<B> updateA, Consumer<List<A>> updateB) {
+        final BidirectionalListBinding<A, B> binding = new BidirectionalListBinding<>(propertyA, propertyB, updateA, updateB);
 
         // use property as initial source
-        listProperty.setAll(mapToList.apply(property.getValue()));
+        updateA.accept(propertyB.getValue());
 
-        listProperty.addListener(binding);
-        property.addListener(binding);
+        propertyA.addListener(binding);
+        propertyB.addListener(binding);
+    }
+
+    public static <A, B> void bindContentBidirectional(ListProperty<A> listProperty, Property<B> property, Function<List<A>, B> mapToB, Function<B, List<A>> mapToList) {
+        Consumer<B> updateList = newValueB -> listProperty.setAll(mapToList.apply(newValueB));
+        Consumer<List<A>> updateB = newValueList -> property.setValue(mapToB.apply(newValueList));
+
+        bindContentBidirectional(
+                listProperty,
+                property,
+                updateList,
+                updateB);
     }
 
     private static class BidirectionalBinding<A, B> {
@@ -139,25 +159,25 @@ private <T> void updateLocked(Consumer<T> update, T oldValue, T newValue) {
 
     private static class BidirectionalListBinding<A, B> implements ListChangeListener<A>, ChangeListener<B> {
 
-        private final ListProperty<A> listProperty;
-        private final Property<B> property;
-        private final Function<List<A>, B> mapToB;
-        private final Function<B, List<A>> mapToList;
+        private final ObservableList<A> listProperty;
+        private final ObservableValue<B> property;
+        private final Consumer<B> updateA;
+        private final Consumer<List<A>> updateB;
         private boolean updating = false;
 
-        public BidirectionalListBinding(ListProperty<A> listProperty, Property<B> property, Function<List<A>, B> mapToB, Function<B, List<A>> mapToList) {
+        public BidirectionalListBinding(ObservableList<A> listProperty, ObservableValue<B> property, Consumer<B> updateA, Consumer<List<A>> updateB) {
             this.listProperty = listProperty;
             this.property = property;
-            this.mapToB = mapToB;
-            this.mapToList = mapToList;
+            this.updateA = updateA;
+            this.updateB = updateB;
         }
 
         @Override
         public void changed(ObservableValue<? extends B> observable, B oldValue, B newValue) {
             if (!updating) {
                 try {
                     updating = true;
-                    listProperty.setAll(mapToList.apply(newValue));
+                    updateA.accept(newValue);
                 } finally {
                     updating = false;
                 }
@@ -169,7 +189,7 @@ public void onChanged(Change<? extends A> c) {
             if (!updating) {
                 try {
                     updating = true;
-                    property.setValue(mapToB.apply(listProperty.getValue()));
+                    updateB.accept(listProperty);
                 } finally {
                     updating = false;
                 }