Skip to content

Commit

Permalink
Implement task progress indicator (and dialog) in the toolbar (#6443)
Browse files Browse the repository at this point in the history
* First draft of a task progress dialog

Implemented a task progress dialog using a TaskProgressView.
Tasks show up, but without info. Neither the progress nor title and
message are shown.

* Added progress indicator in JabRefFrame

There now i a progress indicator at the rightmost postition of JabRefs
toolbar. It shows the average progress of all running background tasks.
Clicking it will show a TaskProgressDialog.

still looks ugly and the binding to the average progress does not seem
to be working.

* Beautified progressindicator and added localization

* Changed to Task<?> in the Tasklist.

This makes the view work with download tasks for example. Most other
tasks are still shown without title, message (because none are set) and
progress.
Also, there are a lot of tasks somehow.

The progress indicator in the main view still does not work as I can't
get the bindings to work.

* Resolved typing issues for bindings

The progress indicator is now successfully bound to the list of tasks.
However, tasks do not show up in the dialogue any more.

* Converted list of Properties to tasks for listbind

When using ObjectProperties in the list of tasks, we can use EasyBind to
convert the list into a list of tasks which in turn can be bound to the
list of tasks in the view.

With this, the basic functionality works. There is a progress indicator
in the toolbar that shows the average progress of all running background
tasks. It is indeterminate if any task has indeterminate progress and
shows 100% if no tasks are running.

Clicking it opens an overview of all running tasks and their progress.

Currently, there are many tasks running all the time. The only tasks
that were adapted for this to be pretty are the download tasks, and they
are also still missing an icon.

* New Tasks are first in the list

* Use a PopOver instead of a dialog

* Only show download tasks

* Better messages for download tasks

These are shorter and therefore the task progress view does not need a
horizontal scroll bar.

* Type Witnesses are not needed any more.

* Added extractor to task list

* Made anyTaskRunningBinding public

For consistency with other variables

* Removed ObjectProperty wrapping

* NOT WORKING: quit dialogue

* Cleanup

* Fix in dialog service

Make showProgressDialogAndWait actually not only show but also wait.

* Add extractor for isRunning

* More informative (and working) quit dialog

The dialog that is shown when the user quits JabRef during ongoing
background tasks now shows which background tasks are still running.

When all of them complete or the user chooses to quit anyway, JabRef
quits.

The user can also cancel the dialog. In that case, the dialog closes and
JobRef remains running.

* Added graphics callback

* Fixed some style issues

There are still some ImportOrder errors though where I can see no issue.

* Registered the save task as background task

This makes the dialog that pops up if background tasks are running wait
for pending saves.

* Revert "Registered the save task as background task"

This reverts commit d7442cc.

* Added note on dialog-order upon close

* Updated changelog

* Fixed style

* Quickfix for resizing indicator when indeterminate

Set the pref-width when the indicator is determinate to keep the
indeterminate state from making the indicator wider.

* Styled dialog waiting for background tasks

* Minor style fix

* Removed Globals from DefaultTaskExecutor

Addresses #6443 (comment)

* Removed WaitForBackgroundtasksFinishedDialog

More in line with the other JabRef dialogs
Addresses #6443 (comment)

* Made Bindings in StateManager private

Addresses #6443 (comment)

* Added tooltip to progress indicator

Addresses #6443 (comment)

* Not working: own styleclass for toolbar progress indicator

Tries to address #6443 (comment)

* Changed callback to method in BackgroundTask

Addresses #6443 (comment)

* Fixed progress-indicator styling

* Improve getIcon method

* Well, I said hopefully ;-)

* Revert "Well, I said hopefully ;-)"

This reverts commit 0557c67.

* Revert "Improve getIcon method"

This reverts commit 478ee05.

* Improved readability in JabRefFrame

Co-authored-by: Benedikt Tutzer <benedikt.tutzer@gmail.com>
Co-authored-by: Tobias Diez <tobiasdiez@gmx.de>
  • Loading branch information
3 people committed May 12, 2020
1 parent 9a62760 commit 4423d27
Show file tree
Hide file tree
Showing 12 changed files with 254 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We now show the number of items found and selected to import in the online search dialog. [#6248](https://github.com/JabRef/jabref/pull/6248)
- We created a new install screen for macOS. [#5759](https://github.com/JabRef/jabref/issues/5759)
- We implemented an option to download fulltext files while importing. [#6381](https://github.com/JabRef/jabref/pull/6381)
- We added a progress-indicator showing the average progress of background tasks to the toolbar. Clicking it reveals a pop-over with a list of running background tasks. [6443](https://github.com/JabRef/jabref/pull/6443)
- We fixed the bug when strike the delete key in the text field. [#6421](https://github.com/JabRef/jabref/issues/6421)
- We added support for jumping to target entry when typing letter/digit after sorting a column in maintable [#6146](https://github.com/JabRef/jabref/issues/6146)

Expand Down
12 changes: 6 additions & 6 deletions src/main/java/org/jabref/Globals.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,13 @@ public class Globals {
// Remote listener
public static final RemoteListenerServerLifecycle REMOTE_LISTENER = new RemoteListenerServerLifecycle();

/**
* Manager for the state of the GUI.
*/
public static StateManager stateManager = new StateManager();

public static final ImportFormatReader IMPORT_FORMAT_READER = new ImportFormatReader();
public static final TaskExecutor TASK_EXECUTOR = new DefaultTaskExecutor();
public static final TaskExecutor TASK_EXECUTOR = new DefaultTaskExecutor(stateManager);

/**
* Each test case initializes this field if required
Expand All @@ -64,11 +69,6 @@ public class Globals {
*/
public static ProtectedTermsLoader protectedTermsLoader;

/**
* Manager for the state of the GUI.
*/
public static StateManager stateManager = new StateManager();

public static ExporterFactory exportFactory;
public static CountingUndoManager undoManager = new CountingUndoManager();
public static BibEntryTypesManager entryTypesManager = new BibEntryTypesManager();
Expand Down
18 changes: 18 additions & 0 deletions src/main/java/org/jabref/gui/Base.css
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,24 @@
-fx-padding: -0.1em 0.5em 0.5em 0.5em;
}

.progress-indicator {
-fx-progress-color: -jr-theme;
-fx-border-width: 0px;
-fx-background-color: -jr-icon-background;
}

.progress-indicator:hover {
-fx-background-color: -jr-icon-background-active;
}

.progress-indicatorToolbar {
-fx-padding: 0.5em;
}

.progress-indicatorToolbar .percentage {
-fx-fill:null;
}

.check-box {
-fx-label-padding: 0.0em 0.0em 0.0em 0.75em;
-fx-text-fill: -fx-text-background-color;
Expand Down
15 changes: 14 additions & 1 deletion src/main/java/org/jabref/gui/DialogService.java
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,21 @@ Optional<ButtonType> showCustomButtonDialogAndWait(Alert.AlertType type, String
* @param title title of the dialog
* @param content message to show above the progress bar
* @param task The {@link Task} which executes the work and for which to show the dialog
* @return
*/
<V> void showProgressDialogAndWait(String title, String content, Task<V> task);
<V> Optional<Void> showProgressDialogAndWait(String title, String content, Task<V> task);

/**
* Constructs and shows a dialog showing the progress of running background tasks.
* Clicking cancel will cancel the underlying service and close the dialog.
* The dialog will exit as soon as none of the background tasks are running
*
* @param title title of the dialog
* @param content message to show below the list of background tasks
* @param stateManager The {@link StateManager} which contains the background tasks
* @return
*/
<V> Optional<ButtonType> showBackgroundProgressDialogAndWait(String title, String content, StateManager stateManager);

/**
* Notify the user in an non-blocking way (i.e., in form of toast in a snackbar).
Expand Down
42 changes: 40 additions & 2 deletions src/main/java/org/jabref/gui/JabRefDialogService.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,19 @@
import javafx.scene.control.CheckBox;
import javafx.scene.control.ChoiceDialog;
import javafx.scene.control.DialogPane;
import javafx.scene.control.Label;
import javafx.scene.control.TextInputDialog;
import javafx.scene.layout.Pane;
import javafx.scene.layout.Region;
import javafx.scene.layout.VBox;
import javafx.stage.DirectoryChooser;
import javafx.stage.FileChooser;
import javafx.stage.Stage;
import javafx.stage.Window;
import javafx.util.Duration;

import org.jabref.gui.icon.IconTheme;
import org.jabref.gui.util.BackgroundTask;
import org.jabref.gui.util.DirectoryDialogConfiguration;
import org.jabref.gui.util.FileDialogConfiguration;
import org.jabref.gui.util.ThemeLoader;
Expand All @@ -43,8 +46,10 @@
import com.jfoenix.controls.JFXSnackbar;
import com.jfoenix.controls.JFXSnackbar.SnackbarEvent;
import com.jfoenix.controls.JFXSnackbarLayout;
import org.controlsfx.control.TaskProgressView;
import org.controlsfx.dialog.ExceptionDialog;
import org.controlsfx.dialog.ProgressDialog;
import org.fxmisc.easybind.EasyBind;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -267,7 +272,7 @@ public <R> Optional<R> showCustomDialogAndWait(Dialog<R> dialog) {
}

@Override
public <V> void showProgressDialogAndWait(String title, String content, Task<V> task) {
public <V> Optional<Void> showProgressDialogAndWait(String title, String content, Task<V> task) {
ProgressDialog progressDialog = new ProgressDialog(task);
progressDialog.setHeaderText(null);
progressDialog.setTitle(title);
Expand All @@ -283,7 +288,40 @@ public <V> void showProgressDialogAndWait(String title, String content, Task<V>
progressDialog.close();
});
themeLoader.installCss(progressDialog.getDialogPane().getScene(), preferences);
progressDialog.show();
return progressDialog.showAndWait();
}

@Override
public <V> Optional<ButtonType> showBackgroundProgressDialogAndWait(String title, String content, StateManager stateManager) {
TaskProgressView taskProgressView = new TaskProgressView();
EasyBind.listBind(taskProgressView.getTasks(), stateManager.getBackgroundTasks());
taskProgressView.setRetainTasks(false);
taskProgressView.setGraphicFactory(BackgroundTask::getIcon);

Label message = new Label(content);

VBox box = new VBox(taskProgressView, message);

DialogPane contentPane = new DialogPane();
contentPane.setContent(box);

FXDialog alert = new FXDialog(AlertType.WARNING, title);
alert.setDialogPane(contentPane);
alert.getButtonTypes().setAll(ButtonType.YES, ButtonType.CANCEL);
alert.getDialogPane().setMinHeight(Region.USE_PREF_SIZE);
alert.setResizable(true);
themeLoader.installCss(alert.getDialogPane().getScene(), preferences);

stateManager.getAnyTaskRunning().addListener((observable, oldValue, newValue) -> {
if (!newValue) {
alert.setResult(ButtonType.YES);
alert.close();
}
});

Dialog<ButtonType> dialog = () -> alert.showAndWait();

return showCustomDialogAndWait(dialog);
}

@Override
Expand Down
80 changes: 78 additions & 2 deletions src/main/java/org/jabref/gui/JabRefFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import javafx.beans.value.ChangeListener;
import javafx.beans.value.ObservableValue;
import javafx.geometry.Orientation;
import javafx.scene.Group;
import javafx.scene.Node;
import javafx.scene.control.Alert;
import javafx.scene.control.Button;
Expand All @@ -23,6 +24,7 @@
import javafx.scene.control.Menu;
import javafx.scene.control.MenuBar;
import javafx.scene.control.MenuItem;
import javafx.scene.control.ProgressIndicator;
import javafx.scene.control.Separator;
import javafx.scene.control.SeparatorMenuItem;
import javafx.scene.control.SplitPane;
Expand All @@ -38,6 +40,7 @@
import javafx.scene.layout.Pane;
import javafx.scene.layout.Priority;
import javafx.scene.layout.VBox;
import javafx.scene.shape.Rectangle;
import javafx.stage.Stage;

import org.jabref.Globals;
Expand Down Expand Up @@ -135,6 +138,8 @@
import org.jabref.preferences.LastFocusedTabPreferences;

import com.google.common.eventbus.Subscribe;
import org.controlsfx.control.PopOver;
import org.controlsfx.control.TaskProgressView;
import org.fxmisc.easybind.EasyBind;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -397,7 +402,23 @@ 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
/*
It is important to wait for unfinished background tasks before checking if a save-operation is needed, because
the background tasks may make changes themselves that need saving.
*/
if (stateManager.getAnyTaskRunning().getValue()) {
Optional<ButtonType> shouldClose = dialogService.showBackgroundProgressDialogAndWait(
Localization.lang("Please wait..."),
Localization.lang("Waiting for background tasks to finish. Quit anyway?"),
stateManager
);
if (!(shouldClose.isPresent() && shouldClose.get() == ButtonType.YES)) {
return false;
}
}

// 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);
Expand Down Expand Up @@ -517,7 +538,9 @@ private Node createToolbar() {
new Separator(Orientation.VERTICAL),
factory.createIconButton(StandardActions.OPEN_GITHUB, new OpenBrowserAction("https://github.com/JabRef/jabref")),
factory.createIconButton(StandardActions.OPEN_FACEBOOK, new OpenBrowserAction("https://www.facebook.com/JabRef/")),
factory.createIconButton(StandardActions.OPEN_TWITTER, new OpenBrowserAction("https://twitter.com/jabref_org"))
factory.createIconButton(StandardActions.OPEN_TWITTER, new OpenBrowserAction("https://twitter.com/jabref_org")),
new Separator(Orientation.VERTICAL),
createTaskIndicator()
);

HBox.setHgrow(globalSearchBar, Priority.ALWAYS);
Expand Down Expand Up @@ -921,6 +944,59 @@ private MenuBar createMenu() {
return menu;
}

private Group createTaskIndicator() {
ProgressIndicator indicator = new ProgressIndicator();
indicator.getStyleClass().add("progress-indicatorToolbar");
indicator.progressProperty().bind(stateManager.getTasksProgress());

Tooltip someTasksRunning = new Tooltip(Localization.lang("Background Tasks are running"));
Tooltip noTasksRunning = new Tooltip(Localization.lang("Background Tasks are done"));
indicator.setTooltip(noTasksRunning);
stateManager.getAnyTaskRunning().addListener(new ChangeListener<Boolean>() {
@Override
public void changed(ObservableValue<? extends Boolean> observable, Boolean oldValue, Boolean newValue) {
if (newValue.booleanValue()) {
indicator.setTooltip(someTasksRunning);
} else {
indicator.setTooltip(noTasksRunning);
}
}
});

/*
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.
*/
if (!indicator.isIndeterminate()) {
indicator.setPrefWidth(newValue.doubleValue());
}
if (newValue.doubleValue() > 0) {
Rectangle clip = new Rectangle(newValue.doubleValue(), newValue.doubleValue());
indicator.setClip(clip);
}
});

indicator.setOnMouseClicked(event -> {
TaskProgressView taskProgressView = new TaskProgressView();
EasyBind.listBind(taskProgressView.getTasks(), stateManager.getBackgroundTasks());
taskProgressView.setRetainTasks(true);
taskProgressView.setGraphicFactory(BackgroundTask::getIcon);

PopOver progressViewPopOver = new PopOver(taskProgressView);
progressViewPopOver.setTitle(Localization.lang("Background Tasks"));
progressViewPopOver.setArrowLocation(PopOver.ArrowLocation.RIGHT_TOP);

progressViewPopOver.show(indicator);
});

return new Group(indicator);
}

public void addParserResult(ParserResult parserResult, boolean focusPanel) {
if (parserResult.toOpenTab()) {
// Add the entries to the open tab.
Expand Down
31 changes: 31 additions & 0 deletions src/main/java/org/jabref/gui/StateManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,18 @@
import java.util.Optional;
import java.util.stream.Collectors;

import javafx.beans.Observable;
import javafx.beans.binding.Bindings;
import javafx.beans.binding.BooleanBinding;
import javafx.beans.binding.DoubleBinding;
import javafx.beans.property.IntegerProperty;
import javafx.beans.property.ReadOnlyListProperty;
import javafx.beans.property.ReadOnlyListWrapper;
import javafx.beans.property.SimpleIntegerProperty;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;
import javafx.collections.ObservableMap;
import javafx.concurrent.Task;
import javafx.scene.Node;

import org.jabref.gui.util.CustomLocalDragboard;
Expand Down Expand Up @@ -41,6 +45,17 @@ public class StateManager {
private final OptionalObjectProperty<SearchQuery> activeSearchQuery = OptionalObjectProperty.empty();
private final ObservableMap<BibDatabaseContext, IntegerProperty> searchResultMap = FXCollections.observableHashMap();
private final OptionalObjectProperty<Node> focusOwner = OptionalObjectProperty.empty();
private final ObservableList<Task<?>> backgroundTasks = FXCollections.observableArrayList(taskProperty -> {
return new Observable[] {taskProperty.progressProperty(), taskProperty.runningProperty()};
});

private BooleanBinding anyTaskRunning = Bindings.createBooleanBinding(
() -> backgroundTasks.stream().anyMatch(Task::isRunning), backgroundTasks
);

private DoubleBinding tasksProgress = Bindings.createDoubleBinding(
() -> backgroundTasks.stream().filter(Task::isRunning).mapToDouble(Task::getProgress).average().orElse(1), backgroundTasks
);

public StateManager() {
activeGroups.bind(Bindings.valueAt(selectedGroups, activeDatabase.orElse(null)));
Expand Down Expand Up @@ -112,4 +127,20 @@ public void setSearchQuery(SearchQuery searchQuery) {
public OptionalObjectProperty<Node> focusOwnerProperty() { return focusOwner; }

public Optional<Node> getFocusOwner() { return focusOwner.get(); }

public ObservableList<Task<?>> getBackgroundTasks() {
return backgroundTasks;
}

public void addBackgroundTask(Task<?> backgroundTask) {
this.backgroundTasks.add(0, backgroundTask);
}

public BooleanBinding getAnyTaskRunning() {
return anyTaskRunning;
}

public DoubleBinding getTasksProgress() {
return tasksProgress;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,10 @@ private void addLinkedFileFromURL(BibDatabaseContext databaseContext, URL url, B
dialogService.notify(Localization.lang("Finished downloading full text document for entry %0.",
entry.getCiteKeyOptional().orElse(Localization.lang("undefined"))));
});
downloadTask.titleProperty().set(Localization.lang("Downloading"));
downloadTask.messageProperty().set(
Localization.lang("Fulltext for") + ": " + entry.getCiteKeyOptional().orElse(Localization.lang("New entry")));
downloadTask.showToUser(true);
Globals.TASK_EXECUTOR.execute(downloadTask);
} catch (MalformedURLException exception) {
dialogService.showErrorDialogAndWait(Localization.lang("Invalid URL"), exception);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,10 @@ public void download() {
entry.addFile(0, newLinkedFile);
});
downloadProgress.bind(downloadTask.workDonePercentageProperty());
downloadTask.titleProperty().set(Localization.lang("Downloading"));
downloadTask.messageProperty().set(
Localization.lang("Fulltext for") + ": " + entry.getCiteKeyOptional().orElse(Localization.lang("New entry")));
downloadTask.showToUser(true);
taskExecutor.execute(downloadTask);
} catch (MalformedURLException exception) {
dialogService.showErrorDialogAndWait(Localization.lang("Invalid URL"), exception);
Expand Down
Loading

0 comments on commit 4423d27

Please sign in to comment.