Convert Preferences/ExternalTab to MVVM

[calixtus]

follow-up to #5033

Converted the ExternalTab to MVVM, but keeps the same functionality like before. Just some small corrections. Ready to review.

---

• Change in CHANGELOG.md described
• Tests created for changes
• Manually tested changed features in running JabRef
• Screenshots added in PR description (for bigger UI changes)
• Ensured that the git commit message is a good one
• Check documentation status (Issue created for outdated help page at help.jabref.org?)

[Siedlerchr]

This is a parameterized localization string, e.g. the %0 works similar as in String.format as argument and is usually used to indicate values that should not be translated

Not sure if it is possible to use them in the fxml directly

[calixtus]

It showed up fine, I did not see any complaints from the compiler nor from the jre. The original line was

new Label(Localization.lang("Note: Use the placeholder %0 for the location of the opened library file.", "%DIR") )

The other option I thought of would be to put this in the initialize-method as setValue. If you want so, I can change it, but I don't think that this would be a good solution, as everything else is completly written in fxml.

[Siedlerchr]

not sure for root components, but You can Inject DialogService and could also inject the PreferencesService interface. You would then just need the getters/setter methods for each value

[calixtus]

Is there some way also to inject the the JabRefFrame?
This @Inject-DI-stuff is really black-magic-vodoo to me.

[tobiasdiez]

It is not possible to inject the JabRefFrame (and we will also not add this functionality as we try to get rid of the omnipresence of JabrefFrame). Here you find an overview of the possible @Inject classes:

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

Lines 31 to 51 in 47e3cc5

	if (clazz == DialogService.class) {
	return JabRefGUI.getMainFrame().getDialogService();
	} else if (clazz == TaskExecutor.class) {
	return Globals.TASK_EXECUTOR;
	} else if (clazz == PreferencesService.class) {
	return Globals.prefs;
	} else if (clazz == KeyBindingRepository.class) {
	return Globals.getKeyPrefs();
	} else if (clazz == JournalAbbreviationLoader.class) {
	return Globals.journalAbbreviationLoader;
	} else if (clazz == StateManager.class) {
	return Globals.stateManager;
	} else if (clazz == FileUpdateMonitor.class) {
	return Globals.getFileUpdateMonitor();
	} else if (clazz == ProtectedTermsLoader.class) {
	return Globals.protectedTermsLoader;
	} else if (clazz == ClipBoardManager.class) {
	return Globals.clipboardManager;
	} else if (clazz == UndoManager.class) {
	return Globals.undoManager;
	} else {

If you want to dig deeper into how the injection works, have a look here: https://github.com/JabRef/afterburner.fx/tree/master/src/main/java/com/airhacks/afterburner/injection
But yes it is kind of black magic by design, since you normally you don't want to think about how the injection works.

[calixtus]

I think I somehow get an idea of how to use it.
If I inject the PreferencesService, would that imply, that I have to add getters and setters for every value accessed by the PreferenceDialog in PreferencesService (which is basically almost every value in JabRefPreferences)? Or do i just don't understand, what you mean?

[Siedlerchr]

Yes, you are right. PreferencesServices is the interface for JabRefPreferences which can be injected. Of course you would need to add a lot of getters and setters for the values in the JabRefPreferences class, but I think it's not worth it for the preferences dialog.
But for other dialogs this is useful.

[calixtus]

What I can do is to inject PreferencesService in PreferencesDialog and downcast it to JabRefPreferences when I create the instances of the single tabs. - Or better I inject it in the TabsView and cast it when I create the ViewModel.

[tobiasdiez]

In the long term, it is indeed desirable to inject the preference service. But this requires that all preference options are grouped in certain preference classes (e.g. EntryEditorPreferences). For the moment, it is ok if you pass a JabRefPreferences as a constructor argument.

[calixtus]

Ok, now dialogService is injected, preferencesService not. Reworking of JabRefPreferences would be another PR in the future sometime, bit this one should be done and ready to review and to merge, after the other one is merged.

[Siedlerchr]

I just merged your other OR, seems to have created conflicts now

[calixtus]

I expected them, since i did a few addidtions, this PR wouldnt be compileable. Back in 15 mins.

[calixtus]

ok, resolved.

[tobiasdiez]

Looks really good to me!

[Siedlerchr]

Thanks again for your hard work!