Hide completed background tasks (#11821)

* Reformat code comments * Finished tasks have 100% completeness Co-authored-by: Carl Christian Snethlage <[email protected]> * Remove finished tasks * Restructure if/then to avoid unnecessary creation of TaskProgressView * Remove subscription on hide * Try to show running tasks only * At shutdown at end, also do not show tasks * Add CHANGELOG.md entry * Fix IndexOutOfBoundsException * Fix hiding task progress view * Update CHANGELOG.md --------- Co-authored-by: Oliver Kopp <[email protected]> Co-authored-by: Carl Christian Snethlage <[email protected]>
JabRef · Sep 24, 2024 · f738629 · f738629
1 parent 7ee0274
commit f738629
Show file tree

Hide file tree

Showing 5 changed files with 42 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -41,6 +41,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 - The Pubmed/Medline Plain importer now imports the PMID field as well [#11488](https://github.com/JabRef/jabref/issues/11488)
 - The 'Check for updates' menu bar button is now always enabled. [#11485](https://github.com/JabRef/jabref/pull/11485)
 - JabRef respects the [configuration for storing files relative to the .bib file](https://docs.jabref.org/finding-sorting-and-cleaning-entries/filelinks#directories-for-files) in more cases. [#11492](https://github.com/JabRef/jabref/pull/11492)
+- JabRef does not show finished background tasks in the status bar popup. [#11821](https://github.com/JabRef/jabref/pull/11821)
 - We enhanced the indexing speed. [#11502](https://github.com/JabRef/jabref/pull/11502)
 - ⚠️ Renamed command line parameters `embeddBibfileInPdf` to `embedBibFileInPdf`, `writeMetadatatoPdf` to `writeMetadataToPdf`, and `writeXMPtoPdf` to `writeXmpToPdf`. [#11575](https://github.com/JabRef/jabref/pull/11575)
 - The browse button for a Custom theme now opens in the directory of the current used CSS file. [#11597](https://github.com/JabRef/jabref/pull/11597)

diff --git a/src/main/java/org/jabref/gui/JabRefDialogService.java b/src/main/java/org/jabref/gui/JabRefDialogService.java
@@ -379,7 +379,7 @@ public <V> void showProgressDialogAndWait(String title, String content, Task<V>
     @Override
     public Optional<ButtonType> showBackgroundProgressDialogAndWait(String title, String content, StateManager stateManager) {
         TaskProgressView<Task<?>> taskProgressView = new TaskProgressView<>();
-        EasyBind.bindContent(taskProgressView.getTasks(), stateManager.getBackgroundTasks());
+        EasyBind.bindContent(taskProgressView.getTasks(), stateManager.getRunningBackgroundTasks());
         taskProgressView.setRetainTasks(false);
         taskProgressView.setGraphicFactory(task -> ThemeManager.getDownloadIconTitleMap.getOrDefault(task.getTitle(), null));
 

diff --git a/src/main/java/org/jabref/gui/StateManager.java b/src/main/java/org/jabref/gui/StateManager.java
@@ -6,13 +6,16 @@
 import java.util.Optional;
 
 import javafx.beans.Observable;
+import javafx.beans.binding.Bindings;
+import javafx.beans.binding.BooleanBinding;
 import javafx.beans.property.IntegerProperty;
 import javafx.beans.property.ObjectProperty;
 import javafx.beans.property.SimpleIntegerProperty;
 import javafx.beans.property.SimpleObjectProperty;
 import javafx.collections.FXCollections;
 import javafx.collections.ObservableList;
 import javafx.collections.ObservableMap;
+import javafx.collections.transformation.FilteredList;
 import javafx.concurrent.Task;
 import javafx.scene.Node;
 import javafx.util.Pair;
@@ -64,10 +67,12 @@ public class StateManager {
     private final IntegerProperty searchResultSize = new SimpleIntegerProperty(0);
     private final IntegerProperty globalSearchResultSize = new SimpleIntegerProperty(0);
     private final OptionalObjectProperty<Node> focusOwner = OptionalObjectProperty.empty();
-    private final ObservableList<Pair<BackgroundTask<?>, Task<?>>> backgroundTasks = FXCollections.observableArrayList(task -> new Observable[] {task.getValue().progressProperty(), task.getValue().runningProperty()});
-    private final EasyBinding<Boolean> anyTaskRunning = EasyBind.reduce(backgroundTasks, tasks -> tasks.map(Pair::getValue).anyMatch(Task::isRunning));
-    private final EasyBinding<Boolean> anyTasksThatWillNotBeRecoveredRunning = EasyBind.reduce(backgroundTasks, tasks -> tasks.anyMatch(task -> !task.getKey().willBeRecoveredAutomatically() && task.getValue().isRunning()));
-    private final EasyBinding<Double> tasksProgress = EasyBind.reduce(backgroundTasks, tasks -> tasks.map(Pair::getValue).filter(Task::isRunning).mapToDouble(Task::getProgress).average().orElse(1));
+    private final ObservableList<Pair<BackgroundTask<?>, Task<?>>> backgroundTasksPairs = FXCollections.observableArrayList(task -> new Observable[] {task.getValue().progressProperty(), task.getValue().runningProperty()});
+    private final ObservableList<Task<?>> backgroundTasks = EasyBind.map(backgroundTasksPairs, Pair::getValue);
+    private final FilteredList<Task<?>> runningBackgroundTasks = new FilteredList<>(backgroundTasks, Task::isRunning);
+    private final BooleanBinding anyTaskRunning = Bindings.createBooleanBinding(() -> !runningBackgroundTasks.isEmpty(), runningBackgroundTasks);
+    private final EasyBinding<Boolean> anyTasksThatWillNotBeRecoveredRunning = EasyBind.reduce(backgroundTasksPairs, tasks -> tasks.anyMatch(task -> !task.getKey().willBeRecoveredAutomatically() && task.getValue().isRunning()));
+    private final EasyBinding<Double> tasksProgress = EasyBind.reduce(backgroundTasksPairs, tasks -> tasks.map(Pair::getValue).filter(Task::isRunning).mapToDouble(Task::getProgress).average().orElse(1));
     private final ObservableMap<String, DialogWindowState> dialogWindowStates = FXCollections.observableHashMap();
     private final ObservableList<SidePaneType> visibleSidePanes = FXCollections.observableArrayList();
     private final ObjectProperty<LastAutomaticFieldEditorEdit> lastAutomaticFieldEditorEdit = new SimpleObjectProperty<>();
@@ -153,14 +158,18 @@ public Optional<Node> getFocusOwner() {
     }
 
     public ObservableList<Task<?>> getBackgroundTasks() {
-        return EasyBind.map(backgroundTasks, Pair::getValue);
+        return backgroundTasks;
+    }
+
+    public ObservableList<Task<?>> getRunningBackgroundTasks() {
+        return runningBackgroundTasks;
     }
 
     public void addBackgroundTask(BackgroundTask<?> backgroundTask, Task<?> task) {
-        this.backgroundTasks.addFirst(new Pair<>(backgroundTask, task));
+        this.backgroundTasksPairs.addFirst(new Pair<>(backgroundTask, task));
     }
 
-    public EasyBinding<Boolean> getAnyTaskRunning() {
+    public BooleanBinding getAnyTaskRunning() {
         return anyTaskRunning;
     }
 

diff --git a/src/main/java/org/jabref/gui/frame/MainToolBar.java b/src/main/java/org/jabref/gui/frame/MainToolBar.java
@@ -46,6 +46,7 @@
 import org.jabref.model.util.FileUpdateMonitor;
 
 import com.tobiasdiez.easybind.EasyBind;
+import com.tobiasdiez.easybind.Subscription;
 import org.controlsfx.control.PopOver;
 import org.controlsfx.control.TaskProgressView;
 
@@ -65,6 +66,7 @@ public class MainToolBar extends ToolBar {
 
     private PopOver entryFromIdPopOver;
     private PopOver progressViewPopOver;
+    private Subscription taskProgressSubscription;
 
     public MainToolBar(LibraryTabContainer tabContainer,
                        PushToApplicationCommand pushToApplicationCommand,
@@ -209,15 +211,11 @@ Group createTaskIndicator() {
             }
         });
 
-        /*
-        The label of the indicator cannot be removed with styling. Therefore,
-        hide it and clip it to a square of (width x width) each time width is updated.
-         */
+        // The label of the indicator cannot be removed with styling. Therefore,
+        // hide it and clip it to a square of (width x width) each time width is updated.
         indicator.widthProperty().addListener((observable, oldValue, newValue) -> {
-            /*
-            The indeterminate spinner is wider than the determinate spinner.
-            We must make sure they are the same width for the clipping to result in a square of the same size always.
-             */
+            // The indeterminate spinner is wider than the determinate spinner.
+            // We must make sure they are the same width for the clipping to result in a square of the same size always.
             if (!indicator.isIndeterminate()) {
                 indicator.setPrefWidth(newValue.doubleValue());
             }
@@ -228,23 +226,25 @@ hide it and clip it to a square of (width x width) each time width is updated.
         });
 
         indicator.setOnMouseClicked(event -> {
+            if ((progressViewPopOver != null) && (progressViewPopOver.isShowing())) {
+                progressViewPopOver.hide();
+                taskProgressSubscription.unsubscribe();
+                return;
+            }
+
             TaskProgressView<Task<?>> taskProgressView = new TaskProgressView<>();
-            EasyBind.bindContent(taskProgressView.getTasks(), stateManager.getBackgroundTasks());
+            taskProgressSubscription = EasyBind.bindContent(taskProgressView.getTasks(), stateManager.getRunningBackgroundTasks());
             taskProgressView.setRetainTasks(false);
             taskProgressView.setGraphicFactory(task -> ThemeManager.getDownloadIconTitleMap.getOrDefault(task.getTitle(), null));
 
             if (progressViewPopOver == null) {
                 progressViewPopOver = new PopOver(taskProgressView);
                 progressViewPopOver.setTitle(Localization.lang("Background Tasks"));
                 progressViewPopOver.setArrowLocation(PopOver.ArrowLocation.RIGHT_TOP);
-                progressViewPopOver.setContentNode(taskProgressView);
-                progressViewPopOver.show(indicator);
-            } else if (progressViewPopOver.isShowing()) {
-                progressViewPopOver.hide();
-            } else {
-                progressViewPopOver.setContentNode(taskProgressView);
-                progressViewPopOver.show(indicator);
             }
+
+            progressViewPopOver.setContentNode(taskProgressView);
+            progressViewPopOver.show(indicator);
         });
 
         return new Group(indicator);

diff --git a/src/main/java/org/jabref/gui/util/UiTaskExecutor.java b/src/main/java/org/jabref/gui/util/UiTaskExecutor.java
@@ -196,9 +196,14 @@ protected V call() throws Exception {
             javaTask.setOnRunning(event -> onRunning.run());
         }
         Consumer<V> onSuccess = task.getOnSuccess();
-        if (onSuccess != null) {
-            javaTask.setOnSucceeded(event -> onSuccess.accept(javaTask.getValue()));
-        }
+        javaTask.setOnSucceeded(event -> {
+            // Set to 100% completed on completion
+            task.updateProgress(1, 1);
+
+            if (onSuccess != null) {
+                onSuccess.accept(javaTask.getValue());
+            }
+        });
         Consumer<Exception> onException = task.getOnException();
         if (onException != null) {
             javaTask.setOnFailed(event -> onException.accept(convertToException(javaTask.getException())));