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

Fix for application dialogs opening in wrong displays #7273

Merged
merged 10 commits into from
Jan 2, 2021
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 fixed an issue with the style of highlighted check boxes while searching in preferences. [#7226](https://github.com/JabRef/jabref/issues/7226)
- We fixed an issue where the option "Move file to file directory" was disabled in the entry editor for all files [#7194](https://github.com/JabRef/jabref/issues/7194)
- We fixed an issue where application dialogs were opening in the wrong display when using multiple screens [#7273](https://github.com/JabRef/jabref/pull/7273)

### Removed

Expand Down
17 changes: 17 additions & 0 deletions docs/getting-into-the-code/code-howtos.md
Original file line number Diff line number Diff line change
Expand Up @@ -398,3 +398,20 @@ All radio buttons that should be grouped together need to have a ToggleGroup def
</VBox>
```

### JavaFX Dialogs

All dialogs should be displayed to the user via `DialogService` interface methods.
`DialogService` provides methods to display various dialogs (including custom ones) to the user.
It also ensures the displayed dialog opens on the correct window via `initOwner()` (for cases where the user has multiple screens).
The following code snippet demonstrates how a custom dialog is displayed to the user:

```java
dialogService.showCustomDialog(new DocumentViewerView());
```

If an instance of `DialogService` is unavailable within current class/scope in which the dialog needs to be displayed,
`DialogService` can be instantiated via the code snippet shown as follows:

```java
DialogService dialogService = Injector.instantiateModelOrService(DialogService.class);
```
10 changes: 9 additions & 1 deletion src/main/java/org/jabref/gui/DialogService.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import javafx.scene.control.DialogPane;
import javafx.scene.control.TextInputDialog;

import org.jabref.gui.util.BaseDialog;
import org.jabref.gui.util.DirectoryDialogConfiguration;
import org.jabref.gui.util.FileDialogConfiguration;
import org.jabref.logic.l10n.Localization;
Expand Down Expand Up @@ -160,6 +161,13 @@ boolean showConfirmationDialogWithOptOutAndWait(String title, String content,
String okButtonLabel, String cancelButtonLabel,
String optOutMessage, Consumer<Boolean> optOutAction);

/**
* Shows a custom dialog without returning any results.
*
* @param dialog dialog to show
*/
void showCustomDialog(BaseDialog<?> dialog);

/**
* This will create and display a new dialog of the specified
* {@link Alert.AlertType} but with user defined buttons as optional
Expand All @@ -184,7 +192,7 @@ Optional<ButtonType> showCustomButtonDialogAndWait(Alert.AlertType type, String
* @param dialog dialog to show
* @param <R> type of result
*/
<R> Optional<R> showCustomDialogAndWait(Dialog<R> dialog);
<R> Optional<R> showCustomDialogAndWait(javafx.scene.control.Dialog<R> dialog);

/**
* Constructs and shows a canceable {@link ProgressDialog}. Clicking cancel will cancel the underlying service and close the dialog
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/EntryTypeViewModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ public void runFetcherWorker() {
Optional<BibEntry> duplicate = new DuplicateCheck(Globals.entryTypesManager).containsDuplicate(libraryTab.getDatabase(), entry, libraryTab.getBibDatabaseContext().getMode());
if ((duplicate.isPresent())) {
DuplicateResolverDialog dialog = new DuplicateResolverDialog(entry, duplicate.get(), DuplicateResolverDialog.DuplicateResolverType.IMPORT_CHECK, libraryTab.getBibDatabaseContext(), stateManager);
switch (dialog.showAndWait().orElse(DuplicateResolverDialog.DuplicateResolverResult.BREAK)) {
switch (dialogService.showCustomDialogAndWait(dialog).orElse(DuplicateResolverDialog.DuplicateResolverResult.BREAK)) {
case KEEP_LEFT:
libraryTab.getDatabase().removeEntry(duplicate.get());
libraryTab.getDatabase().insertEntry(entry);
Expand Down
30 changes: 23 additions & 7 deletions src/main/java/org/jabref/gui/JabRefDialogService.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@

import org.jabref.gui.icon.IconTheme;
import org.jabref.gui.util.BackgroundTask;
import org.jabref.gui.util.BaseDialog;
import org.jabref.gui.util.DirectoryDialogConfiguration;
import org.jabref.gui.util.FileDialogConfiguration;
import org.jabref.gui.util.ZipFileChooser;
Expand Down Expand Up @@ -78,17 +79,18 @@ public JabRefDialogService(Window mainWindow, Pane mainPane, PreferencesService
JabRefDialogService.preferences = preferences;
}

private static FXDialog createDialog(AlertType type, String title, String content) {
private FXDialog createDialog(AlertType type, String title, String content) {
FXDialog alert = new FXDialog(type, title, true);
preferences.getTheme().installCss(alert.getDialogPane().getScene());
alert.setHeaderText(null);
alert.setContentText(content);
alert.getDialogPane().setMinHeight(Region.USE_PREF_SIZE);
alert.initOwner(mainWindow);
return alert;
}

private static FXDialog createDialogWithOptOut(AlertType type, String title, String content,
String optOutMessage, Consumer<Boolean> optOutAction) {
private FXDialog createDialogWithOptOut(AlertType type, String title, String content,
String optOutMessage, Consumer<Boolean> optOutAction) {
FXDialog alert = new FXDialog(type, title, true);
// Need to force the alert to layout in order to grab the graphic as we are replacing the dialog pane with a custom pane
alert.getDialogPane().applyCss();
Expand Down Expand Up @@ -117,6 +119,7 @@ protected Node createDetailsButton() {
alert.setHeaderText(null);
alert.setContentText(content);
alert.getDialogPane().setMinHeight(Region.USE_PREF_SIZE);
alert.initOwner(mainWindow);
return alert;
}

Expand All @@ -136,6 +139,7 @@ public <T> Optional<T> showChoiceDialogAndWait(String title, String content, Str
choiceDialog.setHeaderText(title);
choiceDialog.setTitle(title);
choiceDialog.setContentText(content);
choiceDialog.initOwner(mainWindow);
preferences.getTheme().installCss(choiceDialog.getDialogPane().getScene());
return choiceDialog.showAndWait();
}
Expand All @@ -145,6 +149,7 @@ public Optional<String> showInputDialogAndWait(String title, String content) {
TextInputDialog inputDialog = new TextInputDialog();
inputDialog.setHeaderText(title);
inputDialog.setContentText(content);
inputDialog.initOwner(mainWindow);
preferences.getTheme().installCss(inputDialog.getDialogPane().getScene());
return inputDialog.showAndWait();
}
Expand All @@ -154,6 +159,7 @@ public Optional<String> showInputDialogWithDefaultAndWait(String title, String c
TextInputDialog inputDialog = new TextInputDialog(defaultValue);
inputDialog.setHeaderText(title);
inputDialog.setContentText(content);
inputDialog.initOwner(mainWindow);
preferences.getTheme().installCss(inputDialog.getDialogPane().getScene());
return inputDialog.showAndWait();
}
Expand Down Expand Up @@ -181,6 +187,7 @@ public void showErrorDialogAndWait(String message, Throwable exception) {
ExceptionDialog exceptionDialog = new ExceptionDialog(exception);
exceptionDialog.getDialogPane().setMaxWidth(mainWindow.getWidth() / 2);
exceptionDialog.setHeaderText(message);
exceptionDialog.initOwner(mainWindow);
preferences.getTheme().installCss(exceptionDialog.getDialogPane().getScene());
exceptionDialog.showAndWait();
}
Expand All @@ -190,6 +197,7 @@ public void showErrorDialogAndWait(String title, String content, Throwable excep
ExceptionDialog exceptionDialog = new ExceptionDialog(exception);
exceptionDialog.setHeaderText(title);
exceptionDialog.setContentText(content);
exceptionDialog.initOwner(mainWindow);
preferences.getTheme().installCss(exceptionDialog.getDialogPane().getScene());
exceptionDialog.showAndWait();
}
Expand Down Expand Up @@ -259,12 +267,14 @@ public Optional<ButtonType> showCustomDialogAndWait(String title, DialogPane con
alert.getButtonTypes().setAll(buttonTypes);
alert.getDialogPane().setMinHeight(Region.USE_PREF_SIZE);
alert.setResizable(true);
alert.initOwner(mainWindow);
preferences.getTheme().installCss(alert.getDialogPane().getScene());
return alert.showAndWait();
}

@Override
public <R> Optional<R> showCustomDialogAndWait(Dialog<R> dialog) {
public <R> Optional<R> showCustomDialogAndWait(javafx.scene.control.Dialog<R> dialog) {
dialog.initOwner(mainWindow);
return dialog.showAndWait();
}

Expand All @@ -285,6 +295,7 @@ public <V> void showProgressDialog(String title, String content, Task<V> task) {
progressDialog.close();
});
preferences.getTheme().installCss(progressDialog.getDialogPane().getScene());
progressDialog.initOwner(mainWindow);
progressDialog.show();
}

Expand All @@ -307,6 +318,7 @@ public <V> Optional<ButtonType> showBackgroundProgressDialogAndWait(String title
alert.getButtonTypes().setAll(ButtonType.YES, ButtonType.CANCEL);
alert.getDialogPane().setMinHeight(Region.USE_PREF_SIZE);
alert.setResizable(true);
alert.initOwner(mainWindow);
preferences.getTheme().installCss(alert.getDialogPane().getScene());

stateManager.getAnyTaskRunning().addListener((observable, oldValue, newValue) -> {
Expand All @@ -316,9 +328,7 @@ public <V> Optional<ButtonType> showBackgroundProgressDialogAndWait(String title
}
});

Dialog<ButtonType> dialog = alert::showAndWait;

return showCustomDialogAndWait(dialog);
return alert.showAndWait();
}

@Override
Expand Down Expand Up @@ -385,4 +395,10 @@ public Optional<Path> showFileOpenFromArchiveDialog(Path archivePath) throws IOE
throw new IOException("Could not instantiate ZIP-archive reader.", exc);
}
}

@Override
public void showCustomDialog(BaseDialog<?> aboutDialogView) {
aboutDialogView.initOwner(mainWindow);
aboutDialogView.show();
}
}
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/JabRefFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -1089,7 +1089,7 @@ private void addImportedEntries(final LibraryTab panel, final ParserResult parse
cleanup.doPostCleanup(parserResult.getDatabase().getEntries());
ImportEntriesDialog dialog = new ImportEntriesDialog(panel.getBibDatabaseContext(), task);
dialog.setTitle(Localization.lang("Import"));
dialog.showAndWait();
dialogService.showCustomDialogAndWait(dialog);
}

public FileHistoryMenu getFileHistory() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package org.jabref.gui.auximport;

import org.jabref.gui.DialogService;
import org.jabref.gui.JabRefFrame;
import org.jabref.gui.StateManager;
import org.jabref.gui.actions.SimpleCommand;

import com.airhacks.afterburner.injection.Injector;

import static org.jabref.gui.actions.ActionHelper.needsDatabase;

/**
Expand All @@ -21,7 +24,7 @@ public NewSubLibraryAction(JabRefFrame jabRefFrame, StateManager stateManager) {

@Override
public void execute() {
FromAuxDialog dialog = new FromAuxDialog(jabRefFrame);
dialog.showAndWait();
DialogService dialogService = Injector.instantiateModelOrService(DialogService.class);
dialogService.showCustomDialogAndWait(new FromAuxDialog(jabRefFrame));
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
package org.jabref.gui.bibtexextractor;

import org.jabref.gui.DialogService;
import org.jabref.gui.StateManager;
import org.jabref.gui.actions.SimpleCommand;

import com.airhacks.afterburner.injection.Injector;

import static org.jabref.gui.actions.ActionHelper.needsDatabase;

public class ExtractBibtexAction extends SimpleCommand {
Expand All @@ -13,7 +16,7 @@ public ExtractBibtexAction(StateManager stateManager) {

@Override
public void execute() {
ExtractBibtexDialog dlg = new ExtractBibtexDialog();
dlg.showAndWait();
DialogService dialogService = Injector.instantiateModelOrService(DialogService.class);
dialogService.showCustomDialogAndWait(new ExtractBibtexDialog());
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package org.jabref.gui.citationkeypattern;

import org.jabref.gui.DialogService;
import org.jabref.gui.JabRefFrame;
import org.jabref.gui.StateManager;
import org.jabref.gui.actions.SimpleCommand;

import com.airhacks.afterburner.injection.Injector;

import static org.jabref.gui.actions.ActionHelper.needsDatabase;

public class CitationKeyPatternAction extends SimpleCommand {
Expand All @@ -18,6 +21,7 @@ public CitationKeyPatternAction(JabRefFrame frame, StateManager stateManager) {

@Override
public void execute() {
new CitationKeyPatternDialog(frame.getCurrentLibraryTab()).showAndWait();
DialogService dialogService = Injector.instantiateModelOrService(DialogService.class);
dialogService.showCustomDialogAndWait(new CitationKeyPatternDialog(frame.getCurrentLibraryTab()));
}
}
7 changes: 5 additions & 2 deletions src/main/java/org/jabref/gui/cleanup/CleanupAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,13 @@ public void execute() {
isCanceled = false;
modifiedEntriesCount = 0;

Optional<CleanupPreset> chosenPreset = new CleanupDialog(
CleanupDialog cleanupDialog = new CleanupDialog(
stateManager.getActiveDatabase().get(),
preferences.getCleanupPreset(),
preferences.getFilePreferences()).showAndWait();
preferences.getFilePreferences()
);

Optional<CleanupPreset> chosenPreset = dialogService.showCustomDialogAndWait(cleanupDialog);

chosenPreset.ifPresent(preset -> {
if (preset.isRenamePDFActive() && preferences.getAutoLinkPreferences().shouldAskAutoNamingPdfs()) {
Expand Down
7 changes: 4 additions & 3 deletions src/main/java/org/jabref/gui/collab/DatabaseChangePane.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@

import javafx.scene.Node;

import org.jabref.gui.DialogService;
import org.jabref.gui.icon.IconTheme;
import org.jabref.logic.l10n.Localization;
import org.jabref.model.database.BibDatabaseContext;

import com.airhacks.afterburner.injection.Injector;
import org.controlsfx.control.NotificationPane;
import org.controlsfx.control.action.Action;

Expand All @@ -33,9 +35,8 @@ private void onDatabaseChanged(List<DatabaseChangeViewModel> changes) {
this.hide();
}),
new Action(Localization.lang("Review changes"), event -> {
ChangeDisplayDialog changeDialog = new ChangeDisplayDialog(database, changes);
changeDialog.showAndWait();

DialogService dialogService = Injector.instantiateModelOrService(DialogService.class);
dialogService.showCustomDialogAndWait(new ChangeDisplayDialog(database, changes));
this.hide();
}));
this.show();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package org.jabref.gui.contentselector;

import org.jabref.gui.DialogService;
import org.jabref.gui.JabRefFrame;
import org.jabref.gui.LibraryTab;
import org.jabref.gui.StateManager;
import org.jabref.gui.actions.SimpleCommand;

import com.airhacks.afterburner.injection.Injector;

import static org.jabref.gui.actions.ActionHelper.needsDatabase;

public class ManageContentSelectorAction extends SimpleCommand {
Expand All @@ -19,7 +22,8 @@ public ManageContentSelectorAction(JabRefFrame jabRefFrame, StateManager stateMa

@Override
public void execute() {
DialogService dialogService = Injector.instantiateModelOrService(DialogService.class);
LibraryTab libraryTab = jabRefFrame.getCurrentLibraryTab();
new ContentSelectorDialogView(libraryTab).showAndWait();
dialogService.showCustomDialogAndWait(new ContentSelectorDialogView(libraryTab));
}
}
3 changes: 1 addition & 2 deletions src/main/java/org/jabref/gui/copyfiles/CopyFilesAction.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ private void showDialog(List<CopyFilesResultItemViewModel> data) {
dialogService.showInformationDialogAndWait(Localization.lang("Copy linked files to folder..."), Localization.lang("No linked files found for export."));
return;
}
CopyFilesDialogView dialog = new CopyFilesDialogView(new CopyFilesResultListDependency(data));
dialog.showAndWait();
dialogService.showCustomDialogAndWait(new CopyFilesDialogView(new CopyFilesResultListDependency(data)));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
package org.jabref.gui.customentrytypes;

import org.jabref.gui.DialogService;
import org.jabref.gui.StateManager;
import org.jabref.gui.actions.SimpleCommand;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntryTypesManager;

import com.airhacks.afterburner.injection.Injector;

import static org.jabref.gui.actions.ActionHelper.needsDatabase;

public class CustomizeEntryAction extends SimpleCommand {
Expand All @@ -21,7 +24,7 @@ public CustomizeEntryAction(StateManager stateManager, BibEntryTypesManager entr
@Override
public void execute() {
BibDatabaseContext database = stateManager.getActiveDatabase().orElseThrow(() -> new NullPointerException("Database null"));
CustomizeEntryTypeDialogView dialog = new CustomizeEntryTypeDialogView(database, entryTypesManager);
dialog.showAndWait();
DialogService dialogService = Injector.instantiateModelOrService(DialogService.class);
dialogService.showCustomDialogAndWait(new CustomizeEntryTypeDialogView(database, entryTypesManager));
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
package org.jabref.gui.customizefields;

import org.jabref.gui.DialogService;
import org.jabref.gui.actions.SimpleCommand;

import com.airhacks.afterburner.injection.Injector;

public class SetupGeneralFieldsAction extends SimpleCommand {

@Override
public void execute() {
new CustomizeGeneralFieldsDialogView().showAndWait();
DialogService dialogService = Injector.instantiateModelOrService(DialogService.class);
dialogService.showCustomDialogAndWait(new CustomizeGeneralFieldsDialogView());
}
}
Loading