From 08d8a2ca06d56b5da40fee1cff51c9cd61c1ed72 Mon Sep 17 00:00:00 2001 From: Patrick Scheibe Date: Thu, 16 Nov 2017 04:30:14 +0100 Subject: [PATCH 1/4] Add delay to the search field --- .../org/jabref/gui/groups/GroupTreeController.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeController.java b/src/main/java/org/jabref/gui/groups/GroupTreeController.java index 357331789c3..397b461c5d9 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeController.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeController.java @@ -2,6 +2,7 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.time.Duration; import java.util.ArrayList; import java.util.List; import java.util.Optional; @@ -47,6 +48,8 @@ import org.controlsfx.control.textfield.CustomTextField; import org.controlsfx.control.textfield.TextFields; import org.fxmisc.easybind.EasyBind; +import org.reactfx.util.FxTimer; +import org.reactfx.util.Timer; public class GroupTreeController extends AbstractController { @@ -87,7 +90,13 @@ public void initialize() { this::updateSelection ); - viewModel.filterTextProperty().bind(searchField.textProperty()); + // We try to to prevent publishing changes in the searchfield directly to the search task that takes some time + // for larger group structures. + final Timer searchTask = FxTimer.create(Duration.ofMillis(400), () -> { + LOGGER.info("Run Search " + searchField.getText()); + viewModel.filterTextProperty().setValue(searchField.textProperty().getValue()); + }); + searchField.textProperty().addListener((observable, oldValue, newValue) -> searchTask.restart()); groupTree.rootProperty().bind( EasyBind.map(viewModel.rootGroupProperty(), From 12aab7e26da9b81f4b00f1db630ab5537919a0c2 Mon Sep 17 00:00:00 2001 From: Patrick Scheibe Date: Wed, 20 Dec 2017 06:08:32 +0100 Subject: [PATCH 2/4] Add cache for the current search. This is work in progress! --- .../org/jabref/gui/util/RecursiveTreeItem.java | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jabref/gui/util/RecursiveTreeItem.java b/src/main/java/org/jabref/gui/util/RecursiveTreeItem.java index 1117f9dad36..b3f6a754288 100644 --- a/src/main/java/org/jabref/gui/util/RecursiveTreeItem.java +++ b/src/main/java/org/jabref/gui/util/RecursiveTreeItem.java @@ -15,6 +15,7 @@ import javafx.scene.Node; import javafx.scene.control.TreeItem; import javafx.util.Callback; +import javafx.util.Pair; /** * Taken from https://gist.github.com/lestard/011e9ed4433f9eb791a8 @@ -25,6 +26,7 @@ public class RecursiveTreeItem extends TreeItem { private Callback> childrenFactory; private ObjectProperty> filter = new SimpleObjectProperty<>(); private FilteredList children; + private Pair lastVisibility; public RecursiveTreeItem(final T value, Callback> func) { this(value, func, null, null); @@ -52,7 +54,7 @@ private RecursiveTreeItem(final T value, Node graphic, Callback { + valueProperty().addListener((obs, oldValue, newValue) -> { if (newValue != null) { addChildrenListener(newValue); bindExpandedProperty(newValue, expandedProperty); @@ -76,7 +78,7 @@ private void addChildrenListener(T value) { while (change.next()) { if (change.wasRemoved()) { - change.getRemoved().forEach(t-> { + change.getRemoved().forEach(t -> { final List> itemsToRemove = getChildren().stream().filter(treeItem -> treeItem.getValue().equals(t)).collect(Collectors.toList()); getChildren().removeAll(itemsToRemove); }); @@ -99,12 +101,19 @@ private boolean showNode(T t) { return true; } + if (lastVisibility != null && lastVisibility.getKey().equals(t)) { + return lastVisibility.getValue(); + } + if (filter.get().test(t)) { // Node is directly matched -> so show it + lastVisibility = new Pair<>(t, true); return true; } // Are there children (or children of children...) that are matched? If yes we also need to show this node - return childrenFactory.call(t).stream().anyMatch(this::showNode); + final boolean isShown = childrenFactory.call(t).stream().anyMatch(this::showNode); + lastVisibility = new Pair<>(t, isShown); + return isShown; } } From 4246be76976ebb8552802ca69bd4fb36e504278b Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Wed, 27 Dec 2017 18:17:19 +0100 Subject: [PATCH 3/4] Reuse children instead of calling factory over and over again --- .../gui/groups/GroupTreeController.java | 4 +- .../org/jabref/gui/util/BindingsHelper.java | 2 +- .../jabref/gui/util/RecursiveTreeItem.java | 39 ++++++------------- 3 files changed, 14 insertions(+), 31 deletions(-) diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeController.java b/src/main/java/org/jabref/gui/groups/GroupTreeController.java index 397b461c5d9..a877885eabb 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeController.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeController.java @@ -90,10 +90,10 @@ public void initialize() { this::updateSelection ); - // We try to to prevent publishing changes in the searchfield directly to the search task that takes some time + // We try to to prevent publishing changes in the search field directly to the search task that takes some time // for larger group structures. final Timer searchTask = FxTimer.create(Duration.ofMillis(400), () -> { - LOGGER.info("Run Search " + searchField.getText()); + LOGGER.debug("Run group search " + searchField.getText()); viewModel.filterTextProperty().setValue(searchField.textProperty().getValue()); }); searchField.textProperty().addListener((observable, oldValue, newValue) -> searchTask.restart()); diff --git a/src/main/java/org/jabref/gui/util/BindingsHelper.java b/src/main/java/org/jabref/gui/util/BindingsHelper.java index 22b26f2db82..716ca03b899 100644 --- a/src/main/java/org/jabref/gui/util/BindingsHelper.java +++ b/src/main/java/org/jabref/gui/util/BindingsHelper.java @@ -68,7 +68,7 @@ public String getName() { * the items are converted when the are inserted (and at the initialization) instead of when they are accessed. * Thus the initial CPU overhead and memory consumption is higher but the access to list items is quicker. */ - public static MappedList mapBacked(ObservableList source, Function mapper) { + public static MappedList mapBacked(ObservableList source, Function mapper) { return new MappedList<>(source, mapper); } diff --git a/src/main/java/org/jabref/gui/util/RecursiveTreeItem.java b/src/main/java/org/jabref/gui/util/RecursiveTreeItem.java index b3f6a754288..c8c1f9db4ce 100644 --- a/src/main/java/org/jabref/gui/util/RecursiveTreeItem.java +++ b/src/main/java/org/jabref/gui/util/RecursiveTreeItem.java @@ -1,8 +1,6 @@ package org.jabref.gui.util; -import java.util.List; import java.util.function.Predicate; -import java.util.stream.Collectors; import javafx.beans.binding.Bindings; import javafx.beans.property.BooleanProperty; @@ -15,7 +13,6 @@ import javafx.scene.Node; import javafx.scene.control.TreeItem; import javafx.util.Callback; -import javafx.util.Pair; /** * Taken from https://gist.github.com/lestard/011e9ed4433f9eb791a8 @@ -25,8 +22,7 @@ public class RecursiveTreeItem extends TreeItem { private final Callback expandedProperty; private Callback> childrenFactory; private ObjectProperty> filter = new SimpleObjectProperty<>(); - private FilteredList children; - private Pair lastVisibility; + private FilteredList> children; public RecursiveTreeItem(final T value, Callback> func) { this(value, func, null, null); @@ -69,51 +65,38 @@ private void bindExpandedProperty(T value, Callback expanded } private void addChildrenListener(T value) { - children = new FilteredList<>(childrenFactory.call(value)); + children = new FilteredList<>( + BindingsHelper.mapBacked(childrenFactory.call(value), + child -> new RecursiveTreeItem<>(child, getGraphic(), childrenFactory, expandedProperty, filter))); children.predicateProperty().bind(Bindings.createObjectBinding(() -> this::showNode, filter)); - addAsChildren(children, 0); + getChildren().addAll(0, children); - children.addListener((ListChangeListener) change -> { + children.addListener((ListChangeListener>) change -> { while (change.next()) { if (change.wasRemoved()) { - change.getRemoved().forEach(t -> { - final List> itemsToRemove = getChildren().stream().filter(treeItem -> treeItem.getValue().equals(t)).collect(Collectors.toList()); - getChildren().removeAll(itemsToRemove); - }); + getChildren().removeAll(change.getRemoved()); } if (change.wasAdded()) { - addAsChildren(change.getAddedSubList(), change.getFrom()); + getChildren().addAll(change.getFrom(), change.getAddedSubList()); } } }); } - 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) { + private boolean showNode(RecursiveTreeItem node) { if (filter.get() == null) { return true; } - if (lastVisibility != null && lastVisibility.getKey().equals(t)) { - return lastVisibility.getValue(); - } - - if (filter.get().test(t)) { + if (filter.get().test(node.getValue())) { // Node is directly matched -> so show it - lastVisibility = new Pair<>(t, true); return true; } // Are there children (or children of children...) that are matched? If yes we also need to show this node - final boolean isShown = childrenFactory.call(t).stream().anyMatch(this::showNode); - lastVisibility = new Pair<>(t, isShown); - return isShown; + return node.children.getSource().stream().anyMatch(this::showNode); } } From 3092915214b958bb8a534a53f5438fa6e2e75c93 Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Wed, 3 Jan 2018 20:01:49 +0100 Subject: [PATCH 4/4] Fix bug --- src/main/java/org/jabref/gui/groups/GroupSidePane.java | 6 ++++-- .../java/org/jabref/gui/groups/GroupTreeController.java | 5 +++-- src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/jabref/gui/groups/GroupSidePane.java b/src/main/java/org/jabref/gui/groups/GroupSidePane.java index 326685e0d1b..86e4392c614 100644 --- a/src/main/java/org/jabref/gui/groups/GroupSidePane.java +++ b/src/main/java/org/jabref/gui/groups/GroupSidePane.java @@ -1,5 +1,6 @@ package org.jabref.gui.groups; +import java.util.Collections; import java.util.List; import javafx.application.Platform; @@ -16,6 +17,7 @@ import org.jabref.gui.customjfx.CustomJFXPanel; import org.jabref.gui.keyboard.KeyBinding; import org.jabref.gui.maintable.MainTableDataModel; +import org.jabref.logic.groups.DefaultGroupsFactory; import org.jabref.logic.l10n.Localization; import org.jabref.logic.util.OS; import org.jabref.model.entry.FieldName; @@ -83,8 +85,8 @@ public synchronized void listen(FieldChangedEvent event) { private void updateShownEntriesAccordingToSelectedGroups(List selectedGroups) { if ((selectedGroups == null) || selectedGroups.isEmpty()) { - // No selected group, nothing to do - return; + // No selected group, show all entries + selectedGroups = Collections.singletonList(new GroupTreeNode(DefaultGroupsFactory.getAllEntriesGroup())); } final MatcherSet searchRules = MatcherSets.build( diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeController.java b/src/main/java/org/jabref/gui/groups/GroupTreeController.java index a877885eabb..983bdb4c7ea 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeController.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeController.java @@ -42,6 +42,7 @@ import org.jabref.gui.util.TaskExecutor; import org.jabref.gui.util.ViewModelTreeTableCellFactory; import org.jabref.logic.l10n.Localization; +import org.jabref.model.groups.AllEntriesGroup; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -250,12 +251,12 @@ public void initialize() { } private void updateSelection(List> newSelectedGroups) { - if (newSelectedGroups == null) { + if (newSelectedGroups == null || newSelectedGroups.isEmpty()) { viewModel.selectedGroupsProperty().clear(); } else { List list = new ArrayList<>(); for (TreeItem model : newSelectedGroups) { - if (model != null && model.getValue() != null) { + if (model != null && model.getValue() != null && !(model.getValue().getGroupNode().getGroup() instanceof AllEntriesGroup)) { list.add(model.getValue()); } } diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java index bc11a195de2..a4e2889b9e7 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java @@ -92,7 +92,7 @@ private void onSelectedGroupChanged(ObservableList newValue) } currentDatabase.ifPresent(database -> { - if (newValue == null) { + if (newValue == null || newValue.isEmpty()) { stateManager.clearSelectedGroups(database); } else { stateManager.setSelectedGroups(database, newValue.stream().map(GroupNodeViewModel::getGroupNode).collect(Collectors.toList()));