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
Show file tree
Hide file tree
Changes from 38 commits
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
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)

### Changed
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 {
btut marked this conversation as resolved.
Show resolved Hide resolved
-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
79 changes: 77 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,22 @@ 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()) {
if (!(dialogService.showBackgroundProgressDialogAndWait(
Localization.lang("Please wait..."),
Localization.lang("Waiting for background tasks to finish. Quit anyway?"),
stateManager
).orElse(ButtonType.CANCEL) == ButtonType.YES)) {
Copy link
Member

Choose a reason for hiding this comment

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

This looks odd to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you please elaborate?
If you mean the condition at the end, I just want to make sure that JabRef only exists if the user pressed yes. So if the optional is empty I just put ButtonType.CANCEL so the comparison to YES returns false.
Do you want me to change this or add a comment? Maybe it would be clearer to store the result in a variable and check for (result.isPresent() && result.get() == ButtonType.YES).

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think this is more readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 3db3997

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 +537,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 +943,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