Fix for issue 3861 : Preferences Dialog to javafx #4253

Super-Tang · 2018-08-08T11:02:04Z

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

Super-Tang · 2018-08-08T12:27:31Z

Below are what we have done to the dialog Options->Preference. The left is the original.

Siedlerchr · 2018-08-09T08:32:07Z

I think it would be easier to rebase your changes in the branch on the current (upstream, the JabRef project master) master branch, so that would hopefully avoid that conflitcts.
https://git-scm.com/book/de/v2/Git-Branching-Rebasing

Super-Tang · 2018-08-09T08:58:42Z

Thanks for the advice, I am going to fix it. @Siedlerchr

… fixDialog

tobiasdiez

On a first glance, the code changes look good. Nice work!

Some of the option panes need a bit of cleanup. Mostly visually, but also from the content (e.g. under "File" there is now an option "When downloading files, ..." which was not there before). Besides these points, I have two bigger wishes:

It looks like you already converted all preference panes to JavaFX. Thus, there is no longer a need to have the right pane in Swing and include everything via JFXPanels. Can you please convert the right selection pane also to JavaFX.
Some of the tabs still contain Swing controls, which should be removed/converted.

Moreover, please note that there are still merge problems (e.g. for the changelog).

Siedlerchr · 2018-08-10T10:05:09Z

src/main/java/org/jabref/gui/bibtexkeypattern/BibtexKeyPatternPanel.java

@@ -68,10 +68,11 @@ private void buildGUI() {
        sp.setBorder(BorderFactory.createEmptyBorder());
        pan.setLayout(gbl);
        setLayout(gbl);
-
+        javafx.scene.text.Font font1 = new javafx.scene.text.Font(10);
        // The header - can be removed
        JLabel lblEntryType = new JLabel(Localization.lang("Entry type"));


missed one ;)

Siedlerchr · 2018-08-10T10:05:31Z

src/main/java/org/jabref/gui/bibtexkeypattern/BibtexKeyPatternPanel.java

@@ -87,7 +88,8 @@ private void buildGUI() {

        JLabel lblKeyPattern = new JLabel(Localization.lang("Key pattern"));


I see that, I will do that soon :)

Super-Tang · 2018-08-13T10:43:43Z

I have solved fix the NullPointerException in setting for External Application. Then I run the JabRefMain.java, it goes ok sometimes, like this

but mostly, the program doesn't respond.

Is that OK or I have messed sth up ? @Siedlerchr

… fixDialog

Siedlerchr · 2018-08-13T12:07:03Z

It looks right so far. The Settings Dialog is still an old Swing dialog PushToApplicationSettingsDialog together with PushToApplicationSettings
which will be displayed as modal and blocking. You have to first close the swing dialog to make the rest of the application work again. If the dialog is not visible, use Alt+Tab to switch to the window),

For the moment I would let this dialog stay as is and concentrate to convert the rest of the Panes as it is a bit out of scope of this PR:
I added this to the list in #3861
You can embed Swing in JavaFX by using a SwingNode

tobiasdiez

I scrolled over the code a bit and added some remarks that catched my eye. As I said before, in general the code looks good. It would be great if you now could also cleanup all the small things. Remove duplicates etc.

tobiasdiez · 2018-08-13T12:36:19Z

src/main/java/org/jabref/JabRefGUI.java

-                                   .map(File::getAbsolutePath)
-                                   .orElse(Globals.prefs.get(JabRefPreferences.LAST_FOCUSED));
-
+                .findFirst()


The code formatting was correct before.

tobiasdiez · 2018-08-13T12:37:20Z

build.gradle

            }
        }
-		rules.withModule("com.github.tomtung:latex2unicode_2.12") { ComponentSelection selection ->
+        rules.withModule("com.github.tomtung:latex2unicode_2.12") { ComponentSelection selection ->


I guess these formatting changes are copied from a different PR. Did you merged the latest changes from master?

tobiasdiez · 2018-08-13T12:38:16Z

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

@@ -38,56 +46,87 @@ private void init() {
        fieldNames.add(BibEntry.KEY_FIELD);
        Collections.sort(fieldNames);
        String[] allPlusKey = fieldNames.toArray(new String[fieldNames.size()]);
-        savePriSort = new JComboBox<>(allPlusKey);
+        savePriSort = new ComboBox<>(FXCollections.observableArrayList(allPlusKey));


You no longer need to use an array. Instead, directly use the fieldNames collection.

tobiasdiez · 2018-08-13T12:39:19Z

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

+
+        Font font = new Font(10);
+        GridPane builder = new GridPane();
+        Label label = new Label(Localization.lang("Primary sort criterion"));


Please use descriptive names for the controls, here eg primarySortLabel. This essentially applies to all controls on all tabs.

tobiasdiez · 2018-08-13T12:39:41Z

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

+        Label label = new Label(Localization.lang("Primary sort criterion"));
+        label.setFont(font);
+        builder.add(label,1,1);
+        builder.add(savePriSort,2,1);


Our code formatting convention is that there should be a space after the comma.

tobiasdiez · 2018-08-13T12:40:23Z

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

+
+        JComboBox<String> savePriSort1 = new JComboBox<>(allPlusKey);
+        savePriSort1.setEditable(true);
+        JComboBox<String> saveSecSort1 = new JComboBox<>(allPlusKey);


This part here looks like a unnecessary code duplication.

I tried to remove these, but it seems they are used to initialize a JPanel used in DatabasePropertiesDialog.java, I haven't changed that file so far, so I kept them. As for other duplicates, I will cleanup them soon

Ok, in this case you can use a JFXPanel in DatabasePropertiesDialog to display the JavaFX controls.

tobiasdiez · 2018-08-13T12:44:05Z

src/main/java/org/jabref/gui/help/AboutDialogView.java

@@ -31,8 +31,8 @@ public AboutDialogView() {
        this.setResizable(true);

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


The formatting was ok before (applies also to some other code style changes).

tobiasdiez · 2018-08-13T12:48:22Z

src/main/java/org/jabref/gui/preftabs/AdvancedTab.java

-        pan.setBorder(BorderFactory.createEmptyBorder(5, 5, 5, 5));
-        setLayout(new BorderLayout());
-        add(pan, BorderLayout.CENTER);
+        Font font = new Font(10);


Instead of specifying the font in the code for every tab again, it is better to create one css file for the whole preference dialog. Then you can add a style class heading with a bigger font size (and maybe higher bottom padding). In the code, you then simply use useRemoteServer.setStyleClass("heading").

tobiasdiez · 2018-08-13T12:51:00Z

src/main/java/org/jabref/gui/bibtexkeypattern/BibtexKeyPatternPanel.java

+            button[i].setFont(new javafx.scene.text.Font(10));
+            button[i].setOnAction(e-> defaultPat.setText((String) Globals.prefs.defaults.get(JabRefPreferences.DEFAULT_BIBTEX_KEY_PATTERN)));
+        }
+        label[0] = new Label("Article");


Instead of hard-coding these fields, you can use a foreach loop like for (EntryType type : EntryTypes.getAllValues(mode))

Siedlerchr · 2018-08-13T15:15:18Z

src/main/java/org/jabref/gui/preftabs/AppearancePrefsTab.java

-        add(panel, BorderLayout.CENTER);
+    }
+
+    public VBox getContainer() {


Better set the return type to Node, it's always better to return the Interface Type (https://www.quora.com/What-is-the-use-of-having-an-interface-as-a-return-type-in-Java)

Ok, Done it

Siedlerchr · 2018-08-13T15:32:51Z

src/main/java/org/jabref/gui/preftabs/ExportSortingPrefsTab.java

+        builder.add(exportInTableOrder,  1, 4);
+        builder.add(new Line(), 2, 5);
+        builder.add(exportInSpecifiedOrder, 1, 6);
+        builder.add(new Line(), 2, 7);

        exportOrderPanel = new SaveOrderConfigDisplay();


If you directly initialize this SaveOrderConfigDisplay Variable at declaration above, you can change your event listener to this neat lambda:

EventHandler<ActionEvent> listener = (event) -> { boolean selected = event.getSource() == exportInSpecifiedOrder; exportOrderPanel.setEnabled(selected); };

Thanks for the advice :). Done it

Siedlerchr · 2018-08-19T10:38:13Z

src/main/java/org/jabref/gui/bibtexkeypattern/BibtexKeyPatternPanel.java

@@ -88,6 +102,10 @@ private void buildGUI() {
        con.gridy = 1;
        con.gridx = 0;
        JLabel lab = new JLabel(Localization.lang("Default pattern"));


still and old jlabel

Siedlerchr · 2018-08-19T10:38:44Z

src/main/java/org/jabref/gui/bibtexkeypattern/BibtexKeyPatternPanel.java

@@ -97,6 +115,11 @@ private void buildGUI() {
        JButton btnDefault = new JButton(Localization.lang("Default"));


old jbutton. Please remove all rest of the old swing stuff and it would be good to merge

Siedlerchr

If you just remove the 2 or 3 remainig swing labels and buttons which are no longer needed, we can merge it

Super-Tang · 2018-08-19T12:02:37Z

I am sorry for forgetting this， done it. :) @Siedlerchr

Siedlerchr

Thanks for your work on this!

Super-Tang and others added 8 commits July 31, 2018 00:08

Convert swing dialog to JavaFx

97cd767

convert the swing dialog to javafx

1868304

Change the code style to meet the standard

99caee8

Update CHANGELOG.md

70d304f

Update SaveOrderConfigDisplay.java

9f3b3b7

Merge the latest commits

ee486e7

Merge the latest commits

ad651fa

merge the latest commits

8f5d7f7

Super-Tang added 2 commits August 9, 2018 16:18

Polish the code style

ee1c275

resolve the conficts

58b52fa

Super-Tang added 2 commits August 9, 2018 16:46

polish the code style

ab3f78c

reslove the conflicts

4f1c8ca

Super-Tang and others added 3 commits August 9, 2018 17:07

resolve the conflict

ab409da

Merge branch 'master' into fixDialog

f7f5841

Change the CHANGELOG.md

278693b

tobiasdiez mentioned this pull request Aug 9, 2018

Convert the Swing Dialog to JavaFX #4249

Closed

6 tasks

Super-Tang added 2 commits August 9, 2018 19:48

polish the code style

0b5c0cf

Merge branch 'fixDialog' of https://github.com/1160300311/jabref into…

c2db43a

… fixDialog

Super-Tang changed the title ~~Fix dialog~~ Fix for issue 3861 Aug 9, 2018

Super-Tang added 3 commits August 9, 2018 19:59

polish the code style

e72485e

Modify the code to make test-suite pass

848478e

polish the code style

afbcee0

tobiasdiez requested changes Aug 9, 2018

View reviewed changes

Beautify the option panes

760b837

Siedlerchr reviewed Aug 10, 2018

View reviewed changes

Polish the code style

0fb9d89

Super-Tang added 2 commits August 13, 2018 14:19

Update JabRefGUI.java

e2cb75b

Update JabRefGUI.java

f1e391c

Super-Tang added 2 commits August 13, 2018 19:55

correct the wrong label name in settings for external Application

57a7cfa

Merge branch 'fixDialog' of https://github.com/1160300311/jabref into…

755afae

… fixDialog

Siedlerchr mentioned this pull request Aug 13, 2018

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

Closed

36 tasks

tobiasdiez requested changes Aug 13, 2018

View reviewed changes

Super-Tang added 4 commits August 13, 2018 22:47

remove the duplicates and cleanup the code

8e1bad8

remove unused imports

aec8003

remove the duplicates in SaveOrderConfigDisplay.java

d00704e

remove the unused imports in SaveOrderConfigDisplay.java

e773285

Siedlerchr reviewed Aug 13, 2018

View reviewed changes

Change the return type to Interface Type

d3d0a97

Siedlerchr reviewed Aug 13, 2018

View reviewed changes

Super-Tang and others added 4 commits August 13, 2018 23:47

Change to lambda expression

b8f075b

cleanup the code

6f205f1

Merge latest commits

55fd4ee

Merge branch 'master' into fixDialog

73b4315

Siedlerchr reviewed Aug 19, 2018

View reviewed changes

Siedlerchr approved these changes Aug 19, 2018

View reviewed changes

remove the Swing contron in BibtexKeyPatternPanel

71c5b72

Code style :(

9b9e3d1

Siedlerchr approved these changes Aug 19, 2018

View reviewed changes

tobiasdiez merged commit 6e27922 into JabRef:master Aug 19, 2018

Siedlerchr mentioned this pull request Aug 20, 2018

convert bibtexkeypatterndlg to javafx as well #4277

Merged

6 tasks

tobiasdiez mentioned this pull request Aug 21, 2018

Improve preferences #4280

Merged

6 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

Fix for issue 3861 : Preferences Dialog to javafx #4253

Fix for issue 3861 : Preferences Dialog to javafx #4253

Super-Tang commented Aug 8, 2018 •

edited

Loading

Super-Tang commented Aug 8, 2018

Siedlerchr commented Aug 9, 2018

Super-Tang commented Aug 9, 2018

tobiasdiez left a comment

Siedlerchr Aug 10, 2018

Siedlerchr Aug 10, 2018

Super-Tang Aug 10, 2018

Siedlerchr Aug 19, 2018

Super-Tang commented Aug 13, 2018

Siedlerchr commented Aug 13, 2018

tobiasdiez left a comment

tobiasdiez Aug 13, 2018

tobiasdiez Aug 13, 2018

tobiasdiez Aug 13, 2018

tobiasdiez Aug 13, 2018

tobiasdiez Aug 13, 2018

tobiasdiez Aug 13, 2018

Super-Tang Aug 13, 2018

tobiasdiez Aug 13, 2018

tobiasdiez Aug 13, 2018

tobiasdiez Aug 13, 2018

tobiasdiez Aug 13, 2018

Siedlerchr Aug 13, 2018

Super-Tang Aug 13, 2018

Siedlerchr Aug 13, 2018

Super-Tang Aug 13, 2018

Siedlerchr Aug 19, 2018

Siedlerchr Aug 19, 2018

Siedlerchr left a comment

Super-Tang commented Aug 19, 2018

Siedlerchr left a comment

		@@ -87,7 +88,8 @@ private void buildGUI() {

		JLabel lblKeyPattern = new JLabel(Localization.lang("Key pattern"));

		@@ -97,6 +115,11 @@ private void buildGUI() {
		JButton btnDefault = new JButton(Localization.lang("Default"));

Fix for issue 3861 : Preferences Dialog to javafx #4253

Fix for issue 3861 : Preferences Dialog to javafx #4253

Conversation

Super-Tang commented Aug 8, 2018 • edited Loading

Super-Tang commented Aug 8, 2018

Siedlerchr commented Aug 9, 2018

Super-Tang commented Aug 9, 2018

tobiasdiez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Super-Tang commented Aug 13, 2018

Siedlerchr commented Aug 13, 2018

tobiasdiez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Siedlerchr left a comment

Choose a reason for hiding this comment

Super-Tang commented Aug 19, 2018

Siedlerchr left a comment

Choose a reason for hiding this comment

Super-Tang commented Aug 8, 2018 •

edited

Loading