From 6fef097d4347aa94d305569c0b8e6369f241d2ba Mon Sep 17 00:00:00 2001 From: Tobias Diez Date: Wed, 16 Oct 2019 21:43:59 +0200 Subject: [PATCH] Fix highlight issues for lists Fixes #5277 and fixes #5035. The problem was in both cases that the list view reuses nodes when scrolling (instead of creating new ones). --- CHANGELOG.md | 2 ++ .../jabref/gui/importer/ImportEntriesDialog.java | 2 -- .../importer/fetcher/WebSearchPaneViewModel.java | 2 +- .../java/org/jabref/gui/util/BindingsHelper.java | 6 ++++-- .../jabref/gui/util/ViewModelListCellFactory.java | 14 ++++++++++++-- 5 files changed, 19 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7979b1a365d..ccc8763d0ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,8 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `# - We fixed an issue where empty keywords lead to a strange display of automatic keyword groups. [#5333](https://github.com/JabRef/jabref/issues/5333) - We fixed an error where the default color of a new group was white instead of dark gray. [#4868](https://github.com/JabRef/jabref/issues/4868) - We fixed an issue where the first field in the entry editor got the focus while performing a different action (like searching). [#5084](https://github.com/JabRef/jabref/issues/5084) +- We fixed an issue where multiple entries were highlighted in the web search result after scrolling. [#5035](https://github.com/JabRef/jabref/issues/5035) +- We fixed an issue where the hover indication in the web search pane was not working. [#5277](https://github.com/JabRef/jabref/issues/5277) - We fixed an error mentioning "javafx.controls/com.sun.javafx.scene.control" that was thrown when interacting with the toolbar. - We fixed an error where a cleared search was restored after switching libraries. [#4846](https://github.com/JabRef/jabref/issues/4846) - We fixed an exception which occurred when trying to open a non-existing file from the "Recent files"-menu [#5334](https://github.com/JabRef/jabref/issues/5334) diff --git a/src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java b/src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java index 9a7864fde4f..1e8671150fb 100644 --- a/src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java +++ b/src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java @@ -27,7 +27,6 @@ import org.jabref.gui.icon.IconTheme; import org.jabref.gui.util.BackgroundTask; import org.jabref.gui.util.BaseDialog; -import org.jabref.gui.util.BindingsHelper; import org.jabref.gui.util.NoSelectionModel; import org.jabref.gui.util.TaskExecutor; import org.jabref.gui.util.TextFlowLimited; @@ -110,7 +109,6 @@ private void initialize() { HBox.setHgrow(entryNode, Priority.ALWAYS); HBox container = new HBox(entryNode, separator, addToggle); container.getStyleClass().add("entry-container"); - BindingsHelper.includePseudoClassWhen(container, entrySelected, addToggle.selectedProperty()); BackgroundTask.wrap(() -> viewModel.hasDuplicate(entry)).onSuccess(duplicateFound -> { if (duplicateFound) { diff --git a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java index b88a0cd5fe7..e1f9cc4a247 100644 --- a/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java +++ b/src/main/java/org/jabref/gui/importer/fetcher/WebSearchPaneViewModel.java @@ -97,7 +97,7 @@ public void search() { BackgroundTask> task = BackgroundTask.wrap(() -> activeFetcher.performSearch(getQuery().trim())) .withInitialMessage(Localization.lang("Processing %0", getQuery())); - task.onFailure(ex -> dialogService.showErrorDialogAndWait(ex)); + task.onFailure(dialogService::showErrorDialogAndWait); ImportEntriesDialog dialog = new ImportEntriesDialog(frame.getCurrentBasePanel().getBibDatabaseContext(), task); dialog.setTitle(activeFetcher.getName()); diff --git a/src/main/java/org/jabref/gui/util/BindingsHelper.java b/src/main/java/org/jabref/gui/util/BindingsHelper.java index f594c680607..a64c6a73f7f 100644 --- a/src/main/java/org/jabref/gui/util/BindingsHelper.java +++ b/src/main/java/org/jabref/gui/util/BindingsHelper.java @@ -24,6 +24,7 @@ import org.fxmisc.easybind.EasyBind; import org.fxmisc.easybind.PreboundBinding; +import org.fxmisc.easybind.Subscription; /** * Helper methods for javafx binding. @@ -43,12 +44,13 @@ public static BooleanBinding all(ObservableList source, Predicate pred return Bindings.createBooleanBinding(() -> !source.isEmpty() && source.stream().allMatch(predicate), source); } - public static void includePseudoClassWhen(Node node, PseudoClass pseudoClass, ObservableValue condition) { + public static Subscription includePseudoClassWhen(Node node, PseudoClass pseudoClass, ObservableValue condition) { Consumer changePseudoClass = value -> node.pseudoClassStateChanged(pseudoClass, value); - EasyBind.subscribe(condition, changePseudoClass); + Subscription subscription = EasyBind.subscribe(condition, changePseudoClass); // Put the pseudo class there depending on the current value changePseudoClass.accept(condition.getValue()); + return subscription; } /** diff --git a/src/main/java/org/jabref/gui/util/ViewModelListCellFactory.java b/src/main/java/org/jabref/gui/util/ViewModelListCellFactory.java index c0ce703dee0..72188027e8a 100644 --- a/src/main/java/org/jabref/gui/util/ViewModelListCellFactory.java +++ b/src/main/java/org/jabref/gui/util/ViewModelListCellFactory.java @@ -1,6 +1,8 @@ package org.jabref.gui.util; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.function.BiConsumer; @@ -20,6 +22,8 @@ import org.jabref.gui.icon.JabRefIcon; import org.jabref.model.strings.StringUtil; +import org.fxmisc.easybind.Subscription; + /** * Constructs a {@link ListCell} based on the view model of the row and a bunch of specified converter methods. * @@ -144,10 +148,16 @@ public ListCell call(ListView param) { return new ListCell() { + List subscriptions = new ArrayList<>(); + @Override protected void updateItem(T item, boolean empty) { super.updateItem(item, empty); + // Remove previous subscriptions + subscriptions.forEach(Subscription::unsubscribe); + subscriptions.clear(); + T viewModel = getItem(); if (empty || (viewModel == null)) { setText(null); @@ -190,10 +200,10 @@ protected void updateItem(T item, boolean empty) { } for (Map.Entry>> pseudoClassWithCondition : pseudoClasses.entrySet()) { ObservableValue condition = pseudoClassWithCondition.getValue().call(viewModel); - BindingsHelper.includePseudoClassWhen(this, pseudoClassWithCondition.getKey(), condition); + Subscription subscription = BindingsHelper.includePseudoClassWhen(this, pseudoClassWithCondition.getKey(), condition); + subscriptions.add(subscription); } } - getListView().refresh(); } }; }