Add "Remove group and subgroups" option (#2587)

* Add "Remove group and subgroups" option Issue #1407 is fixed in the process * Remove old code * Implement #1904: filter groups * Entries are filtered again correctly
JabRef · Mar 3, 2017 · 79ff330 · 79ff330
1 parent aeb47a9
commit 79ff330
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 38 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -49,6 +49,8 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
  - ArXiV fetcher now checks similarity of entry when using DOI retrieval to avoid false positives [#2575](https://github.com/JabRef/jabref/issues/2575)
  - Sciencedirect/Elsevier fetcher is now able to scrape new HTML structure [#2576](https://github.com/JabRef/jabref/issues/2576)
  - Fixed the synchronization logic of keywords and special fields and vice versa [#2580](https://github.com/JabRef/jabref/issues/2580)
+ - Fixed a display issue when removing a group with a long name [#1407](https://github.com/JabRef/jabref/issues/1407)
+
  - The group selection is no longer lost when switching tabs [#1104](https://github.com/JabRef/jabref/issues/1104)
 
  - We fixed an issue where the "find unlinked files" functionality threw an error when only one PDF was imported but not assigned to an entry [#2577](https://github.com/JabRef/jabref/issues/2577)

diff --git a/src/main/java/org/jabref/gui/DialogService.java b/src/main/java/org/jabref/gui/DialogService.java
@@ -66,9 +66,19 @@ default void showErrorDialogAndWait(Exception exception) {
      * a OK and Cancel Button. To create a confirmation dialog with custom
      * buttons see also {@link #showCustomButtonDialogAndWait(Alert.AlertType, String, String, ButtonType...)}
      *
-     * @return Optional with the pressed Button as ButtonType
+     * @return true if the use clicked "OK" otherwise false
+     */
+    boolean showConfirmationDialogAndWait(String title, String content);
+
+    /**
+     * Create and display a new confirmation dialog.
+     * It will include a blue question icon on the left and
+     * a OK (with given label) and Cancel Button. To create a confirmation dialog with custom
+     * buttons see also {@link #showCustomButtonDialogAndWait(Alert.AlertType, String, String, ButtonType...)}
+     *
+     * @return true if the use clicked "OK" otherwise false
      */
-    Optional<ButtonType> showConfirmationDialogAndWait(String title, String content);
+    boolean showConfirmationDialogAndWait(String title, String content, String okButtonLabel);
 
     /**
      * This will create and display a new dialog of the specified

diff --git a/src/main/java/org/jabref/gui/FXDialogService.java b/src/main/java/org/jabref/gui/FXDialogService.java
@@ -5,6 +5,7 @@
 import java.util.Optional;
 
 import javafx.scene.control.Alert.AlertType;
+import javafx.scene.control.ButtonBar;
 import javafx.scene.control.ButtonType;
 import javafx.scene.control.DialogPane;
 import javafx.stage.FileChooser;
@@ -65,9 +66,17 @@ public void showErrorDialogAndWait(String message) {
     }
 
     @Override
-    public Optional<ButtonType> showConfirmationDialogAndWait(String title, String content) {
+    public boolean showConfirmationDialogAndWait(String title, String content) {
         FXDialog alert = createDialog(AlertType.CONFIRMATION, title, content);
-        return alert.showAndWait();
+        return alert.showAndWait().filter(buttonType -> buttonType == ButtonType.OK).isPresent();
+    }
+
+    @Override
+    public boolean showConfirmationDialogAndWait(String title, String content, String okButtonLabel) {
+        FXDialog alert = createDialog(AlertType.CONFIRMATION, title, content);
+        ButtonType okButtonType = new ButtonType(okButtonLabel, ButtonBar.ButtonData.OK_DONE);
+        alert.getButtonTypes().setAll(ButtonType.CANCEL, okButtonType);
+        return alert.showAndWait().filter(buttonType -> buttonType == okButtonType).isPresent();
     }
 
     @Override

diff --git a/src/main/java/org/jabref/gui/groups/GroupSelector.java b/src/main/java/org/jabref/gui/groups/GroupSelector.java
@@ -102,7 +102,6 @@ public class GroupSelector extends SidePaneComponent implements TreeSelectionLis
     private final AbstractAction editGroupAction = new EditGroupAction();
     private final NodeAction editGroupPopupAction = new EditGroupAction();
     private final NodeAction addGroupPopupAction = new AddGroupAction();
-    private final NodeAction removeGroupAndSubgroupsPopupAction = new RemoveGroupAndSubgroupsAction();
     private final NodeAction removeSubgroupsPopupAction = new RemoveSubgroupsAction();
     private final NodeAction removeGroupKeepSubgroupsPopupAction = new RemoveGroupKeepSubgroupsAction();
     private final NodeAction moveNodeUpPopupAction = new MoveNodeUpAction();
@@ -307,7 +306,6 @@ private void definePopup() {
         groupsContextMenu.add(editGroupPopupAction);
         groupsContextMenu.add(addGroupPopupAction);
         groupsContextMenu.addSeparator();
-        groupsContextMenu.add(removeGroupAndSubgroupsPopupAction);
         groupsContextMenu.add(removeGroupKeepSubgroupsPopupAction);
         groupsContextMenu.add(removeSubgroupsPopupAction);
         groupsContextMenu.addSeparator();
@@ -383,7 +381,6 @@ private void showPopup(MouseEvent e) {
         final TreePath path = groupsTree.getPathForLocation(e.getPoint().x, e.getPoint().y);
         addGroupPopupAction.setEnabled(true);
         editGroupPopupAction.setEnabled(path != null);
-        removeGroupAndSubgroupsPopupAction.setEnabled(path != null);
         removeGroupKeepSubgroupsPopupAction.setEnabled(path != null);
         moveSubmenu.setEnabled(path != null);
         expandSubtreePopupAction.setEnabled(path != null);
@@ -396,7 +393,6 @@ private void showPopup(MouseEvent e) {
         if (path != null) { // some path dependent enabling/disabling
             GroupTreeNodeViewModel node = (GroupTreeNodeViewModel) path.getLastPathComponent();
             editGroupPopupAction.setNode(node);
-            removeGroupAndSubgroupsPopupAction.setNode(node);
             removeSubgroupsPopupAction.setNode(node);
             removeGroupKeepSubgroupsPopupAction.setNode(node);
             expandSubtreePopupAction.setNode(node);
@@ -407,13 +403,13 @@ private void showPopup(MouseEvent e) {
             if (node.canBeEdited()) {
                 editGroupPopupAction.setEnabled(false);
                 addGroupPopupAction.setEnabled(false);
-                removeGroupAndSubgroupsPopupAction.setEnabled(false);
+                //removeGroupAndSubgroupsPopupAction.setEnabled(false);
                 removeGroupKeepSubgroupsPopupAction.setEnabled(false);
             } else {
                 editGroupPopupAction.setEnabled(true);
                 addGroupPopupAction.setEnabled(true);
                 addGroupPopupAction.setNode(node);
-                removeGroupAndSubgroupsPopupAction.setEnabled(true);
+                //removeGroupAndSubgroupsPopupAction.setEnabled(true);
                 removeGroupKeepSubgroupsPopupAction.setEnabled(true);
             }
             expandSubtreePopupAction
@@ -452,7 +448,6 @@ private void showPopup(MouseEvent e) {
         } else {
             editGroupPopupAction.setNode(null);
             addGroupPopupAction.setNode(null);
-            removeGroupAndSubgroupsPopupAction.setNode(null);
             removeSubgroupsPopupAction.setNode(null);
             removeGroupKeepSubgroupsPopupAction.setNode(null);
             moveNodeUpPopupAction.setNode(null);
@@ -983,31 +978,6 @@ public void actionPerformed(ActionEvent e) {
         }
     }
 
-    private class RemoveGroupAndSubgroupsAction extends NodeAction {
-
-        public RemoveGroupAndSubgroupsAction() {
-            super(Localization.lang("Remove group and subgroups"));
-        }
-
-        @Override
-        public void actionPerformed(ActionEvent e) {
-            final GroupTreeNodeViewModel node = getNodeToUse();
-            final AbstractGroup group = node.getNode().getGroup();
-            int conf = JOptionPane.showConfirmDialog(frame,
-                    Localization.lang("Remove group \"%0\" and its subgroups?", group.getName()),
-                    Localization.lang("Remove group and subgroups"), JOptionPane.YES_NO_OPTION);
-            if (conf == JOptionPane.YES_OPTION) {
-                final UndoableAddOrRemoveGroup undo = new UndoableAddOrRemoveGroup(groupsRoot, node,
-                        UndoableAddOrRemoveGroup.REMOVE_NODE_AND_CHILDREN);
-                node.getNode().removeFromParent();
-                // Store undo information.
-                panel.getUndoManager().addEdit(undo);
-                panel.markBaseChanged();
-                frame.output(Localization.lang("Removed group \"%0\" and its subgroups.", group.getName()));
-            }
-        }
-    }
-
     private class RemoveSubgroupsAction extends NodeAction {
 
         public RemoveSubgroupsAction() {

diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeController.java b/src/main/java/org/jabref/gui/groups/GroupTreeController.java
@@ -34,7 +34,6 @@
 import org.controlsfx.control.textfield.CustomTextField;
 import org.controlsfx.control.textfield.TextFields;
 import org.fxmisc.easybind.EasyBind;
-import org.fxmisc.easybind.monadic.PropertyBinding;
 
 public class GroupTreeController extends AbstractController<GroupTreeViewModel> {
 
@@ -54,7 +53,7 @@ public void initialize() {
         viewModel = new GroupTreeViewModel(stateManager, dialogService);
 
         // Set-up bindings
-        viewModel.selectedGroupProperty().bindBidirectional(
+        viewModel.selectedGroupProperty().bind(
                 EasyBind.monadic(groupTree.selectionModelProperty())
                         .flatMap(SelectionModel::selectedItemProperty)
                         .selectProperty(TreeItem::valueProperty)
@@ -154,7 +153,11 @@ private ContextMenu createContextMenuForGroup(GroupNodeViewModel group) {
         MenuItem addSubgroup = new MenuItem(Localization.lang("Add subgroup"));
         addSubgroup.setOnAction(event -> viewModel.addNewSubgroup(group));
 
+        MenuItem removeGroupAndSubgroups = new MenuItem(Localization.lang("Remove group and subgroups"));
+        removeGroupAndSubgroups.setOnAction(event -> viewModel.removeGroupAndSubgroups(group));
+
         menu.getItems().add(addSubgroup);
+        menu.getItems().add(removeGroupAndSubgroups);
         return menu;
     }
 

diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java
@@ -118,4 +118,23 @@ public void addNewSubgroup(GroupNodeViewModel parent) {
             dialogService.notify(Localization.lang("Added group \"%0\".", group.getName()));
         });
     }
+
+    /**
+     * Removes the specified group and its subgroups (after asking for confirmation).
+     */
+    public void removeGroupAndSubgroups(GroupNodeViewModel group) {
+        boolean confirmed = dialogService.showConfirmationDialogAndWait(
+                Localization.lang("Remove group and subgroups"),
+                Localization.lang("Remove group \"%0\" and its subgroups?", group.getDisplayName()),
+                Localization.lang("Remove"));
+        if (confirmed) {
+            // TODO: Add undo
+            //final UndoableAddOrRemoveGroup undo = new UndoableAddOrRemoveGroup(groupsRoot, node, UndoableAddOrRemoveGroup.REMOVE_NODE_AND_CHILDREN);
+            //panel.getUndoManager().addEdit(undo);
+
+            group.getGroupNode().removeFromParent();
+
+            dialogService.notify(Localization.lang("Removed group \"%0\" and its subgroups.", group.getDisplayName()));
+        }
+    }
 }