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.

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

fix "save" button in preference dialog not response #4406

Merged
merged 3 commits into from
Oct 25, 2018

Conversation

springzfx
Copy link

I notice there is a dark theme in recent commit. So I tried it, and find out an issue that you can not save preference. "Save" button have no response. The issue starts from Provide access to dark theme in preferences (#4372).

The reason is that default value for key "fxTheme" is not set, thus cause a null exception when access this option. Solution is simple, just to set default value "Base.css" for key "fxTheme" in preference.

Finally, I have to say dark theme is a great job. thanks

this is because default value for fxTheme is not set, thus cause null exception
solution: set default value "Base.css" for key "fxTheme" in preferencewq
@springzfx springzfx changed the title fix "save" button in preference dialog not response or work fix "save" button in preference dialog not response Oct 24, 2018
Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Although your solution works, it has the disadvantage that now the Base.css file is loaded twice every time (this was remarked in one of the recent PRs on this topic, not sure which one). It would be nice if you could also fix this in the progress. I'm not sure whether it is easier to modify the ThemeLoader to not load the base css file twice or leave the property value at null by default but prevent the NPE from happening by other means. I would leave this to you to decide.

Thanks also for the kind words regarding the dark theme. There might still be some visual problems with it. If you notice something, please open an issue (or even better a PR) for it!

@@ -780,6 +780,8 @@ private JabRefPreferences() {
+ "\\begin{comment}<BR><BR><b>Comment: </b> \\format[HTMLChars]{\\comment} \\end{comment}"
+ "</dd>__NEWLINE__<p></p></font>");

// set default theme
defaults.put(JabRefPreferences.FX_THEME, "Base.css");
Copy link
Member

Choose a reason for hiding this comment

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

Please don't use the Base.css constant here but instead reuse a corresponding constant in ThemeLoader.

@springzfx
Copy link
Author

public void installBaseCss(Scene scene, JabRefPreferences preferences);
I track the theme loading process, but not find it loaded twice.
More information will help me to figure it out.

@tobiasdiez
Copy link
Member

When jabRefPreferences.get(JabRefPreferences.FX_THEME) returns Base.css, then after

String cssFileName = jabRefPreferences.get(JabRefPreferences.FX_THEME);
if (cssFileName != null) {
try {
cssProperty = Paths.get(JabRefFrame.class.getResource(cssFileName).toURI()).toString();

cssProperty points to the base css file. Hence, it will be loaded twice in installBaseCss:
addAndWatchForChanges(scene, DEFAULT_PATH_MAIN_CSS, 0);

and in
final Path path = Paths.get(cssProperty);
if (Files.isReadable(path)) {
String cssUrl = path.toUri().toString();
addAndWatchForChanges(scene, cssUrl, 1);

@springzfx
Copy link
Author

The most simple fix is this:

private void addAndWatchForChanges(Scene scene, String cssUrl, int index) {
// avoid repeat add
if (scene.getStylesheets().contains(cssUrl)) return;
scene.getStylesheets().add(index, cssUrl);

@springzfx
Copy link
Author

It the issue(#4385 (comment)) mentioned this double loading problem.

Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

That works. Thanks for the follow-up. Please fix the code style issues and then we can merge.

@tobiasdiez tobiasdiez merged commit 4d89b0a into JabRef:master Oct 25, 2018
Siedlerchr added a commit that referenced this pull request Oct 27, 2018
* upstream/master: (30 commits)
  Update README.md (#4419)
  Fix generate bibtex key overwrite warning dialog (#4418)
  Fix group hit counter when adding entries (#4413)
  Update README.md
  fix java.nio.file.FileSystemNotFoundException and reorganize code (#4416)
  fixed an issue where corresponding groups are sometimes not highlighted when clicking on entries (#4404)
  Add a few journal abbreviations (#4412)
  Fix display of radio buttons (#4411)
  Update issue templates
  Delete ISSUE_TEMPLATE.md
  Update issue templates
  Integrate mrdlib redesign (#4361)
  fix "save" button in preference dialog not response (#4406)
  show dialog before creating entry (#4405)
  Scrollbar invisible in Preferences -> BibTex Key Pattern (#4287) (#4398)
  Updates (#4402)
  Fixes for all mods exporter tests (#4396)
  Fix EntryType dialog not closed after generate button (#4390)
  Make rank image narrower (#4395)
  Fix showing multiple icons for one menu entry - fixes #4384 (#4392)
  ...
@springzfx springzfx deleted the fixDefaultThemeNotSet branch October 28, 2018 06:30
Siedlerchr added a commit that referenced this pull request Oct 29, 2018
* upstream/master: (54 commits)
  Update README.md (#4419)
  Fix generate bibtex key overwrite warning dialog (#4418)
  Fix group hit counter when adding entries (#4413)
  Update README.md
  fix java.nio.file.FileSystemNotFoundException and reorganize code (#4416)
  fixed an issue where corresponding groups are sometimes not highlighted when clicking on entries (#4404)
  Add a few journal abbreviations (#4412)
  Fix display of radio buttons (#4411)
  Update issue templates
  Delete ISSUE_TEMPLATE.md
  Update issue templates
  Integrate mrdlib redesign (#4361)
  fix "save" button in preference dialog not response (#4406)
  show dialog before creating entry (#4405)
  Scrollbar invisible in Preferences -> BibTex Key Pattern (#4287) (#4398)
  Updates (#4402)
  Fixes for all mods exporter tests (#4396)
  Fix EntryType dialog not closed after generate button (#4390)
  Make rank image narrower (#4395)
  Fix showing multiple icons for one menu entry - fixes #4384 (#4392)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants