4130 provide dark theme#4372
Conversation
Siedlerchr
left a comment
There was a problem hiding this comment.
Thanks for your contribution, despite some minor things, the code looks good
|
|
||
| String cssFileName = JabRefPreferences.getInstance().get(JabRefPreferences.FX_THEME); | ||
| if (cssFileName != null) { | ||
| String themeName = JabRefFrame.class.getResource(cssFileName).getPath(); |
There was a problem hiding this comment.
Please use the nio methods for accessing the resource. Have a look at the exporter tests to see how
|
Ready for review |
| - We added a minimal height for the entry editor so that it can no longer be hidden by accident. [#4279](https://github.com/JabRef/jabref/issues/4279) | ||
| - We added a new keyboard shortcut so that the entry editor could be closed by <kbd>Ctrl<kbd> + <kbd>E<kbd>. [#4222] (https://github.com/JabRef/jabref/issues/4222) | ||
|
|
||
| - Change theme to dark or light via settings |
There was a problem hiding this comment.
Please formulate the changelog as the above ones and link the issue
|
|
||
| String cssFileName = jabRefPreferences.get(JabRefPreferences.FX_THEME); | ||
| if (cssFileName != null) { | ||
| CSS_SYSTEM_PROPERTY = new File("src/main/java/org/jabref/gui/" + cssFileName).getAbsolutePath(); |
There was a problem hiding this comment.
I think we misunderstood us ;) Paths.get(JabRefFrame.class.getResource(cssFileName).toURI())
This is what I wanted to point to
tobiasdiez
left a comment
There was a problem hiding this comment.
Thanks for your PR. The code looks good to me. I've only one remark about the priority of preferences versus system property. Apart from that +1 for merge.
Currently, the build fails. Please have a look at the report and fix the checkstyle issues mentioned: https://travis-ci.org/JabRef/jabref/jobs/442113462
| public ThemeLoader(FileUpdateMonitor fileUpdateMonitor, JabRefPreferences jabRefPreferences) throws JabRefException { | ||
| this.fileUpdateMonitor = Objects.requireNonNull(fileUpdateMonitor); | ||
|
|
||
| String cssFileName = jabRefPreferences.get(JabRefPreferences.FX_THEME); |
There was a problem hiding this comment.
First and foremost CSS_SYSTEM_PROPERTY should be System.getProperty("jabref.theme.css"), if the latter is not empty /null (StringUtil.isBlank) so that one can override the theme from the preference using command line arguments (mainly for developers). The preference should be consulted only if the system property is empty.
Please also rename CSS_SYSTEM_PROPERTY to userCss or something like this.
tobiasdiez
left a comment
There was a problem hiding this comment.
Thanks for the follow-up! I'll merge this now.
Add ability to change theme via settings