Completed General Fields Customizer conversion to JavaFX #4346

abepolk · 2018-09-15T22:10:13Z

This is what I have so far. The problem is that most, but not all of the time, when I run the "Set up general fields" dialog, there is a problem with ViewLoader and the program throws an error. Perhaps it is because I left the view model unfinished?

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?)

…g to JavaFX

Siedlerchr · 2018-09-16T12:31:00Z

I looked into this and I think the main problem was that you missed the fx:ids for the button and the textarea. The fx:id must correspond to the variable in the View.

To spot such errors, it helps if you set a breakpoint in the initialize method of the View and then debug through it step by step. Then I discovered that the textarea variable was null -> so conclusion was missing id in the fxml

I fixed this and exported it as git patch. Remove the .txt extension and you can import it with
git am < 0001-fix-missing-ids.patch
0001-fix-missing-ids.patch.txt

abepolk · 2018-09-21T20:15:06Z

I finished changing the Set Up General Fields Dialogs to JavaFX. The automatic build failures were unrelated to the changed code, and think I fixed all the true problems that Codacy mentioned.

The functions getCustomTabsNamesAndFields() and getCustomTabFieldNames() in the class JabRefPreferences, are very similar and it might be worth writing one as a function of the other to reduce boilerplate. I thought they were different enough (especially with respect to the return types) to keep them separate.

Siedlerchr

Thank you very much for your contribution! 👍
Overall looks good, just some minor improvements to fix and it's good!

Siedlerchr · 2018-09-21T20:50:09Z

src/main/java/org/jabref/gui/genfields/GenFieldsCustomizerDialog.fxml

+ <VBox prefHeight="300.0" prefWidth="650.0">
+ <children>
+ <Label text="General Fields" />
+ <Label text="Create custom fields for each BibTeX entry" />


To ensure that the labels are correctly translated in other languages it is necessary to add the percent sign in front of the text., e. G. %create custom...
This will probably also resolve the Travis failure due to localisation

src/main/java/org/jabref/gui/genfields/GenFieldsCustomizerDialogView.java

Siedlerchr · 2018-09-21T21:02:56Z

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

+ String name = (String) defaults.get(CUSTOM_TAB_NAME + "_def" + defNumber);
+ String fields = (String) defaults.get(CUSTOM_TAB_FIELDS + "_def" + defNumber);
+
+ if ((name == null) || (fields == null) || name.isEmpty() || fields.isEmpty()) {


You can use StringUtil.IsNullOrEmpty (from org.jabref.model.strings.StringUtil)

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

tobiasdiez

Thanks for your contribution. The code looks good to me, except one or two very minor things. Please also have a look at the travis build, which currently fails.

src/main/java/org/jabref/gui/genfields/GenFieldsCustomizerDialogView.java

tobiasdiez · 2018-09-24T06:16:22Z

src/main/java/org/jabref/gui/genfields/GenFieldsCustomizerDialogView.java

+ @Inject private PreferencesService preferences;
+ private GenFieldsCustomizerDialogViewModel viewModel;
+
+ public GenFieldsCustomizerDialogView() {


Can you please rename all classes related to the dialog to something like CustomizeGeneralFieldsDialog

tobiasdiez · 2018-09-24T06:19:21Z

src/main/java/org/jabref/gui/genfields/GenFieldsCustomizerDialogViewModel.java

+ sb.append(':');
+ sb.append(entry.getValue());
+ sb.append('\n');
+ fieldsText.set(sb.toString());


The fieldsText.set should probably come after the for loop.

abepolk · 2018-09-24T19:29:47Z

Thanks for the feedback! I've made the changes. I believe Codacy fails because it does not recognize that ViewLoader takes care of the FXML in the controller, so I don't need to explicitly use all the variables that I create in the controller. The build now passes, but it does say some dependency updates are needed.

Siedlerchr · 2018-09-25T06:19:44Z

src/main/java/org/jabref/gui/customizefields/CustomizeGeneralFieldsDialogView.java

+ @FXML private ButtonType resetButton;
+ @FXML private ButtonType helpButton;
+ @FXML private ButtonType okButton;
+ @FXML private ButtonType cancelButton;


If you don't use the cancel button further in your code, you can remove it here and in the fxml remove the fx:id from it.

Siedlerchr · 2018-09-25T06:20:40Z

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

@@ -927,7 +927,7 @@ private MenuBar createMenu() {

 new SeparatorMenuItem(),

- factory.createMenuItem(StandardActions.SETUP_GENERAL_FIELDS, new SetupGeneralFieldsAction(this)),
+  factory.createMenuItem(StandardActions.SETUP_GENERAL_FIELDS, new SetupGeneralFieldsAction()),


Please align/indent it with the other ones below

Siedlerchr · 2018-09-25T06:23:04Z

Thank you very much! It looks good to me now. Just one formatting issue and that you don't need the variable in the controller for cancelButton.

Ignore the dependency updates, the dependency check is run with every build and is just a hint that there might be some dependencies which could be updated. It fails if newer versions are found. But some of them are incompatible so ignore them

tobiasdiez

Thanks again!

abepolk · 2018-09-25T13:33:59Z

My pleasure.

abepolk · 2018-09-25T13:48:03Z

Hi, I have another question. In terms of broad categories of fixes, what are the highest priorities right now for working on JabRef, apart from the conversion to JavaFX? I'm glad to convert more dialogs, but I also want to be involved with the logic and model. Let me know. Thanks!

tobiasdiez · 2018-09-25T15:21:04Z

You can have a look at https://github.com/JabRef/jabref/projects/5 for high-priority bugs that need to be fixed. But I fear that most of them are purely UI-related.
Most of stuff labeled with fetcher or code quality involves only the backend. For example, creating a fetcher for worldcat would be nice.

abepolk · 2018-09-25T17:02:54Z

Good to know. I'll keep you posted when I take up another dialog or issue from the issue page. Thanks for being so friendly!

Siedlerchr · 2018-09-25T17:44:52Z

We have to thank you for your interest in making JabRef great again 😏

* upstream/master: Update Libraries (#4366) ArXiv fetcher support http url (#4367) Fix issue 3861 : XMP Dialog, Add new Groups dialog, Append Library dialog to javafx (#4264) fix IndexOutOfBoundsException when saving preferences group RadioButtons in ExportSortingPrefsTab to prevent the selection of multiple or no order types Convert Part of the Issue#3861 Quality->Cleanup entries (#4268) UPDATE gradle to 4.10.2 (#4358) Completed General Fields Customizer conversion to JavaFX (#4346) Fix JavaFX thread exception when fetching new infos (#4354) Auto trim url field (#4355) Fix freezes in entry editor (#4351) # Conflicts: # src/main/java/org/jabref/preferences/PreferencesService.java

abepolk added 8 commits September 6, 2018 19:52

temp commit 09 06 18

6e5694a

starting work on converting Customize General Fields dialog from Swin…

92cddb1

…g to JavaFX

More work on converting JavaFX dialog

24eb0a6

Even more work on converting JavaFX dialog

770a771

more work JavaFX

6a0b878

Further work, now cannot load

4058a70

Cleaned up comments

b3d5238

create newlines at ends of files

51712e2

Siedlerchr changed the title ~~Gen javafx dialog~~ [WIP] Gen javafx dialog Sep 16, 2018

Siedlerchr and others added 8 commits September 16, 2018 09:49

fix missing ids change bidirectional binding use string property

ee919a4

Fix default and help

6eabd86

Got the dialog running, but it is not saving

3435997

Removed _def

5133d44

fix bug

4c6e309

Reformat customize general fields dialog as JavaFX

b726a81

Merge branch 'master' into gen_javafx_dialog

0d9a2a8

Small code fixes

78c8748

abepolk changed the title ~~[WIP] Gen javafx dialog~~ Completed General Fields Customizer conversion to JavaFX Sep 21, 2018

Siedlerchr requested changes Sep 21, 2018

View reviewed changes

abepolk added 3 commits September 22, 2018 11:12

make fxml support multiple languages

857b9ea

minor code fixes

c5ce5b1

added javadoc

4e9d78a

tobiasdiez requested changes Sep 24, 2018

View reviewed changes

abepolk added 5 commits September 24, 2018 14:31

change class name and code fix

bf1c5d3

Add English localization

5bb9195

remove old Swing dialog

a22fcf0

comment fix

7ae7a0e

English localization change

fb22955

Siedlerchr reviewed Sep 25, 2018

View reviewed changes

abepolk added 2 commits September 25, 2018 04:47

fix indentation

ae9ff0d

remove cancelButton from controller

6cd1acd

Siedlerchr approved these changes Sep 25, 2018

View reviewed changes

tobiasdiez approved these changes Sep 25, 2018

View reviewed changes

tobiasdiez merged commit 2f433d2 into JabRef:master Sep 25, 2018

Siedlerchr mentioned this pull request Sep 25, 2018

Convert/rework rest of the Swing Dialogs to javafx #3861

Closed

36 tasks

luizphilipe2 mentioned this pull request Jun 26, 2021

Convert AutoLinkFilesAction to JavafX luizphilipe2/jabref_experiment#32

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Completed General Fields Customizer conversion to JavaFX #4346

Completed General Fields Customizer conversion to JavaFX #4346

abepolk commented Sep 15, 2018 •

edited

Loading

Siedlerchr commented Sep 16, 2018

abepolk commented Sep 21, 2018

Siedlerchr left a comment

Siedlerchr Sep 21, 2018

Siedlerchr Sep 21, 2018

tobiasdiez left a comment

tobiasdiez Sep 24, 2018

tobiasdiez Sep 24, 2018

abepolk commented Sep 24, 2018

Siedlerchr Sep 25, 2018

Siedlerchr Sep 25, 2018

Siedlerchr commented Sep 25, 2018

tobiasdiez left a comment

abepolk commented Sep 25, 2018

abepolk commented Sep 25, 2018

tobiasdiez commented Sep 25, 2018

abepolk commented Sep 25, 2018

Siedlerchr commented Sep 25, 2018

Completed General Fields Customizer conversion to JavaFX #4346

Completed General Fields Customizer conversion to JavaFX #4346

Conversation

abepolk commented Sep 15, 2018 • edited Loading

Siedlerchr commented Sep 16, 2018

abepolk commented Sep 21, 2018

Siedlerchr left a comment

Choose a reason for hiding this comment

Siedlerchr Sep 21, 2018

Choose a reason for hiding this comment

Siedlerchr Sep 21, 2018

Choose a reason for hiding this comment

tobiasdiez left a comment

Choose a reason for hiding this comment

tobiasdiez Sep 24, 2018

Choose a reason for hiding this comment

tobiasdiez Sep 24, 2018

Choose a reason for hiding this comment

abepolk commented Sep 24, 2018

Siedlerchr Sep 25, 2018

Choose a reason for hiding this comment

Siedlerchr Sep 25, 2018

Choose a reason for hiding this comment

Siedlerchr commented Sep 25, 2018

tobiasdiez left a comment

Choose a reason for hiding this comment

abepolk commented Sep 25, 2018

abepolk commented Sep 25, 2018

tobiasdiez commented Sep 25, 2018

abepolk commented Sep 25, 2018

Siedlerchr commented Sep 25, 2018

abepolk commented Sep 15, 2018 •

edited

Loading