Add option to use OS specific colour theme #9263

shafinkamal · 2022-10-18T09:59:58Z

Resolves #8729.

Currently, I have added a new Radio Button (see below) which the user would select so that their JabRef theme corresponds to the theme that their OS is currently. I have achieved that by referencing the jSystemThemeDetector library

However, I am currently at a roadblock and would greatly appreciate some help. If my MacOS Theme is 'Light', I can change it to 'Dark' and within JabRef if I select 'Automatic detection' and then 'Save', JabRef will change from Light to Dark. However when going back the JabRef Preferences, the setting has not stayed at 'Automatic detection' and instead has moved to 'Dark theme'. This means that next time I change from Dark to Light on my MacOS System Preferences, this change won't be reflected in JabRef. I've attached a demo to visually show the behaviour.

Any pointers please? Thank you!!!

Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for UI changes)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

…her based on system settings

…as 'Automatic detection' as the AppearancePreference and so needs to hold this state

Siedlerchr · 2022-10-18T11:17:13Z

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

@@ -2067,7 +2067,8 @@ public AppearancePreferences getAppearancePreferences() {
        appearancePreferences = new AppearancePreferences(
                getBoolean(OVERRIDE_DEFAULT_FONT_SIZE),
                getInt(MAIN_FONT_SIZE),
-                new Theme(get(FX_THEME))
+                new Theme(get(FX_THEME)),
+                false


You will need to add a new constant for the new flag and get the value here : sth like getBoolean(AUTOMATIC_THEME_DETECTION) and then add a new listener so that the value gets stored and read on change

ThiloteE · 2022-10-18T16:42:01Z

To ease organizational workflows I have linked the pull-request to the issue with syntax as described in https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue

Linking a pull request to an issue using a keyword

You can link a pull request to an issue by using a supported keyword in the pull request's description or in a commit message. The pull request must be on the default branch.

close

closes

closed

fix

fixes

fixed

resolve

resolves

resolved

If you use a keyword to reference a pull request comment in another pull request, the pull requests will be linked. Merging the referencing pull request also closes the referenced pull request.

The syntax for closing keywords depends on whether the issue is in the same repository as the pull request.

…actoring + commenting

Bumps [gittools/actions](https://github.com/gittools/actions) from 0.9.13 to 0.9.14. - [Release notes](https://github.com/gittools/actions/releases) - [Commits](GitTools/actions@v0.9.13...v0.9.14) --- updated-dependencies: - dependency-name: gittools/actions dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [hmarr/auto-approve-action](https://github.com/hmarr/auto-approve-action) from 2.4.0 to 3.0.0. - [Release notes](https://github.com/hmarr/auto-approve-action/releases) - [Commits](hmarr/auto-approve-action@v2.4.0...v3.0.0) --- updated-dependencies: - dependency-name: hmarr/auto-approve-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [actions/configure-pages](https://github.com/actions/configure-pages) from 1 to 2. - [Release notes](https://github.com/actions/configure-pages/releases) - [Commits](actions/configure-pages@v1...v2) --- updated-dependencies: - dependency-name: actions/configure-pages dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Bumps [controlsfx](https://github.com/controlsfx/controlsfx) from 11.1.1 to 11.1.2. - [Release notes](https://github.com/controlsfx/controlsfx/releases) - [Commits](controlsfx/controlsfx@11.1.1...11.1.2) --- updated-dependencies: - dependency-name: org.controlsfx:controlsfx dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

)

* Save files again relative to bib file * Reworded * Reintroduced default file chooser directory without swing * Update CHANGELOG.md * Update CHANGELOG.md * Update NativeDesktop.java * Reworked ADR * Added mac directory * Removed comments * Added linux support * Revert "Save files again relative to bib file" This reverts commit 3e640f5. # Conflicts: # CHANGELOG.md # src/main/java/org/jabref/preferences/FilePreferences.java # src/main/java/org/jabref/preferences/JabRefPreferences.java # src/test/java/org/jabref/model/database/BibDatabaseContextTest.java * Update src/main/java/org/jabref/gui/desktop/os/Windows.java * fix merge errors * fix wrong method call * remove defautl fix checkstyle * reintroduce method to avoid calling get two times * Introduced getPath method * Update src/main/java/org/jabref/gui/desktop/os/Linux.java fix env var Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com> Co-authored-by: Siedlerchr <siedlerkiller@gmail.com>

shafinkamal · 2022-10-19T10:13:12Z

Updated behaviour.

…her based on system settings

shafinkamal · 2022-10-19T13:27:49Z

I've just noticed that I (at some point) pulled from upstream when in my remote branch accidentally which is why there are other people's contributions in this PR. Will this cause a problem? If so, let me know and I'll fix it up tomorrow.

Thanks (sorry about that)

Siedlerchr · 2022-10-19T18:24:04Z

@shafinkamal That's no problem, but you will probably need to sync your fork's main branch so that github correclty detecs the changes again

On your fork in github just use the sync fork e.g.

Thanks for your PR so far, it's overall looking good. But unfortunately the new dependency is blocked by https://bugs.openjdk.org/browse/JDK-8240567

…her based on system settings

shafinkamal · 2022-10-21T06:20:42Z

Anything I can do to fix the failing tests or is it all good on my end? the fails seem to be related to servers and what not...

Anyways, let me know.

Thanks!

Siedlerchr · 2022-10-21T07:06:41Z

Thanks it looks good, the failing tests are related to some jitpack error not resolving the dependency correctly.

calixtus · 2022-10-21T21:00:36Z

U sure this works with modern verions of windows too, because it looks like microsoft removed some registry keys:
https://learn.microsoft.com/en-us/answers/questions/715081/how-to-detect-windows-dark-mode.html
?

shafinkamal · 2022-10-21T21:11:28Z

I'm not too sure about the modern versions of Windows as I have geared this towards MacOS, because the issue (#8729) is specific to MacOS and I have MacOS so I can manually test it on modern versions of that.

I had included some Windows operations for the sake of coverage but perhaps another issue can be made to ensure further coverage of Windows (and other OS's). In the issue discussion, @Siedlerchr also mentions that a complete implementation would have to be done individually for each OS.

If we're worried about the code breaking the application on the more recent versions of Windows, we can comment out the Windows section and make this a purely MacOS specific feature (for the time being).

calixtus · 2022-10-21T21:16:55Z

src/main/java/org/jabref/gui/theme/SystemThemeDetector.java

+    public final boolean isDarkMode() {
+        switch (getOperatingSystem()) {
+            case WINDOWS: return isWindowsDarkMode();
+            case MACOS: return isMacOsDarkMode();
+            // currently theme detection not implemented for Linux and Solaris
+            case LINUX: return false;
+            case SOLARIS: return false;
+            default: return false;
+        }
+    }


Can you please split up this class and use the existing OS package and classes to provide a method isDarkMode instead of introducing some code duplication to test for which os is runnning here?

calixtus · 2022-10-21T21:20:02Z

src/main/java/org/jabref/gui/theme/SystemThemeDetector.java

+            Process process = Runtime.getRuntime().exec(DARK_THEME_CMD);
+            StreamReader reader = new StreamReader(process.getInputStream());


You really should use JNA here, we already use it in other occasions and its a way cleaner solution.

calixtus · 2022-10-21T21:25:16Z

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

        this.shouldOverrideDefaultFontSize = new SimpleBooleanProperty(shouldOverrideDefaultFontSize);
        this.mainFontSize = new SimpleIntegerProperty(mainFontSize);
        this.theme = new SimpleObjectProperty<>(theme);
+        this.automaticThemeDetectionFlag = new SimpleBooleanProperty(automaticThemeDetectionFlag);


(should)AutoDetectTheme(Property)

calixtus · 2022-10-21T21:26:49Z

src/main/java/org/jabref/gui/preferences/appearance/AppearanceTab.fxml

+    <Label fx:id="restartJabRefHint" styleClass="text" text="${'\nRestarting JabRef may be required to display changes in theme preferences.'}">
+        <font>
+            <Font name="Italic" size="13.0"/>
+        </font>
+    </Label>


Why just fixed the restart for theme change issue a few months back. This would be a major regression.

calixtus · 2022-10-21T21:28:18Z

Thanks for your interest in JabRef programming. I like the idea, yet there are some issues with you changes that should be adressed...

calixtus · 2022-10-21T21:30:52Z

Make the auto detect option a checkbox and grey out the other options, when checked.

shafinkamal · 2022-10-22T00:04:12Z

Hi @calixtus,

Thanks for the feedback. I'm quite busy this weekend and the following week, but I'll get around to implementing these changes when I have some more free time.

In the meantime, could you please assign me to issue #8729?

Thanks!

* upstream/main: (55 commits) Update guidelines-for-setting-up-a-local-workspace.md Add link to good first issues (and fix link) Refine guidelines (and fix .md file linking) Extend ArXiv fetcher results by using data from related DOIs (JabRef#9170) New Crowdin updates (JabRef#9366) Fix token New translations JabRef_en.properties (Chinese Simplified) (JabRef#9364) Fix date field change causes an uncaught exception (JabRef#9365) Add date format and respective test case (JabRef#9310) Bump jackson-datatype-jsr310 from 2.13.4 to 2.14.0 (JabRef#9357) Bump jackson-dataformat-yaml from 2.13.4 to 2.14.0 (JabRef#9356) Bump tika-core from 2.5.0 to 2.6.0 (JabRef#9358) Refine guideline "Set up a local workspace" (JabRef#9355) Adds missing "requires" statement (JabRef#9354) Bind "Find unlinked files" default directory to the current library (JabRef#9290) Rename "Remote" to "Tele" (JabRef#9270) Fixed display of file field adjusting for the dark theme (JabRef#9343) Update sorting of entries in maintable by special fields immediately (JabRef#9338) New translations JabRef_en.properties (German) (JabRef#9336) Fix Abbreviation for Escaped Ampersand (JabRef#9288) ... # Conflicts: # CHANGELOG.md # src/main/java/org/jabref/preferences/AppearancePreferences.java # src/main/java/org/jabref/preferences/JabRefPreferences.java # src/test/java/org/jabref/gui/theme/ThemeManagerTest.java

HoussemNasri · 2022-12-20T08:46:20Z

Hey @shafinkamal. Do you plan to continue working on this?

* upstream/main: (145 commits) Enable groups drag'n'drop to new library (JabRef#9460) Update MacOS jabrefHost.py to find local installs (JabRef#9487) Fix remember last open valid library with empty new one (JabRef#9489) Observable Preferences R (CitationKeyPatternPreferences) (JabRef#9486) Fixed ZBMathTest and extracted keyWordSeparator (JabRef#9485) New Crowdin updates (JabRef#9483) Add log for ignored excepton (JabRef#9302) Select Library to import into (JabRef#9402) Bump org.eclipse.jgit from 6.3.0.202209071007-r to 6.4.0.202211300538-r (JabRef#9476) Bump com.github.andygoossens.modernizer from 1.6.2 to 1.7.0 (JabRef#9478) Bump mockito-core from 4.9.0 to 4.10.0 (JabRef#9479) Bump checkstyle from 10.4 to 10.5.0 (JabRef#9477) Bump slf4j-api from 2.0.5 to 2.0.6 in /buildSrc (JabRef#9480) Bibtex month not deprecated (JabRef#9404) Show development information\n\n+semver: major Release v5.8 Update external libraries add afterburner fx jabref add jakarta inject fix display name for artifact store Prepare CHANGELOG for release Also trigger on branch arm64mac-release ... # Conflicts: # CHANGELOG.md # src/main/java/org/jabref/preferences/JabRefPreferences.java

Siedlerchr · 2023-01-19T18:11:21Z

Closing this issue due to inactivity 💤
Please reopen the issue with additional information if the problem persists.

- Add "Use System Preference" option under General -> Appearance -> Visual Theme - Add flags to store whether user chose this option - Add SystemThemeDetector.java for auto detect system theme(to do)

* Add basic files and settings followed by PR #9263 - Add "Use System Preference" option under General -> Appearance -> Visual Theme - Add flags to store whether user chose this option - Add SystemThemeDetector.java for auto detect system theme(to do) * Using jSystemThemeDetector to detect system theme - Add jSystemThemeDetector dependency in build.gradle - Implemented detector into codebase - Delete unnecessary java file * Add the "Use System Preference" to the English translation file * Add library document and licence * Add update to CHANGELOG.md * Update checkstyle * Update checkstyle * Update CHANGELOG.md. Former log was put in a released part. * Applied review notes and cleaned up * Update JabRefGUI.java Removed whitespace --------- Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com>

shafinkamal added 4 commits October 17, 2022 23:25

Started initial work on implementing automatic light/dark theme switc…

e9ded9e

…her based on system settings

Added 'Automatic Detection' RadioButton

4e5a163

Working through bug fixes. Current behaviour is that it won't remain …

0db1209

…as 'Automatic detection' as the AppearancePreference and so needs to hold this state

refactoring if conditions

3680dad

Siedlerchr reviewed Oct 18, 2022

View reviewed changes

ThiloteE added Dark Theme preferences labels Oct 18, 2022

ThiloteE changed the title ~~Draft: Fix for issue 8729 -- WIP~~ [WIP] Draft: Fix for issue 8729 - Add option to use OS specific colour theme Oct 18, 2022

shafinkamal and others added 10 commits October 19, 2022 17:59

resolving branch issues.

4d95093

resolving branch issues again.

d17e7af

Live update of theme when OS theme updated now possible + general ref…

d1619f8

…actoring + commenting

Make autosave tick in shared database opening dialog active (JabRef#9258

4b73d9a

)

fixing checkstyle

a5825da

shafinkamal marked this pull request as ready for review October 19, 2022 10:12

shafinkamal changed the title ~~[WIP] Draft: Fix for issue 8729 - Add option to use OS specific colour theme~~ Fix for issue 8729 - Add option to use OS specific colour theme Oct 19, 2022

shafinkamal added 3 commits October 19, 2022 21:20

Started initial work on implementing automatic light/dark theme switc…

2a73cb3

…her based on system settings

added to CHANGELOG.md

43ee443

updated ThemeManager.java

d02deff

shafinkamal and others added 3 commits October 20, 2022 13:03

Merge branch 'JabRef:main' into fix-for-issue-8729

432aec9

Started initial work on implementing automatic light/dark theme switc…

a809ea9

…her based on system settings

Removed jSystemThemeDetector dependency and explore different approach

36d74c6

shafinkamal added 2 commits October 20, 2022 20:29

included @AllowedToUseStandardStreams annotation + rationale

3466ace

included @AllowedToUseStandardStreams annotation + rationale

af033c8

Siedlerchr added status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers hacktoberfest-accepted labels Oct 20, 2022

Siedlerchr requested a review from calixtus October 21, 2022 20:00

calixtus reviewed Oct 21, 2022

View reviewed changes

tobiasdiez changed the title ~~Fix for issue 8729 - Add option to use OS specific colour theme~~ Add option to use OS specific colour theme Oct 25, 2022

tobiasdiez added status: changes required Pull requests that are not yet complete and removed status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Oct 25, 2022

Siedlerchr added 2 commits November 11, 2022 19:51

fix after merge

b0716f7

Siedlerchr added 2 commits December 21, 2022 18:18

add method to os classes

33c97b6

Siedlerchr closed this Jan 19, 2023

tobiasdiez mentioned this pull request Oct 16, 2023

Option to use System Preference for Light/Dark Theme (Win/Lin/OSX) #8729

Closed

EthanYifanJu mentioned this pull request Oct 28, 2023

Use System preference for light dark theme #10593

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add option to use OS specific colour theme #9263

Add option to use OS specific colour theme #9263

shafinkamal commented Oct 18, 2022 •

edited

Loading

Siedlerchr Oct 18, 2022

ThiloteE commented Oct 18, 2022

Linking a pull request to an issue using a keyword

shafinkamal commented Oct 19, 2022

shafinkamal commented Oct 19, 2022

Siedlerchr commented Oct 19, 2022 •

edited

Loading

shafinkamal commented Oct 21, 2022

Siedlerchr commented Oct 21, 2022

calixtus commented Oct 21, 2022

shafinkamal commented Oct 21, 2022 •

edited

Loading

calixtus Oct 21, 2022

calixtus Oct 21, 2022 •

edited

Loading

calixtus Oct 21, 2022

calixtus Oct 21, 2022

calixtus commented Oct 21, 2022

calixtus commented Oct 21, 2022

shafinkamal commented Oct 22, 2022

HoussemNasri commented Dec 20, 2022

Siedlerchr commented Jan 19, 2023

		Process process = Runtime.getRuntime().exec(DARK_THEME_CMD);
		StreamReader reader = new StreamReader(process.getInputStream());

Add option to use OS specific colour theme #9263

Add option to use OS specific colour theme #9263

Conversation

shafinkamal commented Oct 18, 2022 • edited Loading

Siedlerchr Oct 18, 2022

Choose a reason for hiding this comment

ThiloteE commented Oct 18, 2022

Linking a pull request to an issue using a keyword

shafinkamal commented Oct 19, 2022

shafinkamal commented Oct 19, 2022

Siedlerchr commented Oct 19, 2022 • edited Loading

shafinkamal commented Oct 21, 2022

Siedlerchr commented Oct 21, 2022

calixtus commented Oct 21, 2022

shafinkamal commented Oct 21, 2022 • edited Loading

calixtus Oct 21, 2022

Choose a reason for hiding this comment

calixtus Oct 21, 2022 • edited Loading

Choose a reason for hiding this comment

calixtus Oct 21, 2022

Choose a reason for hiding this comment

calixtus Oct 21, 2022

Choose a reason for hiding this comment

calixtus commented Oct 21, 2022

calixtus commented Oct 21, 2022

shafinkamal commented Oct 22, 2022

HoussemNasri commented Dec 20, 2022

Siedlerchr commented Jan 19, 2023

shafinkamal commented Oct 18, 2022 •

edited

Loading

Siedlerchr commented Oct 19, 2022 •

edited

Loading

shafinkamal commented Oct 21, 2022 •

edited

Loading

calixtus Oct 21, 2022 •

edited

Loading