Convert PreferenceDialog to MVVM #5033

calixtus · 2019-06-05T20:38:33Z

One of the last missing things in #3861 is the conversion of the PreferenceDialog and some of its tabs to MVVM. I understood that in the future JabRefPreference probably need Properties, to make them bindable. But for now I think this goes beyond this PR.

It works as expected. I took the liberty, to add a split-pane between the conents and the tabList to enable the user to change the width between them, so this PR is not just about refactoring the Dialog. ;-)

This is very early shot on this matter. I don't know if you think it's worth moving on with this and to look also into the tabs.

Tabs to convert to MVVM

General
File

The following conversions will be done in other PRs, as this one would grow too large.

After conversion of the tabs

Code cleanup, simplification, refactoring
Reordering the preferences

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 · 2019-06-05T21:06:02Z

Wow! Thanks for your take on this!
This is really a great thing.

Ideally for each tab you would have an fxml marked with root, so it's a custom component. (you have a similar concept in modern web front-end stuff)
For the default export options I already did that because they are reused in the library properties dialog.

calixtus · 2019-06-05T21:15:44Z

For the default export options I already did that because they are reused in the library properties dialog.

Oh yeah, I see that now. This is great, this is almost all the work. I can almost just copy'n'paste yours. 😆

calixtus · 2019-06-06T15:50:03Z

I made some visual improvements and converted the GeneralTab to MVVM, still everything WIP, but you can see, where the road goes. I encountered a problem, I'd like to ask to help with:
I wasn't able to apply l10n on the controls on the pane - which is why travis is currently failing - but on the large headings, which are basically labels too... Very strange. Do you have an idea?

src/main/java/org/jabref/gui/preferences/GeneralTab.fxml

calixtus · 2019-06-06T17:00:09Z

The basic workflow is now understood, the other tabs should't take too long, but I do not have much spare time the next days, so it could take some days I can continue to commit and do some cleanups. In the meantime, could you take a quick trivial look into the code, if I did something utterly wrong, so I don't repeat it?

tobiasdiez

Thanks a lot! This looks really good! Well done.

It is nice that you want to continue reworking the other tabs (no hurry!). Feel free to also move preference options between tabs or introduce new tabs. The current layout is sometimes so confusing...
If you want we can also merge this PR (after a small revision) and you can do the rework of the other tabs in new PRs. I fear otherwise this PR gets to large.

src/main/java/org/jabref/gui/preferences/GeneralTab.fxml

src/main/java/org/jabref/gui/preferences/GeneralTabView.java

src/main/java/org/jabref/gui/preferences/GeneralTabViewModel.java

src/main/java/org/jabref/gui/preferences/PreferencesDialogView.java

src/main/java/org/jabref/gui/preferences/GeneralTabViewModel.java

Siedlerchr · 2019-06-07T21:23:07Z

Thank you very much! I think we should follow the suggestion by @tobiasdiez and merge this asap and you or others can then create the fxml and logic for new tabs

calixtus · 2019-06-07T21:26:19Z

ok, i will get this ready to merge asap, but there really need to be some cleanups after converting the last one of the tabs.

calixtus · 2019-06-08T00:55:32Z

I added the FileTab, beacause i already started with it, before we were talking about merging this asap, but should be soon ready. Now I am to tired. Should be ready saturday night.

src/main/java/org/jabref/gui/preferences/FileTab.fxml

calixtus · 2019-06-08T21:56:52Z

Sorry, I was not able to present a solution including the Validator. I got them somehow working, but there is still an big issue: Somehow, the Validator does not seem to be updated after something was typed in the textfield. The StringProperty keeps somehow the old input if i dont disable and reenable it. I will have another try tomorrow.

Siedlerchr · 2019-06-12T06:21:37Z

src/main/java/org/jabref/preferences/JabRefPreferences.java

@@ -2118,4 +2118,24 @@ public void setActivePushToApplication(PushToApplication application, PushToAppl
            manager.updateApplicationAction();
        }
    }
+
+    public NewLineSeparator getNewLineSeparator() {
+        String newline = get(JabRefPreferences.NEWLINE);


we have a method in the OS class which already returns the system line separator

Thanks for looking into the code, I just realized another mistake I made.
In this mentioned case: OS.NEWLINE returns the newline-seperator of the user system. This method here concerns the stored newline-seperator in the preferences, the user can change just as she likes.

Took me a moment, now I think I understand your concern: The variable OS.NEWLINE, which normally returns the system-newline-separator is being overwritten by the method setNewLineSeparator. My question: Is this not a bug? Should the OS.NEWLINE return the user-system newLineSeparator or the one stored in the preferences? I'm a little bit confused.

I guess, as everywhere else OS.NEWLINE is used, JabRef stores in this string the actual var, which is easy to use with other strings. On the other hand, I think in the preferencesDialog, should work with the Globals.prefs directly, as OS.NEWLINE is a helper variable, which is created by initialization out of Globals.prefs, but is never stored.

I created the NewLineSeparator.class in the first place to ease up the work with the ComboBox and to get the if-else-construct out of setValue and storeValue.

I think the code is correct the way it is right now. The OS.NEWLINE returns the os-default newline character except if the user chooses to overwrite it in the preferences. It's a bit confusing (especially since OS.NEWLINE suggests that it is a constant).

tobiasdiez

Again, thanks a lot. The code looks really good and 👍 for merging from my side.

calixtus · 2019-06-14T15:07:30Z

Fixed two little things and injected dialogService. Ready again.

LinusDietz · 2019-08-24T10:32:09Z

src/main/java/org/jabref/gui/preferences/GeneralTabViewModel.java

+        if (preferences.getBoolean(JabRefPreferences.BIBLATEX_DEFAULT_MODE)) {
+            selectedBiblatexModeProperty.setValue(BibDatabaseMode.BIBLATEX);
+        } else {
+            selectedBiblatexModeProperty.setValue(BibDatabaseMode.BIBLATEX);


Bug SPOTTED! (fix incoming #5211 !) @calixtus

6fab78b Create physiologia-plantarum.csl (JabRef#5040) 58d65fd Create cureus.csl (JabRef#5048) 5b68591 Create institut-de-recherches-archeologiques-preventives.csl (JabRef#5042) e238ca2 Create method-and-theory-in-the-study-of-religion.csl (JabRef#5041) 39a3fbb Create finance-and-society.csl (JabRef#5043) cf39e19 Create canadian-biosystems-engineering-journal (JabRef#5046) 3d12b8f Create cardiff-university-biosi-and-carbs-only-harvard.csl (JabRef#5024) 776002d create new independent style for deutsches-arzteblatt.csl (JabRef#5035) b167cd6 Create revista-peruana-de-medicina-experimental-y-salud-publica.csl (JabRef#5036) 13e0a0b Update iso690-full-note-cs.csl (JabRef#5033) 3af2034 Create acta-medica-peruana.csl (JabRef#5037) dcbe494 Create historia-scribere.csl (JabRef#5038) 2174323 make addiction-biology.csl an AMA dependent (JabRef#5028) b01aea8 Update ios-press-books.csl (JabRef#5030) ae6d0e2 add doi for article-journal for ASA.csl (JabRef#5031) d537025 Create organised-sound.csl (JabRef#5032) ef4d15e Create independent EMBO Press 640fdac Create forensic-anthropology.csl (JabRef#5026) 3dc5157 Update amerindia.csl (JabRef#5027) fca38a2 Create journal-of-the-botanical-research-institute-of-texas.csl (JabRef#5029) facc3f9 Aging &Disease: remove unused macros 661e9fa Sort aging and disease by cite number 8746709 show issue numbers in springer basic author-date style (JabRef#4854) c358b17 Update Journal of Universal Computer Science git-subtree-dir: src/main/resources/csl-styles git-subtree-split: 6fab78b

6fab78b Create physiologia-plantarum.csl (#5040) 58d65fd Create cureus.csl (#5048) 5b68591 Create institut-de-recherches-archeologiques-preventives.csl (#5042) e238ca2 Create method-and-theory-in-the-study-of-religion.csl (#5041) 39a3fbb Create finance-and-society.csl (#5043) cf39e19 Create canadian-biosystems-engineering-journal (#5046) 3d12b8f Create cardiff-university-biosi-and-carbs-only-harvard.csl (#5024) 776002d create new independent style for deutsches-arzteblatt.csl (#5035) b167cd6 Create revista-peruana-de-medicina-experimental-y-salud-publica.csl (#5036) 13e0a0b Update iso690-full-note-cs.csl (#5033) 3af2034 Create acta-medica-peruana.csl (#5037) dcbe494 Create historia-scribere.csl (#5038) 2174323 make addiction-biology.csl an AMA dependent (#5028) b01aea8 Update ios-press-books.csl (#5030) ae6d0e2 add doi for article-journal for ASA.csl (#5031) d537025 Create organised-sound.csl (#5032) ef4d15e Create independent EMBO Press 640fdac Create forensic-anthropology.csl (#5026) 3dc5157 Update amerindia.csl (#5027) fca38a2 Create journal-of-the-botanical-research-institute-of-texas.csl (#5029) facc3f9 Aging &Disease: remove unused macros 661e9fa Sort aging and disease by cite number 8746709 show issue numbers in springer basic author-date style (#4854) c358b17 Update Journal of Universal Computer Science git-subtree-dir: src/main/resources/csl-styles git-subtree-split: 6fab78b

6fab78b Create physiologia-plantarum.csl (JabRef#5040) 58d65fd Create cureus.csl (JabRef#5048) 5b68591 Create institut-de-recherches-archeologiques-preventives.csl (JabRef#5042) e238ca2 Create method-and-theory-in-the-study-of-religion.csl (JabRef#5041) 39a3fbb Create finance-and-society.csl (JabRef#5043) cf39e19 Create canadian-biosystems-engineering-journal (JabRef#5046) 3d12b8f Create cardiff-university-biosi-and-carbs-only-harvard.csl (JabRef#5024) 776002d create new independent style for deutsches-arzteblatt.csl (JabRef#5035) b167cd6 Create revista-peruana-de-medicina-experimental-y-salud-publica.csl (JabRef#5036) 13e0a0b Update iso690-full-note-cs.csl (JabRef#5033) 3af2034 Create acta-medica-peruana.csl (JabRef#5037) dcbe494 Create historia-scribere.csl (JabRef#5038) 2174323 make addiction-biology.csl an AMA dependent (JabRef#5028) b01aea8 Update ios-press-books.csl (JabRef#5030) ae6d0e2 add doi for article-journal for ASA.csl (JabRef#5031) d537025 Create organised-sound.csl (JabRef#5032) ef4d15e Create independent EMBO Press 640fdac Create forensic-anthropology.csl (JabRef#5026) 3dc5157 Update amerindia.csl (JabRef#5027) fca38a2 Create journal-of-the-botanical-research-institute-of-texas.csl (JabRef#5029) facc3f9 Aging &Disease: remove unused macros 661e9fa Sort aging and disease by cite number 8746709 show issue numbers in springer basic author-date style (JabRef#4854) c358b17 Update Journal of Universal Computer Science git-subtree-dir: src/main/resources/csl-styles git-subtree-split: 6fab78b

Siedlerchr reviewed Jun 6, 2019

View reviewed changes

src/main/java/org/jabref/gui/preferences/GeneralTab.fxml Outdated Show resolved Hide resolved

tobiasdiez requested changes Jun 6, 2019

View reviewed changes

Siedlerchr reviewed Jun 7, 2019

View reviewed changes

src/main/java/org/jabref/gui/preferences/GeneralTabViewModel.java Outdated Show resolved Hide resolved

calixtus added 11 commits June 8, 2019 00:55

Refactor PreferenceDialog to MVVM with a split-pane

427f74a

Fixed wrong logging class

8462cc7

Fixed unused import

106aea4

Added visual improvements

dbfb3d6

Convertet GeneralTab to MVVM

e8ce43a

Removed old GeneralTab

4e64bde

Removed old GeneralTab

0aad34f

Added tooltip, fixed travis

e51571e

Added initial FileTab-MVVM

86de348

Minor corrections

7f0c7a6

Added FileTab, Reworked with VBox, comments and minor changes

968f028

calixtus force-pushed the preferences_mvvm branch from 47adda9 to 968f028 Compare June 8, 2019 00:50

calixtus added 4 commits June 8, 2019 02:59

Removed unused imports

083fb55

Added FileTab and some minor corrections

a225444

Removed old FileTab

64d6712

Fixed checkstyle

c91af3a

Siedlerchr reviewed Jun 8, 2019

View reviewed changes

src/main/java/org/jabref/gui/preferences/FileTab.fxml Show resolved Hide resolved

calixtus added 2 commits June 9, 2019 20:38

Added Validators and some clean-ups

414f00f

Refactor minor corrections

a12a7c7

Siedlerchr reviewed Jun 12, 2019

View reviewed changes

Refactor

3570c9e

tobiasdiez approved these changes Jun 14, 2019

View reviewed changes

calixtus added 3 commits June 14, 2019 16:03

Added Injection of dialogService

4fd959d

Fixed two glitches

6e2e445

Removed unused import

9776df2

Siedlerchr approved these changes Jun 14, 2019

View reviewed changes

Siedlerchr merged commit d811c17 into JabRef:master Jun 14, 2019

calixtus mentioned this pull request Jun 16, 2019

[WIP] Conversion of Preferences/preview to mvvm #5062

Merged

6 tasks

calixtus deleted the preferences_mvvm branch June 16, 2019 21:43

calixtus mentioned this pull request Jul 20, 2019

Conversion of preferencesDialog/advancedTab, networkTab and groupsTab to mvvm #5141

Merged

6 tasks

calixtus mentioned this pull request Aug 11, 2019

Conversion of Preferences/TableColumnsTab to mvvm #5185

Merged

6 tasks

LinusDietz reviewed Aug 24, 2019

View reviewed changes

stefan-kolb mentioned this pull request Aug 24, 2019

fix: database mode is always saved as Biblatex #5211

Merged

calixtus mentioned this pull request Sep 1, 2019

Conversion of Preferences / XMP and NameFormatters to mvvm #5265

Merged

6 tasks

calixtus mentioned this pull request Sep 15, 2019

Conversion of preferences/exportsorting, import, maintable and entryeditor to mvvm #5315

Merged

6 tasks

calixtus mentioned this pull request Sep 28, 2019

Conversion of prefs/bibtexkeygen and appearance to mvvm #5360

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert PreferenceDialog to MVVM #5033

Convert PreferenceDialog to MVVM #5033

calixtus commented Jun 5, 2019 •

edited

Loading

Siedlerchr commented Jun 5, 2019

calixtus commented Jun 5, 2019 •

edited

Loading

calixtus commented Jun 6, 2019 •

edited

Loading

calixtus commented Jun 6, 2019

tobiasdiez left a comment

Siedlerchr commented Jun 7, 2019

calixtus commented Jun 7, 2019

calixtus commented Jun 8, 2019 •

edited

Loading

calixtus commented Jun 8, 2019 •

edited

Loading

Siedlerchr Jun 12, 2019

calixtus Jun 12, 2019

calixtus Jun 12, 2019 •

edited

Loading

tobiasdiez Jun 14, 2019

tobiasdiez left a comment

calixtus commented Jun 14, 2019

LinusDietz Aug 24, 2019 •

edited

Loading

Convert PreferenceDialog to MVVM #5033

Convert PreferenceDialog to MVVM #5033

Conversation

calixtus commented Jun 5, 2019 • edited Loading

Siedlerchr commented Jun 5, 2019

calixtus commented Jun 5, 2019 • edited Loading

calixtus commented Jun 6, 2019 • edited Loading

calixtus commented Jun 6, 2019

tobiasdiez left a comment

Choose a reason for hiding this comment

Siedlerchr commented Jun 7, 2019

calixtus commented Jun 7, 2019

calixtus commented Jun 8, 2019 • edited Loading

calixtus commented Jun 8, 2019 • edited Loading

Siedlerchr Jun 12, 2019

Choose a reason for hiding this comment

calixtus Jun 12, 2019

Choose a reason for hiding this comment

calixtus Jun 12, 2019 • edited Loading

Choose a reason for hiding this comment

tobiasdiez Jun 14, 2019

Choose a reason for hiding this comment

tobiasdiez left a comment

Choose a reason for hiding this comment

calixtus commented Jun 14, 2019

LinusDietz Aug 24, 2019 • edited Loading

Choose a reason for hiding this comment

calixtus commented Jun 5, 2019 •

edited

Loading

calixtus commented Jun 5, 2019 •

edited

Loading

calixtus commented Jun 6, 2019 •

edited

Loading

calixtus commented Jun 8, 2019 •

edited

Loading

calixtus commented Jun 8, 2019 •

edited

Loading

calixtus Jun 12, 2019 •

edited

Loading

LinusDietz Aug 24, 2019 •

edited

Loading