Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to fix the issue #6039 Font size increase does not increase preferences font size #6429

Closed
wants to merge 12 commits into from
Closed

Attempt to fix the issue #6039 Font size increase does not increase preferences font size #6429

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6e62a53
Merge pull request #1 from JabRef/master
LuckyOne09 May 1, 2020
7d0b7fd
Merge pull request #2 from JabRef/master
LuckyOne09 May 2, 2020
2a431e0
Merge pull request #3 from JabRef/master
LuckyOne09 May 3, 2020
3e449a6
Merge pull request #4 from JabRef/master
LuckyOne09 May 3, 2020
3c6bd01
Merge pull request #5 from JabRef/master
LuckyOne09 May 5, 2020
5e6093f
Merge pull request #6 from JabRef/master
LuckyOne09 May 6, 2020
9af83f4
attempt to fix the issue #6039: Font size increase does not increase …
Luckylys May 6, 2020
75198fb
Pass the ThemeLoader object and JabRefPreferences object into ShowPre…
Luckylys May 6, 2020
dee3543
Create journalList.mv
May 6, 2020
4b33736
Revert "Pass the ThemeLoader object and JabRefPreferences object into…
May 6, 2020
1928a1b
remove unnecessary import
May 6, 2020
54a30f0
add record in CHANGELOG.md
May 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/main/java/org/jabref/gui/preferences/PreferencesDialogView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import javafx.scene.control.ListView;
import javafx.scene.control.ScrollPane;

import org.jabref.Globals;
import org.jabref.gui.DialogService;
import org.jabref.gui.JabRefFrame;
import org.jabref.gui.icon.IconTheme;
Expand Down Expand Up @@ -58,6 +59,8 @@ public PreferencesDialogViewModel getViewModel() {
return viewModel;
}

public ListView<PreferencesTab> getPreferenceTabList() { return preferenceTabList; }

@FXML
private void initialize() {
viewModel = new PreferencesDialogViewModel(dialogService, frame);
Expand Down
9 changes: 8 additions & 1 deletion src/main/java/org/jabref/gui/preferences/ShowPreferencesAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.jabref.gui.preferences;

import org.jabref.Globals;
import org.jabref.gui.JabRefFrame;
import org.jabref.gui.actions.SimpleCommand;
import org.jabref.gui.util.TaskExecutor;
Expand All @@ -16,6 +17,12 @@ public ShowPreferencesAction(JabRefFrame jabRefFrame, TaskExecutor taskExecutor)

@Override
public void execute() {
new PreferencesDialogView(jabRefFrame).show();
PreferencesDialogView preferencesDialogView = new PreferencesDialogView(jabRefFrame);
preferencesDialogView.show();
for (PreferencesTab tab: preferencesDialogView.getPreferenceTabList().getItems()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The action shouldn't really care about how the dialog is displayed, that's the job of the dialog itself. Thus, I would prefer if the fix can be moved to the PreferencesDialogView class. Maybe to https://github.com/JabRef/jabref/pull/6429/files#diff-39a71e5858d655cd93a5ca0a21ff300aL81 ?

Copy link
Contributor Author

@LuckyOne09 LuckyOne09 May 6, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @tobiasdiez
Thanks for your suggestions. In fact, I tried to fix it in the PreferencesDialogView, but the PreferencesTab.getBuilder().getScene() is not null only when it is initialized after preferencesDialogView.show(). Therefore, this is the only way I found to fix it. Maybe I missed something important, but I really have no idea to handle that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did you tried to add it exactly? The scene should be non-null as soon as the control has been added to the dialog. (But to be honest I don't understand why the preference tab doesn't inherit the css from the preference window).

Copy link
Contributor Author

@LuckyOne09 LuckyOne09 May 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I write some codes to make my point clear:

PreferencesDialogView preferencesDialogView = new PreferencesDialogView(jabRefFrame);
for (PreferencesTab tab: preferencesDialogView.getPreferenceTabList().getItems()) {
    System.out.println("result of getBuilder(): ");
    System.out.println(tab.getBuilder());
    System.out.println("result of getScene(): ");
    System.out.println(tab.getBuilder().getScene());
}
preferencesDialogView.show();
System.out.println("after show()");
for (PreferencesTab tab: preferencesDialogView.getPreferenceTabList().getItems()) {
    System.out.println("result of getBuilder(): ");
    System.out.println(tab.getBuilder());
    System.out.println("result of getScene(): ");
    System.out.println(tab.getBuilder().getScene());
}

The part of result is:

result of getBuilder(): 
	GeneralTabView@5448f579
result of getScene(): 
	null
result of getBuilder(): 
	FileTabView@28189b70
result of getScene(): 
	null

after show()

result of getBuilder(): 
	GeneralTabView@5448f579
result of getScene(): 
	javafx.scene.Scene@454c8139
result of getBuilder(): 
	FileTabView@28189b70
result of getScene(): 
	null

You can see that the result of getScene() is not null only after preferencesDialogView.show().

By the way, I found that preference tab is not the only one having this problem.
Nearly every pop window suffer this problem like this screenshot:
捕获

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,
all those popup dialogs inherit from BaseDialog and there is this line for setting the css as well:
Globals.getThemeLoader().installCss(getDialogPane().getScene(), Globals.prefs);

This would explain why those dialogs still have the same size

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thankyou for your guidance.
I tried to use

EasyBind.subscribe(new ReadOnlyObjectWrapper(getDialogPane().getScene()), scene -> Globals.getThemeLoader().installCss((Scene) scene, Globals.prefs));

to replace

Globals.getThemeLoader().installCss(getDialogPane().getScene(), Globals.prefs);

in constructor of BaseDialog.
However, I found that this installCss() was still executed before DialogView.show() (this is pseudo code, if I am press "Customize key bindings", then it should be KeyBindingsDialogView().show())
I am confused now about how to solve this.
The only solution I can come up with is adding some codes after each DialogView.show().

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, you could try to use the properties onShown or onShowing https://docs.oracle.com/javase/8/javafx/api/javafx/scene/control/Dialog.html

Copy link
Member

@tobiasdiez tobiasdiez May 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use getScene then you access the current value of the scene (which is null) and this is static, i.e. the ReadOnlyObjectWrapper will never update. To resolve this, you can use the sceneProperty to get notified about the new value.

Something like the following should work:
EasyBind.wrapNullable(getDialogPane().sceneProperty()).subscribeToValues(scene -> Globals.getThemeLoader().installCss(scene, Globals.prefs))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use getScene then you access the current value of the scene (which is null) and this is static, i.e. the ReadOnlyObjectWrapper will never update. To resolve this, you can use the sceneProperty to get notified about the new value.

Something like the following should work:
EasyBind.wrapNullable(getDialogPane().sceneProperty()).subscribeToValues(scene -> Globals.getThemeLoader().installCss(scene, Globals.prefs))

I found a interesting thing. That is I found that the KeyBindingsDialogView.getDialogPane().getScene() can be initiated properly before KeyBindingsDialogView.show()

To prove this, I add some codes in construtor of BaseDialog:

        System.out.println("before before");
        System.out.println(getDialogPane().getScene());
        Globals.getThemeLoader().installCss(getDialogPane().getScene(), Globals.prefs);

and see the behaviours in the debug mode.

following is results:

command line output is :

before before
javafx.scene.Scene@b1b581d
07:27:19.064 [JavaFX Application Thread] INFO  org.jabref.gui.util.ThemeLoader - Enabling live reloading of {$workspace}\build\resources\main\org\jabref\gui\Base.css

screenshot for debug mode (19 is the font size I set)
image

I think it means Themeloader.installCss() can apply to KeyBindingsDialogView properly but the font size does not change. I am comfused again about this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it also the same scene that is used after the dialog is shown? Maybe it's registering to the old scene that gets replaced?

if (tab.getBuilder().getScene() != null) {
Globals.getThemeLoader().installCss(tab.getBuilder().getScene(), Globals.prefs);
LuckyOne09 marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}