Skip to content

Commit 4c87dde

Browse files
tobiasdieztobiasdiez
authoredMay 25, 2017
Implement #2785: resort groups using drag & drop (#2864)
1 parent bdac5e8 commit 4c87dde

File tree

9 files changed

+220
-9
lines changed

9 files changed

+220
-9
lines changed
 

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
1515
- We continued to improve the new groups interface:
1616
- 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).
1717
- We further improved performance of group operations, especially of the new filter feature [#2852](https://github.com/JabRef/jabref/issues/2852).
18+
- It is now possible to resort groups using drag & drop [#2785](https://github.com/JabRef/jabref/issues/2785).
1819
- The entry editor got a fresh coat of paint:
1920
- Homogenize the size of text fields.
2021
- The buttons were changed to icons.

‎src/main/java/org/jabref/gui/groups/DroppingMouseLocation.java

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
package org.jabref.gui.groups;
2+
3+
/**
4+
* The mouse location within the cell when the dropping gesture occurs.
5+
*/
6+
enum DroppingMouseLocation {
7+
BOTTOM,
8+
CENTER,
9+
TOP
10+
}

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

+45
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,51 @@ public void moveTo(GroupNodeViewModel target) {
254254
//panel.getUndoManager().addEdit(new UndoableMoveGroup(this.groupsRoot, moveChange));
255255
//panel.markBaseChanged();
256256
//frame.output(Localization.lang("Moved group \"%0\".", node.getNode().getGroup().getName()));
257+
}
258+
259+
public void moveTo(GroupTreeNode target, int targetIndex) {
260+
getGroupNode().moveTo(target, targetIndex);
261+
}
262+
263+
public Optional<GroupTreeNode> getParent() {
264+
return groupNode.getParent();
265+
}
266+
267+
public void draggedOn(GroupNodeViewModel target, DroppingMouseLocation mouseLocation) {
268+
Optional<GroupTreeNode> targetParent = target.getParent();
269+
if (targetParent.isPresent()) {
270+
int targetIndex = target.getPositionInParent();
271+
272+
// In case we want to move an item in the same parent
273+
// and the item is moved down, we need to adjust the target index
274+
if (targetParent.equals(getParent())) {
275+
int sourceIndex = this.getPositionInParent();
276+
if (sourceIndex < targetIndex) {
277+
targetIndex--;
278+
}
279+
}
280+
281+
// Different actions depending on where the user releases the drop in the target row
282+
// Bottom + top -> insert source row before / after this row
283+
// Center -> add as child
284+
switch (mouseLocation) {
285+
case BOTTOM:
286+
this.moveTo(targetParent.get(), targetIndex + 1);
287+
break;
288+
case CENTER:
289+
this.moveTo(target);
290+
break;
291+
case TOP:
292+
this.moveTo(targetParent.get(), targetIndex);
293+
break;
294+
}
295+
} else {
296+
// No parent = root -> just add
297+
this.moveTo(target);
298+
}
299+
}
257300

301+
private int getPositionInParent() {
302+
return groupNode.getPositionInParent();
258303
}
259304
}

‎src/main/java/org/jabref/gui/groups/GroupTree.css

+18
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,24 @@
3434
-fx-font-size: 12px;
3535
}
3636

37+
.tree-table-row-cell:dragOver-bottom {
38+
-fx-border-color: #eea82f;
39+
-fx-border-width: 0 0 2 0;
40+
-fx-padding: 0 0 -2 0;
41+
}
42+
43+
.tree-table-row-cell:dragOver-center {
44+
-fx-border-color: #eea82f;
45+
-fx-border-width: 2 2 2 2;
46+
-fx-padding: -2 -2 -2 -2;
47+
}
48+
49+
.tree-table-row-cell:dragOver-top {
50+
-fx-border-color: #eea82f;
51+
-fx-border-width: 2 0 0 0;
52+
-fx-padding: -2 0 0 0;
53+
}
54+
3755
.tree-table-row-cell:sub > .tree-table-cell {
3856
-fx-padding: 0.20em 0em 0.20em 0em;
3957
}

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

+42-1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import javafx.scene.control.TreeTableRow;
2626
import javafx.scene.control.TreeTableView;
2727
import javafx.scene.input.ClipboardContent;
28+
import javafx.scene.input.DragEvent;
2829
import javafx.scene.input.Dragboard;
2930
import javafx.scene.input.TransferMode;
3031
import javafx.scene.layout.StackPane;
@@ -60,6 +61,12 @@ public class GroupTreeController extends AbstractController<GroupTreeViewModel>
6061
@Inject private DialogService dialogService;
6162
@Inject private TaskExecutor taskExecutor;
6263

64+
private static void removePseudoClasses(TreeTableRow<GroupNodeViewModel> row, PseudoClass... pseudoClasses) {
65+
for (PseudoClass pseudoClass : pseudoClasses) {
66+
row.pseudoClassStateChanged(pseudoClass, false);
67+
}
68+
}
69+
6370
@FXML
6471
public void initialize() {
6572
viewModel = new GroupTreeViewModel(stateManager, dialogService, taskExecutor);
@@ -141,6 +148,11 @@ public void initialize() {
141148
// Set pseudo-classes to indicate if row is root or sub-item ( > 1 deep)
142149
PseudoClass rootPseudoClass = PseudoClass.getPseudoClass("root");
143150
PseudoClass subElementPseudoClass = PseudoClass.getPseudoClass("sub");
151+
152+
// Pseudo-classes for drag and drop
153+
PseudoClass dragOverBottom = PseudoClass.getPseudoClass("dragOver-bottom");
154+
PseudoClass dragOverCenter = PseudoClass.getPseudoClass("dragOver-center");
155+
PseudoClass dragOverTop = PseudoClass.getPseudoClass("dragOver-top");
144156
groupTree.setRowFactory(treeTable -> {
145157
TreeTableRow<GroupNodeViewModel> row = new TreeTableRow<>();
146158
row.treeItemProperty().addListener((ov, oldTreeItem, newTreeItem) -> {
@@ -183,9 +195,25 @@ public void initialize() {
183195
Dragboard dragboard = event.getDragboard();
184196
if ((event.getGestureSource() != row) && row.getItem().acceptableDrop(dragboard)) {
185197
event.acceptTransferModes(TransferMode.MOVE, TransferMode.LINK);
198+
199+
removePseudoClasses(row, dragOverBottom, dragOverCenter, dragOverTop);
200+
switch (getDroppingMouseLocation(row, event)) {
201+
case BOTTOM:
202+
row.pseudoClassStateChanged(dragOverBottom, true);
203+
break;
204+
case CENTER:
205+
row.pseudoClassStateChanged(dragOverCenter, true);
206+
break;
207+
case TOP:
208+
row.pseudoClassStateChanged(dragOverTop, true);
209+
break;
210+
}
186211
}
187212
event.consume();
188213
});
214+
row.setOnDragExited(event -> {
215+
removePseudoClasses(row, dragOverBottom, dragOverCenter, dragOverTop);
216+
});
189217

190218
row.setOnDragDropped(event -> {
191219
Dragboard dragboard = event.getDragboard();
@@ -195,7 +223,7 @@ public void initialize() {
195223
Optional<GroupNodeViewModel> source = viewModel.rootGroupProperty().get()
196224
.getChildByPath(pathToSource);
197225
if (source.isPresent()) {
198-
source.get().moveTo(row.getItem());
226+
source.get().draggedOn(row.getItem(), getDroppingMouseLocation(row, event));
199227
success = true;
200228
}
201229
}
@@ -315,4 +343,17 @@ private void setupClearButtonField(CustomTextField customTextField) {
315343
LOGGER.error("Failed to decorate text field with clear button", ex);
316344
}
317345
}
346+
347+
/**
348+
* Determines where the mouse is in the given row.
349+
*/
350+
private DroppingMouseLocation getDroppingMouseLocation(TreeTableRow<GroupNodeViewModel> row, DragEvent event) {
351+
if ((row.getHeight() * 0.25) > event.getY()) {
352+
return DroppingMouseLocation.TOP;
353+
} else if ((row.getHeight() * 0.75) < event.getY()) {
354+
return DroppingMouseLocation.BOTTOM;
355+
} else {
356+
return DroppingMouseLocation.CENTER;
357+
}
358+
}
318359
}

‎src/main/java/org/jabref/gui/util/RecursiveTreeItem.java

+7-7
Original file line numberDiff line numberDiff line change
@@ -70,28 +70,28 @@ private void addChildrenListener(T value) {
7070
children = new FilteredList<>(childrenFactory.call(value));
7171
children.predicateProperty().bind(Bindings.createObjectBinding(() -> this::showNode, filter));
7272

73-
children.forEach(this::addAsChild);
73+
addAsChildren(children, 0);
7474

7575
children.addListener((ListChangeListener<T>) change -> {
7676
while (change.next()) {
7777

7878
if (change.wasRemoved()) {
7979
change.getRemoved().forEach(t-> {
80-
final List<TreeItem<T>> itemsToRemove = RecursiveTreeItem.this.getChildren().stream().filter(treeItem -> treeItem.getValue().equals(t)).collect(Collectors.toList());
81-
82-
RecursiveTreeItem.this.getChildren().removeAll(itemsToRemove);
80+
final List<TreeItem<T>> itemsToRemove = getChildren().stream().filter(treeItem -> treeItem.getValue().equals(t)).collect(Collectors.toList());
81+
getChildren().removeAll(itemsToRemove);
8382
});
8483
}
8584

8685
if (change.wasAdded()) {
87-
change.getAddedSubList().forEach(this::addAsChild);
86+
addAsChildren(change.getAddedSubList(), change.getFrom());
8887
}
8988
}
9089
});
9190
}
9291

93-
private boolean addAsChild(T child) {
94-
return RecursiveTreeItem.this.getChildren().add(new RecursiveTreeItem<>(child, getGraphic(), childrenFactory, expandedProperty, filter));
92+
private void addAsChildren(List<? extends T> children, int startIndex) {
93+
List<RecursiveTreeItem<T>> treeItems = children.stream().map(child -> new RecursiveTreeItem<>(child, getGraphic(), childrenFactory, expandedProperty, filter)).collect(Collectors.toList());
94+
getChildren().addAll(startIndex, treeItems);
9595
}
9696

9797
private boolean showNode(T t) {

‎src/main/java/org/jabref/model/util/TreeCollector.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ private TreeCollector(Function<T, List<T>> getChildren, BiConsumer<T, T> addChil
4040
}
4141

4242
public static <T extends TreeNode<T>> TreeCollector<T> mergeIntoTree(BiPredicate<T, T> equivalence) {
43-
return new TreeCollector<T>(
43+
return new TreeCollector<>(
4444
TreeNode::getChildren,
4545
(parent, child) -> child.moveTo(parent),
4646
equivalence);

‎src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java

+66
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
package org.jabref.gui.groups;
22

3+
import java.util.Arrays;
4+
35
import javafx.collections.FXCollections;
46
import javafx.collections.ObservableList;
57

@@ -87,7 +89,71 @@ public void treeOfAutomaticKeywordGroupIsCombined() throws Exception {
8789
assertEquals(expected, groupViewModel.getChildren());
8890
}
8991

92+
@Test
93+
public void draggedOnTopOfGroupAddsBeforeIt() throws Exception {
94+
GroupNodeViewModel rootViewModel = getViewModelForGroup(new WordKeywordGroup("root", GroupHierarchyType.INCLUDING, "keywords", "A", true, ',', true));
95+
WordKeywordGroup groupA = new WordKeywordGroup("A", GroupHierarchyType.INCLUDING, "keywords", "A", true, ',', true);
96+
WordKeywordGroup groupB = new WordKeywordGroup("B", GroupHierarchyType.INCLUDING, "keywords", "A > B", true, ',', true);
97+
WordKeywordGroup groupC = new WordKeywordGroup("C", GroupHierarchyType.INCLUDING, "keywords", "A > B > B1", true, ',', true);
98+
GroupNodeViewModel groupAViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupA));
99+
GroupNodeViewModel groupBViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupB));
100+
GroupNodeViewModel groupCViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupC));
101+
102+
groupCViewModel.draggedOn(groupBViewModel, DroppingMouseLocation.TOP);
103+
104+
assertEquals(Arrays.asList(groupAViewModel, groupCViewModel, groupBViewModel), rootViewModel.getChildren());
105+
}
106+
107+
@Test
108+
public void draggedOnBottomOfGroupAddsAfterIt() throws Exception {
109+
GroupNodeViewModel rootViewModel = getViewModelForGroup(new WordKeywordGroup("root", GroupHierarchyType.INCLUDING, "keywords", "A", true, ',', true));
110+
WordKeywordGroup groupA = new WordKeywordGroup("A", GroupHierarchyType.INCLUDING, "keywords", "A", true, ',', true);
111+
WordKeywordGroup groupB = new WordKeywordGroup("B", GroupHierarchyType.INCLUDING, "keywords", "A > B", true, ',', true);
112+
WordKeywordGroup groupC = new WordKeywordGroup("C", GroupHierarchyType.INCLUDING, "keywords", "A > B > B1", true, ',', true);
113+
GroupNodeViewModel groupAViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupA));
114+
GroupNodeViewModel groupBViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupB));
115+
GroupNodeViewModel groupCViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupC));
116+
117+
groupCViewModel.draggedOn(groupAViewModel, DroppingMouseLocation.BOTTOM);
118+
119+
assertEquals(Arrays.asList(groupAViewModel, groupCViewModel, groupBViewModel), rootViewModel.getChildren());
120+
}
121+
122+
@Test
123+
public void draggedOnBottomOfGroupAddsAfterItWhenSourceGroupWasBefore() throws Exception {
124+
GroupNodeViewModel rootViewModel = getViewModelForGroup(new WordKeywordGroup("root", GroupHierarchyType.INCLUDING, "keywords", "A", true, ',', true));
125+
WordKeywordGroup groupA = new WordKeywordGroup("A", GroupHierarchyType.INCLUDING, "keywords", "A", true, ',', true);
126+
WordKeywordGroup groupB = new WordKeywordGroup("B", GroupHierarchyType.INCLUDING, "keywords", "A > B", true, ',', true);
127+
WordKeywordGroup groupC = new WordKeywordGroup("C", GroupHierarchyType.INCLUDING, "keywords", "A > B > B1", true, ',', true);
128+
GroupNodeViewModel groupAViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupA));
129+
GroupNodeViewModel groupBViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupB));
130+
GroupNodeViewModel groupCViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupC));
131+
132+
groupAViewModel.draggedOn(groupBViewModel, DroppingMouseLocation.BOTTOM);
133+
134+
assertEquals(Arrays.asList(groupBViewModel, groupAViewModel, groupCViewModel), rootViewModel.getChildren());
135+
}
136+
137+
@Test
138+
public void draggedOnTopOfGroupAddsBeforeItWhenSourceGroupWasBefore() throws Exception {
139+
GroupNodeViewModel rootViewModel = getViewModelForGroup(new WordKeywordGroup("root", GroupHierarchyType.INCLUDING, "keywords", "A", true, ',', true));
140+
WordKeywordGroup groupA = new WordKeywordGroup("A", GroupHierarchyType.INCLUDING, "keywords", "A", true, ',', true);
141+
WordKeywordGroup groupB = new WordKeywordGroup("B", GroupHierarchyType.INCLUDING, "keywords", "A > B", true, ',', true);
142+
WordKeywordGroup groupC = new WordKeywordGroup("C", GroupHierarchyType.INCLUDING, "keywords", "A > B > B1", true, ',', true);
143+
GroupNodeViewModel groupAViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupA));
144+
GroupNodeViewModel groupBViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupB));
145+
GroupNodeViewModel groupCViewModel = getViewModelForGroup(rootViewModel.addSubgroup(groupC));
146+
147+
groupAViewModel.draggedOn(groupCViewModel, DroppingMouseLocation.TOP);
148+
149+
assertEquals(Arrays.asList(groupBViewModel, groupAViewModel, groupCViewModel), rootViewModel.getChildren());
150+
}
151+
90152
private GroupNodeViewModel getViewModelForGroup(AbstractGroup group) {
91153
return new GroupNodeViewModel(databaseContext, stateManager, taskExecutor, group);
92154
}
155+
156+
private GroupNodeViewModel getViewModelForGroup(GroupTreeNode group) {
157+
return new GroupNodeViewModel(databaseContext, stateManager, taskExecutor, group);
158+
}
93159
}

‎src/test/java/org/jabref/model/TreeNodeTest.java

+30
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,36 @@ public void moveToInSameLevelAddsAtEnd() {
172172
assertEquals(Arrays.asList(child2, child1), root.getChildren());
173173
}
174174

175+
@Test
176+
public void moveToInSameLevelWhenNodeWasBeforeTargetIndex() {
177+
TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock();
178+
TreeNodeTestData.TreeNodeMock child1 = new TreeNodeTestData.TreeNodeMock();
179+
TreeNodeTestData.TreeNodeMock child2 = new TreeNodeTestData.TreeNodeMock();
180+
TreeNodeTestData.TreeNodeMock child3 = new TreeNodeTestData.TreeNodeMock();
181+
root.addChild(child1);
182+
root.addChild(child2);
183+
root.addChild(child3);
184+
185+
child1.moveTo(root, 1);
186+
187+
assertEquals(Arrays.asList(child2, child1, child3), root.getChildren());
188+
}
189+
190+
@Test
191+
public void moveToInSameLevelWhenNodeWasAfterTargetIndex() {
192+
TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock();
193+
TreeNodeTestData.TreeNodeMock child1 = new TreeNodeTestData.TreeNodeMock();
194+
TreeNodeTestData.TreeNodeMock child2 = new TreeNodeTestData.TreeNodeMock();
195+
TreeNodeTestData.TreeNodeMock child3 = new TreeNodeTestData.TreeNodeMock();
196+
root.addChild(child1);
197+
root.addChild(child2);
198+
root.addChild(child3);
199+
200+
child3.moveTo(root, 1);
201+
202+
assertEquals(Arrays.asList(child1, child3, child2), root.getChildren());
203+
}
204+
175205
@Test
176206
public void getPathFromRootInSimpleTree() {
177207
TreeNodeTestData.TreeNodeMock root = new TreeNodeTestData.TreeNodeMock();

0 commit comments

Comments
 (0)