javafx replacement for file dialog

[Siedlerchr]

Fixes #2970

The open functionality has been removed -> Directly possible in the entry editor
Contextmenu on maintable ->attach file now opens a file chooser dialog and afterwards this dialog for editing, e.g. adding a description

Only resizable to max screen size: (on my 24" with 1920x1200)

• Change in CHANGELOG.md described
• Tests created for changes
• Screenshots added (for bigger UI changes)
• Manually tested changed features in running JabRef
• Check documentation status (Issue created for outdated help page at help.jabref.org?)
• If you changed the localization: Did you run gradle localizationUpdate?

[Siedlerchr]

@tobiasdiez
I would like to move this method and some others to the view Model, but I don't know how/or if I should expose the selection model of the combobox to the view model.

[tobiasdiez]

You have a few options here:

1. Create a property in the view model and bind the selected item property to it, similar to the following code:
   

   jabref/src/main/java/org/jabref/gui/keyboard/KeyBindingsDialogController.java

   Lines 37 to 41 in cbd6844

   	viewModel.selectedKeyBindingProperty().bind(
   	EasyBind.monadic(keyBindingsTable.selectionModelProperty())
   	.flatMap(SelectionModel::selectedItemProperty)
   	.selectProperty(TreeItem::valueProperty)
   	);

   
   But I'm not sure if using a bidirectional binding works here out of the box, probably you need to use

   jabref/src/main/java/org/jabref/gui/util/BindingsHelper.java

   Line 75 in bee715d

   public static <A, B> void bindBidirectional(Property<A> propertyA, Property<B> propertyB, Function<A, B> mapAtoB, Function<B, A> mapBtoA) {

   or a similar helper method.

2. Just return the to-be-selected item from the method in the view model and then really select it here in the controller.

Option 1 is definitely the cleaner solution but maybe a bit harder to setup (but definitely worth the initial effort since a similar problem might appear in the future).

[Siedlerchr]

@tobiasdiez Here the binding is created, a simple list property for the combobox

[tobiasdiez]

this looks good! (a one-directional binding should suffice I think since it looks like the itemsProperty is never changed directly.)

[koppor]

Discussion @Siedlerchr with @tobiasdiez required.

[Siedlerchr]

@tobiasdiez How do I handle such cases in javafx? The check for a return value? Should I do a showAndWait?
My initial idea would be to pass the UndoManager to the javafx view model (state manager? or other injection)
There are some other clases in which a similar stuff is executed which seems to a bit more complicated, e.g. the
DownloadExternalFile class

[tobiasdiez]

Good question. I think the return value should be accessed per getters of the FileListDialogView (this seems to be the most flexible and easiest solution - you don't need to create "Dialog return value classes" for more complex scenarios). These getters in the view should ask the view model of the dialog about the values. You can get the controller with

jabref/src/main/java/org/jabref/gui/AbstractView.java

Line 45 in 1172ed4

private Optional<AbstractController> getController() {

(just change the visibility) and then the view model with

jabref/src/main/java/org/jabref/gui/AbstractController.java

Line 20 in 1172ed4

public T getViewModel() {

For parameter injection, have a look at CopyFilesDialogView.

[Siedlerchr]

@tobiasdiez I tried to convert the EditfileDialog to the new dialog structure from #3880 but I do still get an NPE on loading the dialog:

caused by: java.lang.NullPointerException
	at org.jabref.gui.filelist.LinkedFileEditDialogView.initialize(LinkedFileEditDialogView.java:55) ~[main/:?]

[tobiasdiez]

And what is null there? If line 54 runs without any problems, then the setup seems to be fine and the FXML nodes are injected. Maybe the description field is not defined in the fxml?

[Siedlerchr]

@tobiasdiez No, it fails directly in the initalize, The viewModel which gets initialized in the ctor is null.
When debugging it crashes directly after/when calling ViewLoader.view(this) .load()
Btw, I noticed the same problem also occurs with the CopyLinkedFilesDialogView (Tools->Copy LInked Files)

[tobiasdiez]

ok, this makes sense since the initialization method is called during ViewLoader.load and at this point the view model is not yet created. Just move the viewModel = ... line to the beginning of the initialization method and it should work.

[Siedlerchr]

Works now. I just additionally had to add a close/cancel button to the dialogPane, for the copyFiles because otherwise the dialog is not closeable: https://docs.oracle.com/javase/8/javafx/api/javafx/scene/control/Dialog.html

[tobiasdiez]

Thanks for the hard work over so many years 🍡

The code looks in general very good. I have a few smaller remarks. And let me add one general comment: These code reformattings make it very very hard to review the PR. For example, some files appearntly only contain formatting changes, without any actual changes to the code. This is confusing. Does eclipse provide an option to reformat only code that is actually changed instead reformatting everything once you open the file (I guess this is what happenend).

[tobiasdiez]

None of the changes in maintable-beta is currently in the changelog. Thus, for consistency, I would say remove it for the moment and add it to the list in #3621.

[tobiasdiez]

Are the changes in this file once again one of the strange eclipse formatting rules?

[tobiasdiez]

The original button was labeled OK and I think this makes more sense than Close. Moreover, the close method in this class can now be removed, right?

[Siedlerchr]

The problem was that with an only OK button I was not able to close the dialog neither via the X in the corner nor via the OK button. It needs to have at least a button with Type CANCEL_CLOSE
Read the last paragraph: https://docs.oracle.com/javase/8/javafx/api/javafx/scene/control/Dialog.html

[Siedlerchr]

The Problem was that the OK button was not at the Dialog Pane, but inside the content and therefore did not work

[tobiasdiez]

This should no longer be necessary.

[tobiasdiez]

Do we really want to show the linked file editor dialog here? In my experience, I just click "OK" anyway and would expect that this is the case for most users (recall how long it took until somebody complained that the description was not shown in the file list). At least, I would add a preferences that controls the appearance of the dialog.

[Siedlerchr]

Previously there wasn't event he FileChooser shown .Instead it only showed the LinkedFilesEditor in which I then had to click open

[tobiasdiez]

Ok, then was the previous behavior even worse. What advantage do you see in displaying the dialog every time?

[Siedlerchr]

You can add a description and or choose a different file.

[tobiasdiez]

If you really think that the majority of users wants to add a description, then keep the current behavior....

[tobiasdiez]

Please don't add generic methods like this! The idea was that the PreferencesService provides a typed interface for the preferences. Thus here it would be set/getWorkingDirectory with nio.path as argument/return value.

[tobiasdiez]

Please add this file in the java folder and not resource

[tobiasdiez]

You can also add buttontypes here in the FXML file.

[tobiasdiez]

padding and layout stuff should be controlled using css and not in the fxml file.

[tobiasdiez]

maxWidth = infinity ?

[Siedlerchr]

@tobiasdiez this is how the indentation at column level works: As I said before it's not possible to align it at the dot position. So when I use indencation on column all things are then wrapped after Builder (Github doesn't show it that wide).

[tobiasdiez]

Sorry, I wasn't clear enough. The private field should be called link with public methods linkProperty() and get/setLink, with the latter optional.

[Siedlerchr]

I fixed all things open and did the renamings. So I merge this now