Hide completed background tasks #11821

LoayGhreeb · 2024-09-24T06:30:04Z

PR #11574 was reverted in #11818 due to an IndexOutOfBoundsException (see: #11574 (comment)).

This PR reinstates #11574 with the exception issue fixed.

Closes: #11701

Mandatory checks

Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for UI changes)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Co-authored-by: Carl Christian Snethlage <[email protected]>

koppor · 2024-09-24T06:43:09Z

The diff to the original attempt seems to be:

diff --git a/src/main/java/org/jabref/gui/StateManager.java b/src/main/java/org/jabref/gui/StateManager.java
index cf73b00da9..e75ffcdc89 100644
--- a/src/main/java/org/jabref/gui/StateManager.java
+++ b/src/main/java/org/jabref/gui/StateManager.java
@@ -6,6 +6,8 @@ import java.util.Objects;
 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;
@@ -65,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,16 +157,19 @@ public class StateManager {
         return focusOwner.get();
     }

+    public ObservableList<Task<?>> getBackgroundTasks() {
+        return backgroundTasks;
+    }
+
     public ObservableList<Task<?>> getRunningBackgroundTasks() {
-        FilteredList<Pair<BackgroundTask<?>, Task<?>>> pairs = new FilteredList<>(backgroundTasks, task -> task.getValue().isRunning());
-        return EasyBind.map(pairs, Pair::getValue);
+        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
index 2f61f953c6..03bbc72e3b 100644
--- a/src/main/java/org/jabref/gui/frame/MainToolBar.java
+++ b/src/main/java/org/jabref/gui/frame/MainToolBar.java
@@ -229,6 +229,7 @@ public class MainToolBar extends ToolBar {
             if ((progressViewPopOver != null) && (progressViewPopOver.isShowing())) {
                 progressViewPopOver.hide();
                 taskProgressSubscription.unsubscribe();
+                return;
             }

             TaskProgressView<Task<?>> taskProgressView = new TaskProgressView<>();
diff --git a/src/main/java/org/jabref/gui/util/UiTaskExecutor.java b/src/main/java/org/jabref/gui/util/UiTaskExecutor.java
index 15aeaf0105..43c0fad08e 100644
--- a/src/main/java/org/jabref/gui/util/UiTaskExecutor.java
+++ b/src/main/java/org/jabref/gui/util/UiTaskExecutor.java
@@ -145,7 +145,7 @@ public class UiTaskExecutor implements TaskExecutor {
     public void shutdown() {
         StateManager stateManager = Injector.instantiateModelOrService(StateManager.class);
         if (stateManager != null) {
-            stateManager.getRunningBackgroundTasks().forEach(Task::cancel);
+            stateManager.getBackgroundTasks().stream().filter(task -> !task.isDone()).forEach(Task::cancel);
         }
         executor.shutdownNow();
         scheduledExecutor.shutdownNow();
(END)
-    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,16 +157,19 @@ public class StateManager {
         return focusOwner.get();
     }

+    public ObservableList<Task<?>> getBackgroundTasks() {
+        return backgroundTasks;
+    }
+
     public ObservableList<Task<?>> getRunningBackgroundTasks() {
-        FilteredList<Pair<BackgroundTask<?>, Task<?>>> pairs = new FilteredList<>(backgroundTasks, task -> task.getValue().isRunning());
-        return EasyBind.map(pairs, Pair::getValue);
+        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
index 2f61f953c6..03bbc72e3b 100644
--- a/src/main/java/org/jabref/gui/frame/MainToolBar.java
+++ b/src/main/java/org/jabref/gui/frame/MainToolBar.java
@@ -229,6 +229,7 @@ public class MainToolBar extends ToolBar {
             if ((progressViewPopOver != null) && (progressViewPopOver.isShowing())) {
                 progressViewPopOver.hide();
                 taskProgressSubscription.unsubscribe();
+                return;
             }

             TaskProgressView<Task<?>> taskProgressView = new TaskProgressView<>();
diff --git a/src/main/java/org/jabref/gui/util/UiTaskExecutor.java b/src/main/java/org/jabref/gui/util/UiTaskExecutor.java
index 15aeaf0105..43c0fad08e 100644
--- a/src/main/java/org/jabref/gui/util/UiTaskExecutor.java
+++ b/src/main/java/org/jabref/gui/util/UiTaskExecutor.java
@@ -145,7 +145,7 @@ public class UiTaskExecutor implements TaskExecutor {
     public void shutdown() {
         StateManager stateManager = Injector.instantiateModelOrService(StateManager.class);
         if (stateManager != null) {
-            stateManager.getRunningBackgroundTasks().forEach(Task::cancel);
+            stateManager.getBackgroundTasks().stream().filter(task -> !task.isDone()).forEach(Task::cancel);
         }
         executor.shutdownNow();
         scheduledExecutor.shutdownNow();

github-actions · 2024-09-24T06:44:27Z

The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build.

LoayGhreeb · 2024-09-24T07:27:29Z

The diff to the original attempt seems to be

Yes, added two commits e810fc0, c8bfcbf.

Siedlerchr · 2024-09-24T16:40:20Z

I'm confused now Should we use this PR now or th eother?

LoayGhreeb · 2024-09-24T16:47:28Z

Both. This PR hides completed tasks from the popup, while the other PR controls when tasks should be shown to the user (i.e., added to the popup).

…

Message ID: ***@***.***>

koppor and others added 12 commits August 4, 2024 17:15

Reformat code comments

b213c59

Finished tasks have 100% completeness

2e14cf4

Co-authored-by: Carl Christian Snethlage <[email protected]>

Remove finished tasks

64da209

Restructure if/then to avoid unnecessary creation of TaskProgressView

21d0e90

Remove subscription on hide

6519b0b

Try to show running tasks only

13e3094

At shutdown at end, also do not show tasks

57eb232

Add CHANGELOG.md entry

4c1fa72

Merge branch 'main' into demo-background

887bf3d

Fix IndexOutOfBoundsException

e810fc0

Fix hiding task progress view

c8bfcbf

Update CHANGELOG.md

7d6ef6d

koppor requested a review from calixtus September 24, 2024 06:43

koppor added this to the 6.0-alpha milestone Sep 24, 2024

koppor changed the title ~~Hide completed backgorund tasks~~ Hide completed background tasks Sep 24, 2024

LoayGhreeb mentioned this pull request Sep 24, 2024

Fix indexing task progress #11822

Merged

6 tasks

calixtus approved these changes Sep 24, 2024

View reviewed changes

Siedlerchr added this pull request to the merge queue Sep 24, 2024

Merged via the queue into main with commit f738629 Sep 24, 2024
26 of 27 checks passed

Siedlerchr deleted the demo-background branch September 24, 2024 17:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hide completed background tasks #11821

Hide completed background tasks #11821

LoayGhreeb commented Sep 24, 2024 •

edited

Loading

koppor commented Sep 24, 2024

github-actions bot commented Sep 24, 2024 •

edited

Loading

LoayGhreeb commented Sep 24, 2024

Siedlerchr commented Sep 24, 2024

LoayGhreeb commented Sep 24, 2024 via email

Hide completed background tasks #11821

Hide completed background tasks #11821

Conversation

LoayGhreeb commented Sep 24, 2024 • edited Loading

Mandatory checks

koppor commented Sep 24, 2024

github-actions bot commented Sep 24, 2024 • edited Loading

LoayGhreeb commented Sep 24, 2024

Siedlerchr commented Sep 24, 2024

LoayGhreeb commented Sep 24, 2024 via email

LoayGhreeb commented Sep 24, 2024 •

edited

Loading

github-actions bot commented Sep 24, 2024 •

edited

Loading