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

[WIP] Convert Exporter Customization Dialog to javafx #4394

Merged
merged 78 commits into from
Dec 9, 2018
Merged
Show file tree
Hide file tree
Changes from 76 commits
Commits
Show all changes
78 commits
Select commit Hold shift + click to select a range
f120b41
Exporter to JavaFX commit 1
abepolk Oct 19, 2018
12e3e7d
Work on Export Customization Dialog VM
abepolk Oct 21, 2018
ec364ad
Work on VM
abepolk Oct 21, 2018
f5c89fa
Combine methods in CustomExporterList and move to preferences
abepolk Oct 25, 2018
6773b22
work on VM
abepolk Oct 25, 2018
1f318f0
Add methods to VM
abepolk Oct 25, 2018
1a0b760
Fix typos
abepolk Oct 25, 2018
5c65b57
started work on exporter subdialog
abepolk Oct 26, 2018
2de4124
Continue work on exporter subdialog
abepolk Oct 26, 2018
a869aa2
Implement save exporter from add/modify exporter dialog
abepolk Oct 27, 2018
9ae84c0
Add two comments
abepolk Oct 27, 2018
e992c5d
More work on VMs and prefs
abepolk Oct 29, 2018
3a4baa3
Fix browse method in export customization subdialog
abepolk Oct 29, 2018
f63d43c
Remove int reference in VM
abepolk Oct 29, 2018
1f094d6
Create FXML for main exporter customization dialog
abepolk Oct 29, 2018
83b7dac
Work on View, not complete
abepolk Nov 2, 2018
99d7041
Work on using EasyBind
abepolk Nov 2, 2018
0479893
fix some binding issues add stubs
Siedlerchr Nov 2, 2018
547898c
More work on Export Customization Dialog
abepolk Nov 3, 2018
e2ae2c9
FXML changes among other things
abepolk Nov 3, 2018
3be5672
Work on View for subdialog
abepolk Nov 6, 2018
debd2bc
Changes to View and logic
abepolk Nov 6, 2018
7141a46
Deal with stubs, convert some EVMs to Optionals
abepolk Nov 8, 2018
889489b
Change Optional in subdialog
abepolk Nov 10, 2018
1fb234d
Add FXML and View
abepolk Nov 10, 2018
e17d0da
Remove typos and syntax error
abepolk Nov 11, 2018
73e683a
Move various code to logic
abepolk Nov 12, 2018
19720cc
Fix modify exporter
abepolk Nov 12, 2018
4e8c8df
Fix remove and close bugs
abepolk Nov 12, 2018
3618f83
Merge branch 'master' into exporter_to_javafx
abepolk Nov 12, 2018
51a726e
Fix customExports references
abepolk Nov 12, 2018
1821da3
Merge remote-tracking branch 'upstream/master' into exporter_to_javafx
abepolk Nov 12, 2018
d339f78
Fix get custom prefs
abepolk Nov 12, 2018
eea34ed
Remove prefs.customExports object from JabRef
abepolk Nov 12, 2018
d36eec2
Small FXML changes
abepolk Nov 12, 2018
294a367
Remove unused old Swing dialogs
abepolk Nov 12, 2018
4269ca0
Fix indentation
abepolk Nov 12, 2018
25db576
Make clear comments and Javadoc
abepolk Nov 12, 2018
3b54bc8
Remove CustomExportList
abepolk Nov 12, 2018
84f293e
Use new TemplateExporter constructor
abepolk Nov 12, 2018
00bac82
Fix spacing and a var name
abepolk Nov 12, 2018
b26d592
Add constant vars to prefs
abepolk Nov 12, 2018
b681ae8
Remove comments
abepolk Nov 12, 2018
be47855
Change log message
abepolk Nov 13, 2018
9aa0887
Fix tests
abepolk Nov 13, 2018
d7277e8
Add spacing between imports
abepolk Nov 13, 2018
7087cfb
Update localization
abepolk Nov 13, 2018
1daf7c4
Replace Optional parameter with null
abepolk Nov 24, 2018
83d490a
Replace Optional return with null
abepolk Nov 24, 2018
17d20cc
Remove AnchorPane
abepolk Nov 26, 2018
40eed27
Localize FXML labels
abepolk Nov 26, 2018
379f201
Add subdialog injections
abepolk Nov 26, 2018
4679150
Set browse action in FXML
abepolk Nov 26, 2018
0053abf
Add to l10n
abepolk Nov 26, 2018
9442025
Add cancel button to subdialog
abepolk Nov 26, 2018
de3bf63
Correction to VM try-catch
abepolk Nov 26, 2018
19d8c62
Inline setTextFields
abepolk Nov 26, 2018
df42676
Remove SortedList TODO
abepolk Nov 26, 2018
f31bc51
Inline closeDialog
abepolk Nov 26, 2018
d95a1aa
loadExporters to private
abepolk Nov 26, 2018
80ffa33
forEach to Stream and other fixes
abepolk Nov 26, 2018
5f56c25
inline init
abepolk Nov 26, 2018
f1d920e
Fixes to ExporterViewModel
abepolk Nov 26, 2018
f22e97a
Remove Optional
abepolk Nov 26, 2018
252ecbb
Add constants to TemplateExporter
abepolk Nov 27, 2018
ccdbf1f
Simplify getExtensions without dot
abepolk Nov 29, 2018
aba31a6
Merge branch 'master' into exporter_to_javafx
abepolk Nov 29, 2018
8cd7ff3
Change try-catch to empty list check
abepolk Nov 29, 2018
3e09a7b
Remove unnecessary method
abepolk Nov 29, 2018
3051022
Fix property getters
abepolk Nov 29, 2018
0ef3061
Fix property getters again
abepolk Nov 29, 2018
7bf6dd6
Fix typo
abepolk Nov 29, 2018
93838de
Update references to customFormats
abepolk Nov 29, 2018
a79d037
Add import
abepolk Nov 29, 2018
70479cb
Travis CI fixes
abepolk Nov 30, 2018
bf22adf
Further Travis CI fixes
abepolk Dec 5, 2018
c2b3489
Remove extra comments
abepolk Dec 8, 2018
a364e1d
Remove extra newlines
abepolk Dec 8, 2018
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/cli/ArgumentProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.prefs.BackingStoreException;

Expand Down Expand Up @@ -459,8 +458,7 @@ private void importPreferences() {
Globals.prefs.importPreferences(cli.getPreferencesImport());
EntryTypes.loadCustomEntryTypes(Globals.prefs.loadCustomEntryTypes(BibDatabaseMode.BIBTEX),
Globals.prefs.loadCustomEntryTypes(BibDatabaseMode.BIBLATEX));
Map<String, TemplateExporter> customExporters = Globals.prefs.customExports.getCustomExportFormats(Globals.prefs,
Globals.journalAbbreviationLoader);
List<TemplateExporter> customExporters = Globals.prefs.getCustomExportFormats(Globals.journalAbbreviationLoader);
LayoutFormatterPreferences layoutPreferences = Globals.prefs
.getLayoutFormatterPreferences(Globals.journalAbbreviationLoader);
SavePreferences savePreferences = Globals.prefs.loadForExportFromPreferences();
Expand Down
5 changes: 1 addition & 4 deletions src/main/java/org/jabref/gui/JabRefFrame.java
Original file line number Diff line number Diff line change
Expand Up @@ -454,9 +454,6 @@ private void tearDownJabRef(List<String> filenames) {
}

fileHistory.storeHistory();
prefs.customExports.store(Globals.prefs);
prefs.customImports.store();

prefs.flush();

// dispose all windows, even if they are not displayed anymore
Expand Down Expand Up @@ -928,7 +925,7 @@ private MenuBar createMenu() {

factory.createMenuItem(StandardActions.SETUP_GENERAL_FIELDS, new SetupGeneralFieldsAction()),
factory.createMenuItem(StandardActions.MANAGE_CUSTOM_IMPORTS, new ManageCustomImportsAction(this)),
factory.createMenuItem(StandardActions.MANAGE_CUSTOM_EXPORTS, new ManageCustomExportsAction(this)),
factory.createMenuItem(StandardActions.MANAGE_CUSTOM_EXPORTS, new ManageCustomExportsAction()),
factory.createMenuItem(StandardActions.MANAGE_EXTERNAL_FILETYPES, new EditExternalFileTypesAction()),
factory.createMenuItem(StandardActions.MANAGE_JOURNALS, new ManageJournalsAction()),
factory.createMenuItem(StandardActions.CUSTOMIZE_KEYBINDING, new CustomizeKeyBindingAction()),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,12 @@
package org.jabref.gui.actions;

import org.jabref.gui.JabRefFrame;
import org.jabref.gui.exporter.ExportCustomizationDialog;
import org.jabref.gui.exporter.ExportCustomizationDialogView;

public class ManageCustomExportsAction extends SimpleCommand {

private final JabRefFrame jabRefFrame;

public ManageCustomExportsAction(JabRefFrame jabRefFrame) {
this.jabRefFrame = jabRefFrame;
}

@Override
public void execute() {
ExportCustomizationDialog ecd = new ExportCustomizationDialog(jabRefFrame);
ecd.setVisible(true);
new ExportCustomizationDialogView().show();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?xml version="1.0" encoding="UTF-8"?>

<?import javafx.geometry.Insets?>
<?import javafx.scene.control.Button?>
<?import javafx.scene.control.ButtonType?>
<?import javafx.scene.control.DialogPane?>
<?import javafx.scene.control.Label?>
<?import javafx.scene.control.TextField?>
<?import javafx.scene.layout.AnchorPane?>
<?import javafx.scene.layout.ColumnConstraints?>
<?import javafx.scene.layout.GridPane?>
<?import javafx.scene.layout.RowConstraints?>

<DialogPane maxHeight="-Infinity" maxWidth="-Infinity" minHeight="-Infinity" minWidth="-Infinity" prefHeight="300.0" prefWidth="480.0" xmlns="http://javafx.com/javafx/8.0.171" xmlns:fx="http://javafx.com/fxml/1" fx:controller="org.jabref.gui.exporter.CreateModifyExporterDialogView">
<content>
<AnchorPane minHeight="0.0" minWidth="0.0" prefHeight="200.0" prefWidth="320.0">
<children>
<GridPane prefWidth="480.0" scaleShape="false">
<columnConstraints>
<ColumnConstraints hgrow="SOMETIMES" minWidth="10.0" prefWidth="100.0" />
</columnConstraints>
<rowConstraints>
<RowConstraints minHeight="10.0" prefHeight="30.0" vgrow="SOMETIMES" />
<RowConstraints minHeight="10.0" prefHeight="30.0" vgrow="SOMETIMES" />
<RowConstraints minHeight="10.0" prefHeight="30.0" vgrow="SOMETIMES" />
<RowConstraints minHeight="10.0" prefHeight="30.0" vgrow="SOMETIMES" />
<RowConstraints minHeight="10.0" prefHeight="30.0" vgrow="SOMETIMES" />
<RowConstraints minHeight="10.0" prefHeight="30.0" vgrow="SOMETIMES" />
<RowConstraints minHeight="10.0" prefHeight="30.0" vgrow="SOMETIMES" />
</rowConstraints>
<children>
<TextField fx:id="name" GridPane.rowIndex="1" />
<Label text="%Main layout file:" GridPane.rowIndex="2" />
<TextField fx:id="fileName" GridPane.rowIndex="3" />
<TextField fx:id="extension" GridPane.rowIndex="6" />
<Label alignment="TOP_LEFT" text="%Export format name:">
<GridPane.margin>
<Insets />
</GridPane.margin>
</Label>
<Label text="%File extension:" GridPane.rowIndex="5" />
<Button fx:id="browseButton" text="%Browse" onAction="#browse" GridPane.rowIndex="4" />
</children>
<padding>
<Insets bottom="30.0" left="30.0" right="30.0" top="30.0" />
</padding>
</GridPane>
</children></AnchorPane>
</content>
<ButtonType fx:id="saveExporter" text="%Save exporter" />
Copy link
Member

Choose a reason for hiding this comment

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

There should probably be a Cancel button as well (if the user decides to abort his editing).

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This hasn't worked despite my adding<ButtonType fx:constant="CANCEL" /> to the FXML, and I'm not sure why. The button does not appear in the dialog.

<ButtonType fx:constant="CANCEL" />
</DialogPane>
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package org.jabref.gui.exporter;

import javax.inject.Inject;

import javafx.event.ActionEvent;
import javafx.fxml.FXML;
import javafx.scene.control.Button;
import javafx.scene.control.ButtonType;
import javafx.scene.control.TextField;

import org.jabref.gui.DialogService;
import org.jabref.gui.util.BaseDialog;
import org.jabref.logic.journals.JournalAbbreviationLoader;
import org.jabref.logic.l10n.Localization;
import org.jabref.preferences.PreferencesService;

import com.airhacks.afterburner.views.ViewLoader;

public class CreateModifyExporterDialogView extends BaseDialog<ExporterViewModel> {

//Browse must be a Button because ButtonTypes must be on the buttom, not next to the filename field
@FXML private Button browseButton;
@FXML private TextField name;
@FXML private TextField fileName;
@FXML private TextField extension;
@FXML private ButtonType saveExporter;

@Inject private DialogService dialogService;
@Inject private PreferencesService preferences;
@Inject private final JournalAbbreviationLoader loader;
private CreateModifyExporterDialogViewModel viewModel;

private final ExporterViewModel exporter;

public CreateModifyExporterDialogView(ExporterViewModel exporter, DialogService dialogService,
PreferencesService preferences, JournalAbbreviationLoader loader) { //should the latter three have been injected as in the main dialog rather than passed as a param?
Copy link
Member

Choose a reason for hiding this comment

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

Yes it is slightly better to use @Inject. In the end, it doesn't really matter since the injection will populate the variables with the same values but makes the dialog somewhat easier to use in the code since the caller does not need to worry about how to get theses services. So, @Inject is just a shorthand form of manual constructor injection.

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. I have also injected dialogService and preferences.

this.setTitle(Localization.lang("Customize Export Formats"));
this.exporter = exporter;
this.loader = loader;
this.dialogService = dialogService;
this.preferences = preferences;

ViewLoader.view(this)
.load()
.setAsDialogPane(this);

this.setResultConverter(button -> {
if (button == saveExporter) {
return viewModel.saveExporter();
} else {
return null;
}
});
}

@FXML
private void initialize() {
viewModel = new CreateModifyExporterDialogViewModel(exporter, dialogService, preferences, loader);
name.textProperty().bindBidirectional(viewModel.getName());
fileName.textProperty().bindBidirectional(viewModel.getLayoutFileName());
extension.textProperty().bindBidirectional(viewModel.getExtension());
}

@FXML
private void browse(ActionEvent event) {
viewModel.browse();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
package org.jabref.gui.exporter;

import java.nio.file.Path;
import java.nio.file.Paths;

import javafx.beans.property.SimpleStringProperty;
import javafx.beans.property.StringProperty;

import org.jabref.gui.AbstractViewModel;
import org.jabref.gui.DialogService;
import org.jabref.gui.util.FileDialogConfiguration;
import org.jabref.logic.exporter.SavePreferences;
import org.jabref.logic.exporter.TemplateExporter;
import org.jabref.logic.journals.JournalAbbreviationLoader;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.layout.LayoutFormatterPreferences;
import org.jabref.logic.util.StandardFileType;
import org.jabref.preferences.PreferencesService;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
*
* This view model can be used both for "add exporter" and "modify exporter" functionalities.
* It takes an optional exporter which is empty for "add exporter," and takes the selected exporter
* for "modify exporter." It returns an optional exporter which empty if an invalid or no exporter is
* created, and otherwise contains the exporter to be added or that is modified.
*
*/

public class CreateModifyExporterDialogViewModel extends AbstractViewModel {

private static final Logger LOGGER = LoggerFactory.getLogger(CreateModifyExporterDialogViewModel.class);

private final DialogService dialogService;
private final PreferencesService preferences;

private final StringProperty name = new SimpleStringProperty("");
private final StringProperty layoutFile = new SimpleStringProperty("");
private final StringProperty extension = new SimpleStringProperty("");

private final JournalAbbreviationLoader loader;


public CreateModifyExporterDialogViewModel(ExporterViewModel exporter, DialogService dialogService, PreferencesService preferences,
JournalAbbreviationLoader loader) {
this.dialogService = dialogService;
this.preferences = preferences;
this.loader = loader;

//Set text of each of the boxes
if (exporter != null) {
name.setValue(exporter.name().get());
layoutFile.setValue(exporter.layoutFileName().get());
extension.setValue(exporter.extension().get());
}
}

public ExporterViewModel saveExporter() {
Path layoutFileDir = Paths.get(layoutFile.get()).getParent();
if (layoutFileDir != null) {
String layoutFileDirString = layoutFileDir.toString();
preferences.setExportWorkingDirectory(layoutFileDirString);
}

// Check that there are no empty strings.
if (layoutFile.get().isEmpty() || name.get().isEmpty() || extension.get().isEmpty()
|| !layoutFile.get().endsWith(".layout")) {

LOGGER.info("One of the fields is empty or invalid!");
// Return empty exporter to the main exporter customization dialog
return null;
}

// Create a new exporter to be returned to ExportCustomizationDialogViewModel, which requested it
LayoutFormatterPreferences layoutPreferences = preferences.getLayoutFormatterPreferences(loader);
SavePreferences savePreferences = preferences.loadForExportFromPreferences();
TemplateExporter format = new TemplateExporter(name.get(), layoutFile.get(), extension.get(),
layoutPreferences, savePreferences);
format.setCustomExport(true);
return new ExporterViewModel(format);
}

public String getExportWorkingDirectory() { //i.e. layout dir
return preferences.getExportWorkingDirectory();
}

public void browse() {
FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder()
.addExtensionFilter(Localization.lang("Custom layout file"), StandardFileType.LAYOUT)
.withDefaultExtension(Localization.lang("Custom layout file"), StandardFileType.LAYOUT)
.withInitialDirectory(getExportWorkingDirectory()).build();
dialogService.showFileOpenDialog(fileDialogConfiguration).ifPresent(f -> layoutFile.set(f.toAbsolutePath().toString())); //implement setting the text
Copy link
Member

Choose a reason for hiding this comment

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

The call preferences.setExportWorkingDirectory(layoutFileDirString); (which currently is in saveExporter) should probably come here in the ifPresent call. In this way, the correct directory is selected when the user clicks twice on "Browse".

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 put it in both, since saving the exporter might also indicate where the working directory should be. However, now it won't set the directory as before. Since this fix isn't working I am pushing this into a separate branch exporter_to_javafx_setwd (in my fork, but not in this pull request). I will try to get to the other changes you and Christoph mentioned later.

Copy link
Member

Choose a reason for hiding this comment

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

The call preferences.setExportWorkingDirectory(layoutFileDirString); (which currently is in saveExporter) should probably come here in the ifPresent call. In this way, the correct directory is selected when the user clicks twice on "Browse".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now it's called in as part of fileDialogConfiguration in the line above. How would I make that change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See above.

}

public StringProperty getName() {
return name;
}

public StringProperty getLayoutFileName() {
return layoutFile;
}

public StringProperty getExtension() {
return extension;
}

}
Loading