Merge pull request #5328 from JabRef/fix5327

Improve group coloring and item count

CHANGELOG.md

@@ -20,11 +20,11 @@ We refer to [GitHub issues](/~https://github.com/JabRef/jabref/issues) by using `#
 - Inherit fields from cross-referenced entries as specified by biblatex. [#5045](/~https://github.com/JabRef/jabref/issues/5045)
 - We fixed an issue where it was no longer possible to connect to LibreOffice. [#5261](/~https://github.com/JabRef/jabref/issues/5261)
 - The "All entries group" is no longer shown when no library is open.
+- After assigning an entry to a group, the item count is now properly colored to reflect the new membership of the entry. [#3112](/~https://github.com/JabRef/jabref/issues/3112)
 - The group panel is now properly updated when switching between libraries (or when closing/opening one). [#3142](/~https://github.com/JabRef/jabref/issues/3142)
 - We fixed an error where the number of matched entries shown in the group pane was not updated correctly. [#4441](/~https://github.com/JabRef/jabref/issues/4441)
 - We fixed an error mentioning "javafx.controls/com.sun.javafx.scene.control" that was thrown when interacting with the toolbar.
 
-
 ### Removed
 
 

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

@@ -113,8 +113,13 @@ public List<FieldChange> addEntriesToGroup(List<BibEntry> entries) {
         //    return; // user aborted operation
         //}
 
-        return groupNode.addEntriesToGroup(entries);
+        var changes = groupNode.addEntriesToGroup(entries);
 
+        // Update appearance of group
+        anySelectedEntriesMatched.invalidate();
+        allSelectedEntriesMatched.invalidate();
+
+        return changes;
         // TODO: Store undo
         // if (!undo.isEmpty()) {
         // groupSelector.concludeAssignment(UndoableChangeEntriesOfGroup.getUndoableEdit(target, undo), target.getNode(), assignedEntries);

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

@@ -111,8 +111,7 @@ public void initialize() {
                         }));
 
         // Icon and group name
-        mainColumn.setCellValueFactory(cellData -> cellData.getValue().valueProperty());
-        new ViewModelTreeTableCellFactory<GroupNodeViewModel, GroupNodeViewModel>()
+        new ViewModelTreeTableCellFactory<GroupNodeViewModel>()
                 .withText(GroupNodeViewModel::getDisplayName)
                 .withIcon(GroupNodeViewModel::getIcon)
                 .withTooltip(GroupNodeViewModel::getDescription)
@@ -121,7 +120,7 @@ public void initialize() {
         // Number of hits
         PseudoClass anySelected = PseudoClass.getPseudoClass("any-selected");
         PseudoClass allSelected = PseudoClass.getPseudoClass("all-selected");
-        new ViewModelTreeTableCellFactory<GroupNodeViewModel, GroupNodeViewModel>()
+        new ViewModelTreeTableCellFactory<GroupNodeViewModel>()
                 .withGraphic(group -> {
                     final StackPane node = new StackPane();
                     node.getStyleClass().setAll("hits");
@@ -141,8 +140,7 @@ public void initialize() {
                 .install(numberColumn);
 
         // Arrow indicating expanded status
-        disclosureNodeColumn.setCellValueFactory(cellData -> cellData.getValue().valueProperty());
-        new ViewModelTreeTableCellFactory<GroupNodeViewModel, GroupNodeViewModel>()
+        new ViewModelTreeTableCellFactory<GroupNodeViewModel>()
                 .withGraphic(viewModel -> {
                     final StackPane disclosureNode = new StackPane();
                     disclosureNode.visibleProperty().bind(viewModel.hasChildrenProperty());

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

@@ -52,6 +52,7 @@ public GroupTreeViewModel(StateManager stateManager, DialogService dialogService
         this.dialogService = Objects.requireNonNull(dialogService);
         this.taskExecutor = Objects.requireNonNull(taskExecutor);
         this.localDragboard = Objects.requireNonNull(localDragboard);
+
         // Register listener
         EasyBind.subscribe(stateManager.activeDatabaseProperty(), this::onActiveDatabaseChanged);
         EasyBind.subscribe(selectedGroups, this::onSelectedGroupChanged);

src/main/java/org/jabref/gui/keyboard/KeyBindingsDialogView.java

@@ -29,7 +29,7 @@ public class KeyBindingsDialogView extends BaseDialog<Void> {
     @FXML private TreeTableView<KeyBindingViewModel> keyBindingsTable;
     @FXML private TreeTableColumn<KeyBindingViewModel, String> actionColumn;
     @FXML private TreeTableColumn<KeyBindingViewModel, String> shortcutColumn;
-    @FXML private TreeTableColumn<KeyBindingViewModel, String> resetColumn;
+    @FXML private TreeTableColumn<KeyBindingViewModel, KeyBindingViewModel> resetColumn;
 
     @Inject private KeyBindingRepository keyBindingRepository;
     @Inject private DialogService dialogService;
@@ -65,10 +65,10 @@ private void initialize() {
         );
         actionColumn.setCellValueFactory(cellData -> cellData.getValue().getValue().nameProperty());
         shortcutColumn.setCellValueFactory(cellData -> cellData.getValue().getValue().shownBindingProperty());
-        resetColumn.setCellFactory(new ViewModelTreeTableCellFactory<KeyBindingViewModel, String>()
+        new ViewModelTreeTableCellFactory<KeyBindingViewModel>()
                 .withGraphic(keyBinding -> keyBinding.getIcon().map(JabRefIcon::getGraphicNode).orElse(null))
                 .withOnMouseClickedEvent(keyBinding -> evt -> keyBinding.resetToDefault())
-        );
+                .install(resetColumn);
     }
 
     @FXML

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

@@ -15,56 +15,54 @@
  * Constructs a {@link TreeTableCell} based on the view model of the row and a bunch of specified converter methods.
  *
  * @param <S> view model
- * @param <T> cell value
  */
-public class ViewModelTreeTableCellFactory<S, T> implements Callback<TreeTableColumn<S, T>, TreeTableCell<S, T>> {
+public class ViewModelTreeTableCellFactory<S> implements Callback<TreeTableColumn<S, S>, TreeTableCell<S, S>> {
 
     private Callback<S, String> toText;
     private Callback<S, Node> toGraphic;
     private Callback<S, EventHandler<? super MouseEvent>> toOnMouseClickedEvent;
     private Callback<S, String> toTooltip;
 
-    public ViewModelTreeTableCellFactory<S, T> withText(Callback<S, String> toText) {
+    public ViewModelTreeTableCellFactory<S> withText(Callback<S, String> toText) {
         this.toText = toText;
         return this;
     }
 
-    public ViewModelTreeTableCellFactory<S, T> withGraphic(Callback<S, Node> toGraphic) {
+    public ViewModelTreeTableCellFactory<S> withGraphic(Callback<S, Node> toGraphic) {
         this.toGraphic = toGraphic;
         return this;
     }
 
-    public ViewModelTreeTableCellFactory<S, T> withIcon(Callback<S, JabRefIcon> toIcon) {
+    public ViewModelTreeTableCellFactory<S> withIcon(Callback<S, JabRefIcon> toIcon) {
         this.toGraphic = viewModel -> toIcon.call(viewModel).getGraphicNode();
         return this;
     }
 
-    public ViewModelTreeTableCellFactory<S, T> withTooltip(Callback<S, String> toTooltip) {
+    public ViewModelTreeTableCellFactory<S> withTooltip(Callback<S, String> toTooltip) {
         this.toTooltip = toTooltip;
         return this;
     }
 
-    public ViewModelTreeTableCellFactory<S, T> withOnMouseClickedEvent(
+    public ViewModelTreeTableCellFactory<S> withOnMouseClickedEvent(
             Callback<S, EventHandler<? super MouseEvent>> toOnMouseClickedEvent) {
         this.toOnMouseClickedEvent = toOnMouseClickedEvent;
         return this;
     }
 
     @Override
-    public TreeTableCell<S, T> call(TreeTableColumn<S, T> param) {
+    public TreeTableCell<S, S> call(TreeTableColumn<S, S> param) {
 
-        return new TreeTableCell<S, T>() {
+        return new TreeTableCell<S, S>() {
 
             @Override
-            protected void updateItem(T item, boolean empty) {
-                super.updateItem(item, empty);
+            protected void updateItem(S viewModel, boolean empty) {
+                super.updateItem(viewModel, empty);
 
-                if (empty || getTreeTableRow() == null || getTreeTableRow().getItem() == null) {
+                if (empty || viewModel == null) {
                     setText(null);
                     setGraphic(null);
                     setOnMouseClicked(null);
                 } else {
-                    S viewModel = getTreeTableRow().getItem();
                     if (toText != null) {
                         setText(toText.call(viewModel));
                     }
@@ -85,7 +83,8 @@ protected void updateItem(T item, boolean empty) {
         };
     }
 
-    public void install(TreeTableColumn<S, T> column) {
+    public void install(TreeTableColumn<S, S> column) {
+        column.setCellValueFactory(cellData -> cellData.getValue().valueProperty());
         column.setCellFactory(this);
     }
 }