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

removed default constructor of FXDialogService #4847

Merged
merged 4 commits into from
Apr 5, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
4 changes: 1 addition & 3 deletions src/main/java/org/jabref/gui/DefaultInjector.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@ public class DefaultInjector implements PresenterFactory {
* Dependencies without default constructor are constructed by hand.
*/
private static Object createDependency(Class<?> clazz) {
if (clazz == DialogService.class) {
return new FXDialogService();
} else if (clazz == TaskExecutor.class) {
if (clazz == TaskExecutor.class) {
Copy link
Member

Choose a reason for hiding this comment

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

Please don't remove the injection. Instead of a new FXDialogService you can here return JabRefGUI.getMainFrame().getDialogService().

Copy link
Member

Choose a reason for hiding this comment

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

Yep, please don't remove injection. This is very handy and makes it otherwise very complicated

Copy link
Contributor Author

@r0light r0light Apr 3, 2019

Choose a reason for hiding this comment

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

Ok, second try :) I reverted the removal of the injection. But it was not possible to simply call JabRefGUI.getMainFrame().getDialogService() inside DefaultInjector, because at the first time it is needed, JabRefFrame is not ready yet. In fact, from what i concluded, the injection is used somwhere inside the init-method of JabRefFrame which was called inside its constructor.
Therefore, i now splitted the constructor of JabRefFrame and the init-method, so that in JabRefGUI the object ist first created and then init is called. I'm not so happy with setting init public, but found no other way.. I also tried to use a factory method, but the constructed JabRefFrame-object is needed in JabRefGUI, so that JabRefGUI.getJabRefFrame().getDialogService()works.
Finally, i also removed the second DialogService in JabRefGUI, so that there is only a single DialogService.

Copy link
Member

Choose a reason for hiding this comment

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

And you need the JabRefFrame as container for the JFXSnackbar? So that was the initial reason for the sitting of the notify method in BasePanel.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, well the reason for the output method in JabRefFrame. So i have to admit, i broke (some of) the notifications with #4815, but it was also confusing with the different methods and the deprecated method in JabRefFrame. So i hope, the code is a little better now.

return Globals.TASK_EXECUTOR;
} else if (clazz == PreferencesService.class) {
return Globals.prefs;
Expand Down
8 changes: 0 additions & 8 deletions src/main/java/org/jabref/gui/FXDialogService.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,6 @@ public class FXDialogService implements DialogService {
private final Window mainWindow;
private final JFXSnackbar statusLine;

/**
* @deprecated try not to initialize a new dialog service but reuse the one constructed in {@link org.jabref.gui.JabRefFrame}.
*/
@Deprecated
public FXDialogService() {
this(null);
}

public FXDialogService(Window mainWindow) {
this.mainWindow = mainWindow;
this.statusLine = new JFXSnackbar();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import javafx.scene.Node;

import org.jabref.Globals;
import org.jabref.gui.FXDialogService;
import org.jabref.JabRefGUI;
import org.jabref.gui.PreviewPanel;
import org.jabref.gui.externalfiletype.ExternalFileTypes;
import org.jabref.gui.undo.NamedCompound;
Expand All @@ -29,7 +29,7 @@ public void makeChange(BibDatabaseContext database, NamedCompound undoEdit) {

@Override
public Node description() {
PreviewPanel previewPanel = new PreviewPanel(null, new BibDatabaseContext(), Globals.getKeyPrefs(), Globals.prefs.getPreviewPreferences(), new FXDialogService(), ExternalFileTypes.getInstance());
PreviewPanel previewPanel = new PreviewPanel(null, new BibDatabaseContext(), Globals.getKeyPrefs(), Globals.prefs.getPreviewPreferences(), JabRefGUI.getMainFrame().getDialogService(), ExternalFileTypes.getInstance());
previewPanel.setEntry(diskEntry);
return previewPanel;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import javafx.scene.Node;

import org.jabref.Globals;
import org.jabref.gui.FXDialogService;
import org.jabref.JabRefGUI;
import org.jabref.gui.PreviewPanel;
import org.jabref.gui.externalfiletype.ExternalFileTypes;
import org.jabref.gui.undo.NamedCompound;
Expand Down Expand Up @@ -46,7 +46,7 @@ public void makeChange(BibDatabaseContext database, NamedCompound undoEdit) {

@Override
public Node description() {
PreviewPanel previewPanel = new PreviewPanel(null, new BibDatabaseContext(), Globals.getKeyPrefs(), Globals.prefs.getPreviewPreferences(), new FXDialogService(), ExternalFileTypes.getInstance());
PreviewPanel previewPanel = new PreviewPanel(null, new BibDatabaseContext(), Globals.getKeyPrefs(), Globals.prefs.getPreviewPreferences(), JabRefGUI.getMainFrame().getDialogService(), ExternalFileTypes.getInstance());
previewPanel.setEntry(memEntry);
return previewPanel;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import javafx.scene.control.ButtonType;
import javafx.scene.control.TextArea;

import org.jabref.gui.DialogService;
import org.jabref.JabRefGUI;
import org.jabref.gui.help.HelpAction;
import org.jabref.gui.util.BaseDialog;
import org.jabref.gui.util.ControlHelper;
Expand All @@ -23,7 +23,6 @@ public class CustomizeGeneralFieldsDialogView extends BaseDialog<Void> {
@FXML private ButtonType okButton;
@FXML private TextArea fieldsTextArea;

@Inject private DialogService dialogService;
@Inject private PreferencesService preferences;
private CustomizeGeneralFieldsDialogViewModel viewModel;

Expand All @@ -44,7 +43,7 @@ public CustomizeGeneralFieldsDialogView() {

@FXML
private void initialize() {
viewModel = new CustomizeGeneralFieldsDialogViewModel(dialogService, preferences);
viewModel = new CustomizeGeneralFieldsDialogViewModel(JabRefGUI.getMainFrame().getDialogService(), preferences);
fieldsTextArea.textProperty().bindBidirectional(viewModel.fieldsTextProperty());

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
import javafx.stage.Modality;
import javafx.util.Callback;

import org.jabref.JabRefGUI;
import org.jabref.gui.ClipBoardManager;
import org.jabref.gui.DialogService;
import org.jabref.gui.icon.IconTheme;
import org.jabref.gui.keyboard.KeyBinding;
import org.jabref.gui.keyboard.KeyBindingRepository;
Expand All @@ -40,7 +40,6 @@ public class ErrorConsoleView extends BaseDialog<Void> {
@FXML private ListView<LogEventViewModel> messagesListView;
@FXML private Label descriptionLabel;

@Inject private DialogService dialogService;
@Inject private ClipBoardManager clipBoardManager;
@Inject private BuildInfo buildInfo;
@Inject private KeyBindingRepository keyBindingRepository;
Expand All @@ -60,7 +59,7 @@ public ErrorConsoleView() {

@FXML
private void initialize() {
viewModel = new ErrorConsoleViewModel(dialogService, clipBoardManager, buildInfo);
viewModel = new ErrorConsoleViewModel(JabRefGUI.getMainFrame().getDialogService(), clipBoardManager, buildInfo);
messagesListView.setCellFactory(createCellFactory());
messagesListView.itemsProperty().bind(viewModel.allMessagesDataProperty());
messagesListView.scrollTo(viewModel.allMessagesDataProperty().getSize() - 1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ public class CreateModifyExporterDialogView extends BaseDialog<ExporterViewModel
@FXML private TextField fileName;
@FXML private TextField extension;
@FXML private ButtonType saveExporter;
@Inject private DialogService dialogService;
@Inject private PreferencesService preferences;
private CreateModifyExporterDialogViewModel viewModel;
private DialogService dialogService;

public CreateModifyExporterDialogView(ExporterViewModel exporter, DialogService dialogService,
PreferencesService preferences, JournalAbbreviationLoader loader) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import javafx.scene.control.TableColumn;
import javafx.scene.control.TableView;

import org.jabref.gui.DialogService;
import org.jabref.JabRefGUI;
import org.jabref.gui.util.BaseDialog;
import org.jabref.gui.util.ControlHelper;
import org.jabref.logic.journals.JournalAbbreviationLoader;
Expand All @@ -29,7 +29,6 @@ public class ExportCustomizationDialogView extends BaseDialog<Void> {
@FXML private TableColumn<ExporterViewModel, String> layoutColumn;
@FXML private TableColumn<ExporterViewModel, String> extensionColumn;

@Inject private DialogService dialogService;
@Inject private PreferencesService preferences;
@Inject private JournalAbbreviationLoader loader;
private ExportCustomizationDialogViewModel viewModel;
Expand All @@ -52,7 +51,7 @@ public ExportCustomizationDialogView() {

@FXML
private void initialize() {
viewModel = new ExportCustomizationDialogViewModel(preferences, dialogService, loader);
viewModel = new ExportCustomizationDialogViewModel(preferences, JabRefGUI.getMainFrame().getDialogService(), loader);
exporterTable.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
exporterTable.itemsProperty().bind(viewModel.exportersProperty());
EasyBind.listBind(viewModel.selectedExportersProperty(), exporterTable.getSelectionModel().getSelectedItems());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
import javax.swing.event.DocumentListener;

import org.jabref.Globals;
import org.jabref.JabRefGUI;
import org.jabref.gui.DialogService;
import org.jabref.gui.FXDialogService;
import org.jabref.gui.icon.IconTheme;
import org.jabref.gui.util.DefaultTaskExecutor;
import org.jabref.gui.util.FileDialogConfiguration;
Expand Down Expand Up @@ -62,7 +62,7 @@ public class ExternalFileTypeEntryEditor {

FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder()
.withInitialDirectory(Paths.get(appDir)).build();
DialogService ds = new FXDialogService();
DialogService ds = JabRefGUI.getMainFrame().getDialogService();

Optional<Path> path = DefaultTaskExecutor
.runInJavaFXThread(() -> ds.showFileOpenDialog(fileDialogConfiguration));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import javafx.scene.control.ComboBox;
import javafx.scene.control.TextField;

import org.jabref.gui.DialogService;
import org.jabref.JabRefGUI;
import org.jabref.gui.StateManager;
import org.jabref.gui.externalfiletype.ExternalFileType;
import org.jabref.gui.externalfiletype.ExternalFileTypes;
Expand All @@ -24,7 +24,6 @@ public class LinkedFileEditDialogView extends BaseDialog<LinkedFile> {
@FXML private TextField description;
@FXML private ComboBox<ExternalFileType> fileType;

@Inject private DialogService dialogService;
@Inject private StateManager stateManager;

@Inject private PreferencesService preferences;
Expand Down Expand Up @@ -56,7 +55,7 @@ public LinkedFileEditDialogView(LinkedFile linkedFile) {
@FXML
private void initialize() {

viewModel = new LinkedFilesEditDialogViewModel(linkedFile, stateManager.getActiveDatabase().get(), dialogService, preferences, externalFileTypes);
viewModel = new LinkedFilesEditDialogViewModel(linkedFile, stateManager.getActiveDatabase().get(), JabRefGUI.getMainFrame().getDialogService(), preferences, externalFileTypes);
fileType.itemsProperty().bindBidirectional(viewModel.externalFileTypeProperty());
description.textProperty().bindBidirectional(viewModel.descriptionProperty());
link.textProperty().bindBidirectional(viewModel.linkProperty());
Expand Down
4 changes: 1 addition & 3 deletions src/main/java/org/jabref/gui/groups/GroupTreeView.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import javafx.scene.layout.StackPane;
import javafx.scene.text.Text;

import org.jabref.gui.DialogService;
import org.jabref.gui.DragAndDropDataFormats;
import org.jabref.gui.GUIGlobals;
import org.jabref.gui.StateManager;
Expand Down Expand Up @@ -66,7 +65,6 @@ public class GroupTreeView {
@FXML private CustomTextField searchField;

@Inject private StateManager stateManager;
@Inject private DialogService dialogService;
@Inject private TaskExecutor taskExecutor;
private GroupTreeViewModel viewModel;
private CustomLocalDragboard localDragboard;
Expand All @@ -82,7 +80,7 @@ private static void removePseudoClasses(TreeTableRow<GroupNodeViewModel> row, Ps
@FXML
public void initialize() {
this.localDragboard = GUIGlobals.localDragboard;
viewModel = new GroupTreeViewModel(stateManager, dialogService, taskExecutor, localDragboard);
viewModel = new GroupTreeViewModel(stateManager, taskExecutor, localDragboard);

// Set-up groups tree
groupTree.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
Expand Down
33 changes: 16 additions & 17 deletions src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;

import org.jabref.JabRefGUI;
import org.jabref.gui.AbstractViewModel;
import org.jabref.gui.DialogService;
import org.jabref.gui.StateManager;
import org.jabref.gui.util.CustomLocalDragboard;
import org.jabref.gui.util.TaskExecutor;
Expand All @@ -37,7 +37,7 @@ public class GroupTreeViewModel extends AbstractViewModel {
private final ObjectProperty<GroupNodeViewModel> rootGroup = new SimpleObjectProperty<>();
private final ListProperty<GroupNodeViewModel> selectedGroups = new SimpleListProperty<>(FXCollections.observableArrayList());
private final StateManager stateManager;
private final DialogService dialogService;

private final TaskExecutor taskExecutor;
private final CustomLocalDragboard localDragboard;
private final ObjectProperty<Predicate<GroupNodeViewModel>> filterPredicate = new SimpleObjectProperty<>();
Expand All @@ -47,9 +47,8 @@ public class GroupTreeViewModel extends AbstractViewModel {
.compareToIgnoreCase(v2.getName());
private Optional<BibDatabaseContext> currentDatabase;

public GroupTreeViewModel(StateManager stateManager, DialogService dialogService, TaskExecutor taskExecutor, CustomLocalDragboard localDragboard) {
public GroupTreeViewModel(StateManager stateManager, TaskExecutor taskExecutor, CustomLocalDragboard localDragboard) {
this.stateManager = Objects.requireNonNull(stateManager);
this.dialogService = Objects.requireNonNull(dialogService);
this.taskExecutor = Objects.requireNonNull(taskExecutor);
this.localDragboard = Objects.requireNonNull(localDragboard);
// Register listener
Expand Down Expand Up @@ -109,7 +108,7 @@ public void addNewGroupToRoot() {
if (currentDatabase.isPresent()) {
addNewSubgroup(rootGroup.get());
} else {
dialogService.showWarningDialogAndWait(Localization.lang("Cannot create group"), Localization.lang("Cannot create group. Please create a library first."));
JabRefGUI.getMainFrame().getDialogService().showWarningDialogAndWait(Localization.lang("Cannot create group"), Localization.lang("Cannot create group. Please create a library first."));
}
}

Expand Down Expand Up @@ -141,7 +140,7 @@ private void onActiveDatabaseChanged(Optional<BibDatabaseContext> newDatabase) {
* Opens "New Group Dialog" and add the resulting group to the specified group
*/
public void addNewSubgroup(GroupNodeViewModel parent) {
Optional<AbstractGroup> newGroup = dialogService.showCustomDialogAndWait(new GroupDialog(dialogService));
Optional<AbstractGroup> newGroup = JabRefGUI.getMainFrame().getDialogService().showCustomDialogAndWait(new GroupDialog(JabRefGUI.getMainFrame().getDialogService()));
newGroup.ifPresent(group -> {
parent.addSubgroup(group);

Expand All @@ -152,7 +151,7 @@ public void addNewSubgroup(GroupNodeViewModel parent) {
// TODO: Expand parent to make new group visible
//parent.expand();

dialogService.notify(Localization.lang("Added group \"%0\".", group.getName()));
JabRefGUI.getMainFrame().getDialogService().notify(Localization.lang("Added group \"%0\".", group.getName()));
writeGroupChangesToMetaData();
});
}
Expand All @@ -165,11 +164,11 @@ private void writeGroupChangesToMetaData() {
* Opens "Edit Group Dialog" and changes the given group to the edited one.
*/
public void editGroup(GroupNodeViewModel oldGroup) {
Optional<AbstractGroup> newGroup = dialogService
.showCustomDialogAndWait(new GroupDialog(dialogService, oldGroup.getGroupNode().getGroup()));
Optional<AbstractGroup> newGroup = JabRefGUI.getMainFrame().getDialogService()
.showCustomDialogAndWait(new GroupDialog(JabRefGUI.getMainFrame().getDialogService(), oldGroup.getGroupNode().getGroup()));
newGroup.ifPresent(group -> {
// TODO: Keep assignments
boolean keepPreviousAssignments = dialogService.showConfirmationDialogAndWait(
boolean keepPreviousAssignments = JabRefGUI.getMainFrame().getDialogService().showConfirmationDialogAndWait(
Localization.lang("Change of Grouping Method"),
Localization.lang("Assign the original group's entries to this group?"));
// WarnAssignmentSideEffects.warnAssignmentSideEffects(newGroup, panel.frame());
Expand Down Expand Up @@ -199,7 +198,7 @@ public void editGroup(GroupNodeViewModel oldGroup) {
// undoAddPreviousEntries = UndoableChangeEntriesOfGroup.getUndoableEdit(null, addChange);
//}

dialogService.notify(Localization.lang("Modified group \"%0\".", group.getName()));
JabRefGUI.getMainFrame().getDialogService().notify(Localization.lang("Modified group \"%0\".", group.getName()));
writeGroupChangesToMetaData();

// This is ugly but we have no proper update mechanism in place to propagate the changes, so redraw everything
Expand All @@ -208,21 +207,21 @@ public void editGroup(GroupNodeViewModel oldGroup) {
}

public void removeSubgroups(GroupNodeViewModel group) {
boolean confirmation = dialogService.showConfirmationDialogAndWait(
boolean confirmation = JabRefGUI.getMainFrame().getDialogService().showConfirmationDialogAndWait(
Localization.lang("Remove subgroups"),
Localization.lang("Remove all subgroups of \"%0\"?", group.getDisplayName()));
if (confirmation) {
/// TODO: Add undo
//final UndoableModifySubtree undo = new UndoableModifySubtree(getGroupTreeRoot(), node, "Remove subgroups");
//panel.getUndoManager().addEdit(undo);
group.getGroupNode().removeAllChildren();
dialogService.notify(Localization.lang("Removed all subgroups of group \"%0\".", group.getDisplayName()));
JabRefGUI.getMainFrame().getDialogService().notify(Localization.lang("Removed all subgroups of group \"%0\".", group.getDisplayName()));
writeGroupChangesToMetaData();
}
}

public void removeGroupKeepSubgroups(GroupNodeViewModel group) {
boolean confirmation = dialogService.showConfirmationDialogAndWait(
boolean confirmation = JabRefGUI.getMainFrame().getDialogService().showConfirmationDialogAndWait(
Localization.lang("Remove group"),
Localization.lang("Remove group \"%0\"?", group.getDisplayName()));

Expand All @@ -235,7 +234,7 @@ public void removeGroupKeepSubgroups(GroupNodeViewModel group) {
.ifPresent(parent -> groupNode.moveAllChildrenTo(parent, parent.getIndexOfChild(groupNode).get()));
groupNode.removeFromParent();

dialogService.notify(Localization.lang("Removed group \"%0\".", group.getDisplayName()));
JabRefGUI.getMainFrame().getDialogService().notify(Localization.lang("Removed group \"%0\".", group.getDisplayName()));
writeGroupChangesToMetaData();
}
}
Expand All @@ -244,7 +243,7 @@ public void removeGroupKeepSubgroups(GroupNodeViewModel group) {
* Removes the specified group and its subgroups (after asking for confirmation).
*/
public void removeGroupAndSubgroups(GroupNodeViewModel group) {
boolean confirmed = dialogService.showConfirmationDialogAndWait(
boolean confirmed = JabRefGUI.getMainFrame().getDialogService().showConfirmationDialogAndWait(
Localization.lang("Remove group and subgroups"),
Localization.lang("Remove group \"%0\" and its subgroups?", group.getDisplayName()),
Localization.lang("Remove"));
Expand All @@ -257,7 +256,7 @@ public void removeGroupAndSubgroups(GroupNodeViewModel group) {

group.getGroupNode().removeFromParent();

dialogService.notify(Localization.lang("Removed group \"%0\" and its subgroups.", group.getDisplayName()));
JabRefGUI.getMainFrame().getDialogService().notify(Localization.lang("Removed group \"%0\" and its subgroups.", group.getDisplayName()));
writeGroupChangesToMetaData();
}
}
Expand Down
5 changes: 2 additions & 3 deletions src/main/java/org/jabref/gui/help/AboutDialogView.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
import javafx.scene.control.ButtonType;
import javafx.scene.control.TextArea;

import org.jabref.JabRefGUI;
import org.jabref.gui.ClipBoardManager;
import org.jabref.gui.DialogService;
import org.jabref.gui.util.BaseDialog;
import org.jabref.gui.util.ControlHelper;
import org.jabref.logic.l10n.Localization;
Expand All @@ -20,7 +20,6 @@ public class AboutDialogView extends BaseDialog<Void> {
@FXML private ButtonType copyVersionButton;
@FXML private TextArea textAreaVersions;

@Inject private DialogService dialogService;
@Inject private ClipBoardManager clipBoardManager;
@Inject private BuildInfo buildInfo;

Expand All @@ -42,7 +41,7 @@ public AboutDialogViewModel getViewModel() {

@FXML
private void initialize() {
viewModel = new AboutDialogViewModel(dialogService, clipBoardManager, buildInfo);
viewModel = new AboutDialogViewModel(JabRefGUI.getMainFrame().getDialogService(), clipBoardManager, buildInfo);

textAreaVersions.setText(viewModel.getVersionInfo());
}
Expand Down
Loading