Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement task progress indicator (and dialog) in the toolbar #6443

Merged
merged 42 commits into from
May 12, 2020
Merged
Changes from 1 commit
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
fdfe074
First draft of a task progress dialog
btut May 7, 2020
6fd1811
Added progress indicator in JabRefFrame
btut May 7, 2020
b883491
Beautified progressindicator and added localization
btut May 8, 2020
255a6e4
Changed to Task<?> in the Tasklist.
btut May 8, 2020
38dd89d
Resolved typing issues for bindings
btut May 8, 2020
1b2aaa6
Converted list of Properties to tasks for listbind
btut May 8, 2020
baf9bd0
New Tasks are first in the list
btut May 8, 2020
40a8feb
Use a PopOver instead of a dialog
btut May 8, 2020
cac989b
Only show download tasks
btut May 8, 2020
8f525b8
Better messages for download tasks
btut May 8, 2020
c877431
Type Witnesses are not needed any more.
btut May 8, 2020
1ad9598
Added extractor to task list
btut May 9, 2020
cd4e38e
Made anyTaskRunningBinding public
btut May 9, 2020
23f8cf0
Removed ObjectProperty wrapping
btut May 9, 2020
4628c3d
NOT WORKING: quit dialogue
btut May 9, 2020
90ff19e
Cleanup
btut May 9, 2020
62d7c14
Fix in dialog service
btut May 9, 2020
008d55e
Add extractor for isRunning
btut May 10, 2020
a9f4493
More informative (and working) quit dialog
btut May 10, 2020
66ac316
Added graphics callback
btut May 10, 2020
e9a7176
Fixed some style issues
btut May 10, 2020
d7442cc
Registered the save task as background task
btut May 10, 2020
5defe3e
Revert "Registered the save task as background task"
btut May 10, 2020
fba9c70
Added note on dialog-order upon close
btut May 10, 2020
6a62e6f
Updated changelog
btut May 10, 2020
a97af13
Fixed style
btut May 10, 2020
3ee4571
Merge branch 'master' of https://github.com/JabRef/jabref into featur…
btut May 10, 2020
44d9ca8
Quickfix for resizing indicator when indeterminate
btut May 11, 2020
bd2eefd
Styled dialog waiting for background tasks
btut May 11, 2020
41efc04
Minor style fix
btut May 11, 2020
2c9ccea
Removed Globals from DefaultTaskExecutor
btut May 11, 2020
ff9ce00
Removed WaitForBackgroundtasksFinishedDialog
btut May 11, 2020
d56138b
Made Bindings in StateManager private
btut May 11, 2020
cf10859
Added tooltip to progress indicator
btut May 11, 2020
fcb1d0c
Not working: own styleclass for toolbar progress indicator
btut May 11, 2020
396411a
Changed callback to method in BackgroundTask
btut May 11, 2020
e24c141
Fixed progress-indicator styling
btut May 12, 2020
478ee05
Improve getIcon method
tobiasdiez May 12, 2020
0557c67
Well, I said hopefully ;-)
tobiasdiez May 12, 2020
23b7e69
Revert "Well, I said hopefully ;-)"
btut May 12, 2020
e3ae796
Revert "Improve getIcon method"
btut May 12, 2020
3db3997
Improved readability in JabRefFrame
btut May 12, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
NOT WORKING: quit dialogue
  • Loading branch information
btut committed May 9, 2020
commit 4628c3dcb0b234025046ecfad6acd3d4aceddd2b
8 changes: 7 additions & 1 deletion src/main/java/org/jabref/gui/JabRefFrame.java
Original file line number Diff line number Diff line change
@@ -405,7 +405,13 @@ private void tearDownJabRef(List<String> filenames) {
* @return true if the user chose to quit; false otherwise
*/
public boolean quit() {
// First ask if the user really wants to close, if the library has not been saved since last save.
// First ask if the user really wants to close, if there are still background tasks running
if (stateManager.anyTaskRunningBinding.getValue()) {
WaitForBackgroundtasksFinishedDialog waitForBackgroundtasksFinishedDialog = new WaitForBackgroundtasksFinishedDialog(dialogService);
waitForBackgroundtasksFinishedDialog.showAndWait(stateManager);
}

// Then ask if the user really wants to close, if the library has not been saved since last save.
List<String> filenames = new ArrayList<>();
for (int i = 0; i < tabbedPane.getTabs().size(); i++) {
BasePanel panel = getBasePanelAt(i);
Original file line number Diff line number Diff line change
@@ -1,28 +1,30 @@
package org.jabref.gui;

import java.util.List;

import javafx.concurrent.Task;

import org.jabref.logic.l10n.Localization;

import java.util.List;

/**
* Dialog shown when closing of application needs to wait for a save operation to finish.
* Dialog shown when closing of application needs to wait for some background tasks.
*/
public class WaitForSaveFinishedDialog {
public class WaitForBackgroundtasksFinishedDialog {

private final DialogService dialogService;

public WaitForSaveFinishedDialog(DialogService dialogService) {
public WaitForBackgroundtasksFinishedDialog(DialogService dialogService) {
this.dialogService = dialogService;
}

public void showAndWait(List<BasePanel> basePanels) {
if (basePanels.stream().anyMatch(BasePanel::isSaving)) {
Task<Void> waitForSaveFinished = new Task<Void>() {
public void showAndWait(StateManager stateManager) {
if (stateManager.anyTaskRunningBinding.getValue()) {
Task<Void> waitForBackgroundtasksFinished = new Task<Void>() {
@Override
protected Void call() throws Exception {
while (basePanels.stream().anyMatch(BasePanel::isSaving)) {
System.out.println("THREAD STARTED");
while (stateManager.anyTaskRunningBinding.getValue()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a wild guess, could it be that actually all tasks are finished, because the TaskExectutor is already shutdown?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some logs in JabRefMain's stop method, it is run after the dialogue is already gone.

System.out.println("updated value to " + stateManager.tasksProgressBinding.getValue());
updateProgress(stateManager.tasksProgressBinding.getValue(), 1);
if (isCancelled()) {
return null;
} else {
@@ -33,11 +35,20 @@ protected Void call() throws Exception {
}
};

Thread th = new Thread(waitForBackgroundtasksFinished);
th.setDaemon(true);
th.start();

dialogService.showProgressDialogAndWait(
Localization.lang("Please wait..."),
Localization.lang("Waiting for save operation to finish") + "...",
waitForSaveFinished
Localization.lang("Waiting for background tasks to finish") + "...",
waitForBackgroundtasksFinished
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a log statement here as well to make sure that the progress dialog does indeed wait?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I currently have a log message after the loop to see whether the isCancelled property or my tasks-running property caused the stop. I now also put a log message where you suggested, just after the wait. They are exectued in the correct order. So yes, the wait does work, but the task exits too early.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I now also get a warning about the binding:

May 10, 2020 12:02:50 AM javafx.beans.binding.Bindings$1 computeValue
WARNING: Exception while evaluating binding
java.lang.IllegalStateException: Task must only be used from the FX Application Thread
at javafx.graphics/javafx.concurrent.Task.checkThread(Task.java:1220)
at javafx.graphics/javafx.concurrent.Task.isRunning(Task.java:987)
at java.base/java.util.stream.MatchOps$1MatchSink.accept(MatchOps.java:90)
at java.base/java.util.AbstractList$RandomAccessSpliterator.tryAdvance(AbstractList.java:706)
at java.base/java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:127)
at java.base/java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:502)
at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:488)
at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474)
at java.base/java.util.stream.MatchOps$MatchOp.evaluateSequential(MatchOps.java:230)
at java.base/java.util.stream.MatchOps$MatchOp.evaluateSequential(MatchOps.java:196)
at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.base/java.util.stream.ReferencePipeline.anyMatch(ReferencePipeline.java:528)
at org.jabref/org.jabref.gui.StateManager.lambda$new$1(StateManager.java:132)
at javafx.base/javafx.beans.binding.Bindings$1.computeValue(Bindings.java:157)
at javafx.base/javafx.beans.binding.BooleanBinding.get(BooleanBinding.java:155)
at org.jabref/org.jabref.gui.WaitForBackgroundtasksFinishedDialog$1.call(WaitForBackgroundtasksFinishedDialog.java:28)
at org.jabref/org.jabref.gui.WaitForBackgroundtasksFinishedDialog$1.call(WaitForBackgroundtasksFinishedDialog.java:24)
at javafx.graphics/javafx.concurrent.Task$TaskCallable.call(Task.java:1425)
at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
at java.base/java.lang.Thread.run(Thread.java:832)

I tried to run the task on the Javafx thread by using:

DefaultTaskExecutor.runInJavaFXThread(waitForBackgroundtasksFinished);

The loop is then run more often (I guess until the download is finished), but the gui is frozen because of the task.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem was that the extractor I added to the task list in StateManager did not include the isRunning property. It still did not run perfectly and was not pretty, therefore I moved to a custom dialog which works fine.

try {
Thread.sleep(60000);
} catch (InterruptedException e) {
e.printStackTrace();
}
}
}
}
1 change: 1 addition & 0 deletions src/main/resources/l10n/JabRef_en.properties
Original file line number Diff line number Diff line change
@@ -1005,6 +1005,7 @@ search\ expression=search expression

Optional\ fields\ 2=Optional fields 2
Waiting\ for\ save\ operation\ to\ finish=Waiting for save operation to finish
Waiting\ for\ background\ tasks\ to\ finish=Waiting for background tasks to finish

Find\ and\ remove\ duplicate\ BibTeX\ keys=Find and remove duplicate BibTeX keys
Expected\ syntax\ for\ --fetch\='<name\ of\ fetcher>\:<query>'=Expected syntax for --fetch='<name of fetcher>:<query>'