diff --git a/CHANGELOG.md b/CHANGELOG.md index bdcbfa43487..1fbc4c0ff7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `# - 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). - We further improved performance of group operations, especially of the new filter feature [#2852](https://github.com/JabRef/jabref/issues/2852). + - It is now possible to resort groups using drag & drop [#2785](https://github.com/JabRef/jabref/issues/2785). - The entry editor got a fresh coat of paint: - Homogenize the size of text fields. - The buttons were changed to icons. diff --git a/src/main/java/org/jabref/gui/groups/DroppingMouseLocation.java b/src/main/java/org/jabref/gui/groups/DroppingMouseLocation.java new file mode 100644 index 00000000000..e946facdcf7 --- /dev/null +++ b/src/main/java/org/jabref/gui/groups/DroppingMouseLocation.java @@ -0,0 +1,10 @@ +package org.jabref.gui.groups; + +/** + * The mouse location within the cell when the dropping gesture occurs. + */ +enum DroppingMouseLocation { + BOTTOM, + CENTER, + TOP +} diff --git a/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java index 65c083e66c1..4e1b85993a2 100644 --- a/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java @@ -254,6 +254,51 @@ public void moveTo(GroupNodeViewModel target) { //panel.getUndoManager().addEdit(new UndoableMoveGroup(this.groupsRoot, moveChange)); //panel.markBaseChanged(); //frame.output(Localization.lang("Moved group \"%0\".", node.getNode().getGroup().getName())); + } + + public void moveTo(GroupTreeNode target, int targetIndex) { + getGroupNode().moveTo(target, targetIndex); + } + + public Optional getParent() { + return groupNode.getParent(); + } + + public void draggedOn(GroupNodeViewModel target, DroppingMouseLocation mouseLocation) { + Optional targetParent = target.getParent(); + if (targetParent.isPresent()) { + int targetIndex = target.getPositionInParent(); + + // In case we want to move an item in the same parent + // and the item is moved down, we need to adjust the target index + if (targetParent.equals(getParent())) { + int sourceIndex = this.getPositionInParent(); + if (sourceIndex < targetIndex) { + targetIndex--; + } + } + + // Different actions depending on where the user releases the drop in the target row + // Bottom + top -> insert source row before / after this row + // Center -> add as child + switch (mouseLocation) { + case BOTTOM: + this.moveTo(targetParent.get(), targetIndex + 1); + break; + case CENTER: + this.moveTo(target); + break; + case TOP: + this.moveTo(targetParent.get(), targetIndex); + break; + } + } else { + // No parent = root -> just add + this.moveTo(target); + } + } + private int getPositionInParent() { + return groupNode.getPositionInParent(); } } diff --git a/src/main/java/org/jabref/gui/groups/GroupTree.css b/src/main/java/org/jabref/gui/groups/GroupTree.css index d0d7859dbfc..7a46b16a4bb 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTree.css +++ b/src/main/java/org/jabref/gui/groups/GroupTree.css @@ -34,6 +34,24 @@ -fx-font-size: 12px; } +.tree-table-row-cell:dragOver-bottom { + -fx-border-color: #eea82f; + -fx-border-width: 0 0 2 0; + -fx-padding: 0 0 -2 0; +} + +.tree-table-row-cell:dragOver-center { + -fx-border-color: #eea82f; + -fx-border-width: 2 2 2 2; + -fx-padding: -2 -2 -2 -2; +} + +.tree-table-row-cell:dragOver-top { + -fx-border-color: #eea82f; + -fx-border-width: 2 0 0 0; + -fx-padding: -2 0 0 0; +} + .tree-table-row-cell:sub > .tree-table-cell { -fx-padding: 0.20em 0em 0.20em 0em; } diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeController.java b/src/main/java/org/jabref/gui/groups/GroupTreeController.java index d5979ed1370..1d1d58c2534 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeController.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeController.java @@ -25,6 +25,7 @@ import javafx.scene.control.TreeTableRow; import javafx.scene.control.TreeTableView; import javafx.scene.input.ClipboardContent; +import javafx.scene.input.DragEvent; import javafx.scene.input.Dragboard; import javafx.scene.input.TransferMode; import javafx.scene.layout.StackPane; @@ -60,6 +61,12 @@ public class GroupTreeController extends AbstractController @Inject private DialogService dialogService; @Inject private TaskExecutor taskExecutor; + private static void removePseudoClasses(TreeTableRow row, PseudoClass... pseudoClasses) { + for (PseudoClass pseudoClass : pseudoClasses) { + row.pseudoClassStateChanged(pseudoClass, false); + } + } + @FXML public void initialize() { viewModel = new GroupTreeViewModel(stateManager, dialogService, taskExecutor); @@ -141,6 +148,11 @@ public void initialize() { // Set pseudo-classes to indicate if row is root or sub-item ( > 1 deep) PseudoClass rootPseudoClass = PseudoClass.getPseudoClass("root"); PseudoClass subElementPseudoClass = PseudoClass.getPseudoClass("sub"); + + // Pseudo-classes for drag and drop + PseudoClass dragOverBottom = PseudoClass.getPseudoClass("dragOver-bottom"); + PseudoClass dragOverCenter = PseudoClass.getPseudoClass("dragOver-center"); + PseudoClass dragOverTop = PseudoClass.getPseudoClass("dragOver-top"); groupTree.setRowFactory(treeTable -> { TreeTableRow row = new TreeTableRow<>(); row.treeItemProperty().addListener((ov, oldTreeItem, newTreeItem) -> { @@ -183,9 +195,25 @@ public void initialize() { Dragboard dragboard = event.getDragboard(); if ((event.getGestureSource() != row) && row.getItem().acceptableDrop(dragboard)) { event.acceptTransferModes(TransferMode.MOVE, TransferMode.LINK); + + removePseudoClasses(row, dragOverBottom, dragOverCenter, dragOverTop); + switch (getDroppingMouseLocation(row, event)) { + case BOTTOM: + row.pseudoClassStateChanged(dragOverBottom, true); + break; + case CENTER: + row.pseudoClassStateChanged(dragOverCenter, true); + break; + case TOP: + row.pseudoClassStateChanged(dragOverTop, true); + break; + } } event.consume(); }); + row.setOnDragExited(event -> { + removePseudoClasses(row, dragOverBottom, dragOverCenter, dragOverTop); + }); row.setOnDragDropped(event -> { Dragboard dragboard = event.getDragboard(); @@ -195,7 +223,7 @@ public void initialize() { Optional source = viewModel.rootGroupProperty().get() .getChildByPath(pathToSource); if (source.isPresent()) { - source.get().moveTo(row.getItem()); + source.get().draggedOn(row.getItem(), getDroppingMouseLocation(row, event)); success = true; } } @@ -315,4 +343,17 @@ private void setupClearButtonField(CustomTextField customTextField) { LOGGER.error("Failed to decorate text field with clear button", ex); } } + + /** + * Determines where the mouse is in the given row. + */ + private DroppingMouseLocation getDroppingMouseLocation(TreeTableRow row, DragEvent event) { + if ((row.getHeight() * 0.25) > event.getY()) { + return DroppingMouseLocation.TOP; + } else if ((row.getHeight() * 0.75) < event.getY()) { + return DroppingMouseLocation.BOTTOM; + } else { + return DroppingMouseLocation.CENTER; + } + } } diff --git a/src/main/java/org/jabref/gui/util/RecursiveTreeItem.java b/src/main/java/org/jabref/gui/util/RecursiveTreeItem.java index e6f15fb2caf..1117f9dad36 100644 --- a/src/main/java/org/jabref/gui/util/RecursiveTreeItem.java +++ b/src/main/java/org/jabref/gui/util/RecursiveTreeItem.java @@ -70,28 +70,28 @@ private void addChildrenListener(T value) { children = new FilteredList<>(childrenFactory.call(value)); children.predicateProperty().bind(Bindings.createObjectBinding(() -> this::showNode, filter)); - children.forEach(this::addAsChild); + addAsChildren(children, 0); children.addListener((ListChangeListener) change -> { while (change.next()) { if (change.wasRemoved()) { change.getRemoved().forEach(t-> { - final List> itemsToRemove = RecursiveTreeItem.this.getChildren().stream().filter(treeItem -> treeItem.getValue().equals(t)).collect(Collectors.toList()); - - RecursiveTreeItem.this.getChildren().removeAll(itemsToRemove); + final List> itemsToRemove = getChildren().stream().filter(treeItem -> treeItem.getValue().equals(t)).collect(Collectors.toList()); + getChildren().removeAll(itemsToRemove); }); } if (change.wasAdded()) { - change.getAddedSubList().forEach(this::addAsChild); + addAsChildren(change.getAddedSubList(), change.getFrom()); } } }); } - private boolean addAsChild(T child) { - return RecursiveTreeItem.this.getChildren().add(new RecursiveTreeItem<>(child, getGraphic(), childrenFactory, expandedProperty, filter)); + private void addAsChildren(List children, int startIndex) { + List> treeItems = children.stream().map(child -> new RecursiveTreeItem<>(child, getGraphic(), childrenFactory, expandedProperty, filter)).collect(Collectors.toList()); + getChildren().addAll(startIndex, treeItems); } private boolean showNode(T t) { diff --git a/src/main/java/org/jabref/model/util/TreeCollector.java b/src/main/java/org/jabref/model/util/TreeCollector.java index 2955774bc1f..28614f4d448 100644 --- a/src/main/java/org/jabref/model/util/TreeCollector.java +++ b/src/main/java/org/jabref/model/util/TreeCollector.java @@ -40,7 +40,7 @@ private TreeCollector(Function> getChildren, BiConsumer addChil } public static > TreeCollector mergeIntoTree(BiPredicate equivalence) { - return new TreeCollector( + return new TreeCollector<>( TreeNode::getChildren, (parent, child) -> child.moveTo(parent), equivalence); diff --git a/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java b/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java index cb8801ed880..c1956a7e5e7 100644 --- a/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java +++ b/src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java @@ -1,5 +1,7 @@ package org.jabref.gui.groups; +import java.util.Arrays; + import javafx.collections.FXCollections; import javafx.collections.ObservableList; @@ -87,7 +89,71 @@ public void treeOfAutomaticKeywordGroupIsCombined() throws Exception { assertEquals(expected, groupViewModel.getChildren()); } + @Test + public void draggedOnTopOfGroupAddsBeforeIt() throws Exception { + GroupNodeViewModel rootViewModel = getViewModelForGroup(new WordKeywordGroup("root", GroupHierarchyType.INCLUDING, "keywords", "A", true, ',', true)); + WordKeywordGroup groupA = new WordKeywordGroup("A", GroupHierarchyType.INCLUDING, "keywords", "A", true, ',', true); + WordKeywordGroup groupB = new WordKeywordGroup("B", GroupHierarchyType.INCLUDING, "keywords", "A > B", true, ',', true); + WordKeywordGroup groupC = new WordKeywordGroup("C", GroupHierarchyType.INCLUDING, "keywords", "A > B > B1", true, ',', true); + GroupNodeViewModel groupAViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupA)); + GroupNodeViewModel groupBViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupB)); + GroupNodeViewModel groupCViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupC)); + + groupCViewModel.draggedOn(groupBViewModel, DroppingMouseLocation.TOP); + + assertEquals(Arrays.asList(groupAViewModel, groupCViewModel, groupBViewModel), rootViewModel.getChildren()); + } + + @Test + public void draggedOnBottomOfGroupAddsAfterIt() throws Exception { + GroupNodeViewModel rootViewModel = getViewModelForGroup(new WordKeywordGroup("root", GroupHierarchyType.INCLUDING, "keywords", "A", true, ',', true)); + WordKeywordGroup groupA = new WordKeywordGroup("A", GroupHierarchyType.INCLUDING, "keywords", "A", true, ',', true); + WordKeywordGroup groupB = new WordKeywordGroup("B", GroupHierarchyType.INCLUDING, "keywords", "A > B", true, ',', true); + WordKeywordGroup groupC = new WordKeywordGroup("C", GroupHierarchyType.INCLUDING, "keywords", "A > B > B1", true, ',', true); + GroupNodeViewModel groupAViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupA)); + GroupNodeViewModel groupBViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupB)); + GroupNodeViewModel groupCViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupC)); + + groupCViewModel.draggedOn(groupAViewModel, DroppingMouseLocation.BOTTOM); + + assertEquals(Arrays.asList(groupAViewModel, groupCViewModel, groupBViewModel), rootViewModel.getChildren()); + } + + @Test + public void draggedOnBottomOfGroupAddsAfterItWhenSourceGroupWasBefore() throws Exception { + GroupNodeViewModel rootViewModel = getViewModelForGroup(new WordKeywordGroup("root", GroupHierarchyType.INCLUDING, "keywords", "A", true, ',', true)); + WordKeywordGroup groupA = new WordKeywordGroup("A", GroupHierarchyType.INCLUDING, "keywords", "A", true, ',', true); + WordKeywordGroup groupB = new WordKeywordGroup("B", GroupHierarchyType.INCLUDING, "keywords", "A > B", true, ',', true); + WordKeywordGroup groupC = new WordKeywordGroup("C", GroupHierarchyType.INCLUDING, "keywords", "A > B > B1", true, ',', true); + GroupNodeViewModel groupAViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupA)); + GroupNodeViewModel groupBViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupB)); + GroupNodeViewModel groupCViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupC)); + + groupAViewModel.draggedOn(groupBViewModel, DroppingMouseLocation.BOTTOM); + + assertEquals(Arrays.asList(groupBViewModel, groupAViewModel, groupCViewModel), rootViewModel.getChildren()); + } + + @Test + public void draggedOnTopOfGroupAddsBeforeItWhenSourceGroupWasBefore() throws Exception { + GroupNodeViewModel rootViewModel = getViewModelForGroup(new WordKeywordGroup("root", GroupHierarchyType.INCLUDING, "keywords", "A", true, ',', true)); + WordKeywordGroup groupA = new WordKeywordGroup("A", GroupHierarchyType.INCLUDING, "keywords", "A", true, ',', true); + WordKeywordGroup groupB = new WordKeywordGroup("B", GroupHierarchyType.INCLUDING, "keywords", "A > B", true, ',', true); + WordKeywordGroup groupC = new WordKeywordGroup("C", GroupHierarchyType.INCLUDING, "keywords", "A > B > B1", true, ',', true); + GroupNodeViewModel groupAViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupA)); + GroupNodeViewModel groupBViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupB)); + GroupNodeViewModel groupCViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupC)); + + groupAViewModel.draggedOn(groupCViewModel, DroppingMouseLocation.TOP); + + assertEquals(Arrays.asList(groupBViewModel, groupAViewModel, groupCViewModel), rootViewModel.getChildren()); + } + private GroupNodeViewModel getViewModelForGroup(AbstractGroup group) { return new GroupNodeViewModel(databaseContext, stateManager, taskExecutor, group); } + + private GroupNodeViewModel getViewModelForGroup(GroupTreeNode group) { + return new GroupNodeViewModel(databaseContext, stateManager, taskExecutor, group); + } } diff --git a/src/test/java/org/jabref/model/TreeNodeTest.java b/src/test/java/org/jabref/model/TreeNodeTest.java index 841dfaeab29..cff784ef02b 100644 --- a/src/test/java/org/jabref/model/TreeNodeTest.java +++ b/src/test/java/org/jabref/model/TreeNodeTest.java @@ -172,6 +172,36 @@ public void moveToInSameLevelAddsAtEnd() { assertEquals(Arrays.asList(child2, child1), root.getChildren()); } + @Test + public void moveToInSameLevelWhenNodeWasBeforeTargetIndex() { + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock child1 = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock child2 = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock child3 = new TreeNodeTestData.TreeNodeMock(); + root.addChild(child1); + root.addChild(child2); + root.addChild(child3); + + child1.moveTo(root, 1); + + assertEquals(Arrays.asList(child2, child1, child3), root.getChildren()); + } + + @Test + public void moveToInSameLevelWhenNodeWasAfterTargetIndex() { + TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock child1 = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock child2 = new TreeNodeTestData.TreeNodeMock(); + TreeNodeTestData.TreeNodeMock child3 = new TreeNodeTestData.TreeNodeMock(); + root.addChild(child1); + root.addChild(child2); + root.addChild(child3); + + child3.moveTo(root, 1); + + assertEquals(Arrays.asList(child1, child3, child2), root.getChildren()); + } + @Test public void getPathFromRootInSimpleTree() { TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock();