Live reloading when switching themes

src/main/java/org/jabref/gui/preferences/AppearanceTabViewModel.java

@@ -39,7 +39,6 @@ public class AppearanceTabViewModel implements PreferenceTabViewModel {
 
     private final DialogService dialogService;
     private final PreferencesService preferences;
-    private final AppearancePreferences initialAppearancePreferences;
 
     private final Validator fontSizeValidator;
     private final Validator customPathToThemeValidator;
@@ -49,7 +48,6 @@ public class AppearanceTabViewModel implements PreferenceTabViewModel {
     public AppearanceTabViewModel(DialogService dialogService, PreferencesService preferences) {
         this.dialogService = dialogService;
         this.preferences = preferences;
-        this.initialAppearancePreferences = preferences.getAppearancePreferences();
 
         fontSizeValidator = new FunctionBasedValidator<>(
                 fontSizeProperty,
@@ -76,10 +74,11 @@ public AppearanceTabViewModel(DialogService dialogService, PreferencesService pr
 
     @Override
     public void setValues() {
-        fontOverrideProperty.setValue(initialAppearancePreferences.shouldOverrideDefaultFontSize());
-        fontSizeProperty.setValue(String.valueOf(initialAppearancePreferences.getMainFontSize()));
+        AppearancePreferences appearancePreferences = preferences.getAppearancePreferences();
+        fontOverrideProperty.setValue(appearancePreferences.shouldOverrideDefaultFontSize());
+        fontSizeProperty.setValue(String.valueOf(appearancePreferences.getMainFontSize()));
 
-        ThemePreference currentTheme = initialAppearancePreferences.getThemePreference();
+        ThemePreference currentTheme = appearancePreferences.getThemePreference();
         if (currentTheme.getType() == Theme.Type.LIGHT) {
             themeLightProperty.setValue(true);
             themeDarkProperty.setValue(false);
@@ -98,24 +97,20 @@ public void setValues() {
 
     @Override
     public void storeSettings() {
-        if (initialAppearancePreferences.shouldOverrideDefaultFontSize() != fontOverrideProperty.getValue()) {
+
+        AppearancePreferences currentPreferences = preferences.getAppearancePreferences();
+        if (currentPreferences.shouldOverrideDefaultFontSize() != fontOverrideProperty.getValue()) {
             restartWarnings.add(Localization.lang("Override font settings"));
         }
 
         int newFontSize = Integer.parseInt(fontSizeProperty.getValue());
-        if (initialAppearancePreferences.getMainFontSize() != newFontSize) {
-            restartWarnings.add(Localization.lang("Override font size"));
-        }
 
-        ThemePreference newTheme = initialAppearancePreferences.getThemePreference();
-        if (themeLightProperty.getValue() && initialAppearancePreferences.getThemePreference().getType() != Theme.Type.LIGHT) {
+        ThemePreference newTheme = currentPreferences.getThemePreference();
+        if (themeLightProperty.getValue()) {
             newTheme = ThemePreference.light();
-        } else if (themeDarkProperty.getValue() && initialAppearancePreferences.getThemePreference().getType() != Theme.Type.DARK) {
+        } else if (themeDarkProperty.getValue()) {
             newTheme = ThemePreference.dark();
-        } else if (themeCustomProperty.getValue() &&
-                (!initialAppearancePreferences.getThemePreference().getName()
-                                              .equalsIgnoreCase(customPathToThemeProperty.getValue())
-                        || initialAppearancePreferences.getThemePreference().getType() != Theme.Type.CUSTOM)) {
+        } else if (themeCustomProperty.getValue()) {
             newTheme = ThemePreference.custom(customPathToThemeProperty.getValue());
         }
 

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

@@ -4,6 +4,7 @@
 import java.nio.file.Path;
 import java.util.Collections;
 import java.util.List;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.WeakHashMap;
@@ -16,7 +17,6 @@
 import org.jabref.gui.util.Theme;
 import org.jabref.model.util.FileUpdateMonitor;
 import org.jabref.preferences.AppearancePreferences;
-import org.jabref.preferences.PreferencesService;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -38,51 +38,20 @@ public class ThemeImpl implements Theme {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(ThemeImpl.class);
 
-    private final PreferencesService preferencesService;
-
     private final FileUpdateMonitor fileUpdateMonitor;
 
     private final Consumer<Runnable> updateRunner;
 
     private final StyleSheet baseStyleSheet;
 
-    private final AtomicReference<ThemeState> state = new AtomicReference<>();
+    private final AtomicReference<AppearancePreferences> appearancePreferences = new AtomicReference<>();
 
     private final Set<Scene> scenes = Collections.newSetFromMap(new WeakHashMap<>());
     private final Set<WebEngine> webEngines = Collections.newSetFromMap(new WeakHashMap<>());
 
-    private static final class ThemeState {
-
-        private final ThemePreference themePreference;
-
-        private final StyleSheet additionalStylesheet;
-
-        public ThemeState(ThemePreference themePreference) {
-            this.themePreference = themePreference;
-            this.additionalStylesheet = switch (themePreference.getType()) {
-                case LIGHT -> StyleSheetEmpty.EMPTY;
-                case DARK -> StyleSheet.create("Dark.css");
-                case CUSTOM -> StyleSheet.create(themePreference.getName());
-            };
-        }
-    }
-
-    public ThemeImpl(ThemePreference themePreference, PreferencesService preferencesService, FileUpdateMonitor fileUpdateMonitor, Consumer<Runnable> updateRunner) {
-        if (themePreference == null) {
-            throw new IllegalArgumentException("Theme preference required");
-        }
-        if (preferencesService == null) {
-            throw new IllegalArgumentException("Preference service required");
-        }
-        if (fileUpdateMonitor == null) {
-            throw new IllegalArgumentException("File update monitor required");
-        }
-        if (updateRunner == null) {
-            throw new IllegalArgumentException("Update runner required");
-        }
-        this.preferencesService = preferencesService;
-        this.fileUpdateMonitor = fileUpdateMonitor;
-        this.updateRunner = updateRunner;
+    public ThemeImpl(AppearancePreferences initialAppearance, FileUpdateMonitor fileUpdateMonitor, Consumer<Runnable> updateRunner) {
+        this.fileUpdateMonitor = Objects.requireNonNull(fileUpdateMonitor);
+        this.updateRunner = Objects.requireNonNull(updateRunner);
 
         this.baseStyleSheet = StyleSheet.create(BASE_CSS);
 
@@ -98,34 +67,34 @@ public ThemeImpl(ThemePreference themePreference, PreferencesService preferences
             }
         }
 
-        baseCssLiveUpdate();
-        updateThemePreference(themePreference);
+        updateAppearancePreferences(Objects.requireNonNull(initialAppearance));
+    }
+
+    private StyleSheet additionalStylesheet() {
+        return appearancePreferences.get().getThemePreference().getAdditionalStylesheet();
     }
 
     @Override
-    public void updateThemePreference(ThemePreference themePreference) {
+    public void updateAppearancePreferences(AppearancePreferences newPreferences) {
 
-        if (themePreference == null) {
+        if (newPreferences == null) {
             throw new IllegalArgumentException("Theme preference required");
         }
 
-        ThemeState oldState = this.state.get();
-        if (oldState != null) {
-
-            if (oldState.themePreference.equals(themePreference)) {
-                LOGGER.info("Not updating theme preference because it hasn't changed");
-                return;
-            }
+        AppearancePreferences oldPreferences = this.appearancePreferences.get();
+        if (oldPreferences != null) {
+            if (!newPreferences.equals(oldPreferences)) {
+                LOGGER.info("Not updating appearance preferences because it hasn't changed");
 
-            Path oldPath = oldState.additionalStylesheet.getWatchPath();
-            if (oldPath != null) {
-                fileUpdateMonitor.removeListener(oldPath, this::additionalCssLiveUpdate);
-                LOGGER.info("No longer watch css {} for live updates", oldPath);
+                Path oldPath = oldPreferences.getThemePreference().getAdditionalStylesheet().getWatchPath();
+                if (oldPath != null) {
+                    fileUpdateMonitor.removeListener(oldPath, this::additionalCssLiveUpdate);
+                    LOGGER.info("No longer watch css {} for live updates", oldPath);
+                }
             }
         }
-        ThemeState newState = new ThemeState(themePreference);
-        Path newPath = newState.additionalStylesheet.getWatchPath();
-        this.state.set(newState);
+        Path newPath = newPreferences.getThemePreference().getAdditionalStylesheet().getWatchPath();
+        this.appearancePreferences.set(newPreferences);
 
         if (newPath != null) {
             try {
@@ -139,28 +108,24 @@ public void updateThemePreference(ThemePreference themePreference) {
         additionalCssLiveUpdate();
 
         LOGGER.info("Theme set to {} with base css {} and additional css {}",
-                themePreference, baseStyleSheet, state.get().additionalStylesheet);
+                newPreferences.getThemePreference(), baseStyleSheet, additionalStylesheet());
     }
 
     @Override
     public void installCss(Scene scene) {
-        AppearancePreferences appearancePreferences = preferencesService.getAppearancePreferences();
         updateRunner.accept(() -> {
             if (this.scenes.add(scene)) {
                 updateBaseCss(scene);
                 updateAdditionalCss(scene);
             }
-            if (appearancePreferences.shouldOverrideDefaultFontSize()) {
-                scene.getRoot().setStyle("-fx-font-size: " + appearancePreferences.getMainFontSize() + "pt;");
-            }
         });
     }
 
     @Override
     public void installCss(WebEngine webEngine) {
         updateRunner.accept(() -> {
             if (this.webEngines.add(webEngine)) {
-                webEngine.setUserStyleSheetLocation(state.get().additionalStylesheet.getWebEngineStylesheet());
+                webEngine.setUserStyleSheetLocation(additionalStylesheet().getWebEngineStylesheet());
             }
         });
     }
@@ -172,9 +137,9 @@ private void baseCssLiveUpdate() {
     }
 
     private void additionalCssLiveUpdate() {
-        StyleSheet styleSheet = state.get().additionalStylesheet;
+        StyleSheet styleSheet = additionalStylesheet();
         styleSheet.reload();
-        String newStyleSheetLocation = state.get().additionalStylesheet.getWebEngineStylesheet();
+        String newStyleSheetLocation = styleSheet.getWebEngineStylesheet();
 
         LOGGER.debug("Updating additional CSS for {} scenes and {} web engines", scenes.size(), webEngines.size());
         updateRunner.accept(() -> {
@@ -199,21 +164,29 @@ private void updateBaseCss(Scene scene) {
     }
 
     private void updateAdditionalCss(Scene scene) {
+        AppearancePreferences appearance = this.appearancePreferences.get();
         List<String> stylesheets = scene.getStylesheets();
         if (stylesheets.size() == 2) {
             stylesheets.remove(1);
         }
-        stylesheets.add(1, state.get().additionalStylesheet.getSceneStylesheet().toExternalForm());
+        stylesheets.add(1,
+                appearance.getThemePreference().getAdditionalStylesheet().getSceneStylesheet().toExternalForm());
+
+        if (appearance.shouldOverrideDefaultFontSize()) {
+            scene.getRoot().setStyle("-fx-font-size: " + appearance.getMainFontSize() + "pt;");
+        } else {
+            scene.getRoot().setStyle("");
+        }
     }
 
     @Override
-    public ThemePreference getPreference() {
-        return state.get().themePreference;
+    public AppearancePreferences getCurrentAppearancePreferences() {
+        return appearancePreferences.get();
     }
 
     @Override
     @Deprecated(forRemoval = true) // TODO ThemeTest needs updating, and should be based on installCss(WebEngine) instead
     public Optional<String> getAdditionalStylesheet() {
-        return Optional.of(state.get().additionalStylesheet.getWebEngineStylesheet());
+        return Optional.of(additionalStylesheet().getWebEngineStylesheet());
     }
 }

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

@@ -13,6 +13,8 @@ public class ThemePreference {
 
     private final String name;
 
+    private final StyleSheet additionalStylesheet;
+
     public ThemePreference(String name) {
         this.name = name != null ? name : "";
         if (StringUtil.isBlank(this.name) || Theme.BASE_CSS.equalsIgnoreCase(this.name)) {
@@ -22,6 +24,11 @@ public ThemePreference(String name) {
         } else {
             this.type = Theme.Type.CUSTOM;
         }
+        this.additionalStylesheet = switch (type) {
+            case LIGHT -> StyleSheetEmpty.EMPTY;
+            case DARK -> StyleSheet.create("Dark.css");
+            case CUSTOM -> StyleSheet.create(name);
+        };
     }
 
     public static ThemePreference light() {
@@ -72,6 +79,10 @@ public int hashCode() {
         return Objects.hash(type, name);
     }
 
+    public StyleSheet getAdditionalStylesheet() {
+        return additionalStylesheet;
+    }
+
     @Override
     public String toString() {
         return "ThemePreference{" +

src/main/java/org/jabref/gui/util/Theme.java

@@ -5,7 +5,7 @@
 import javafx.scene.Scene;
 import javafx.scene.web.WebEngine;
 
-import org.jabref.gui.theme.ThemePreference;
+import org.jabref.preferences.AppearancePreferences;
 
 public interface Theme {
 
@@ -31,9 +31,9 @@ enum Type {
      */
     void installCss(WebEngine webEngine);
 
-    void updateThemePreference(ThemePreference themePreference);
+    void updateAppearancePreferences(AppearancePreferences appearancePreferences);
 
-    ThemePreference getPreference();
+    AppearancePreferences getCurrentAppearancePreferences();
 
     /**
      * This method allows callers to obtain the theme's additional stylesheet.

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

@@ -1,5 +1,7 @@
 package org.jabref.preferences;
 
+import java.util.Objects;
+
 import org.jabref.gui.theme.ThemePreference;
 
 public class AppearancePreferences {
@@ -24,4 +26,23 @@ public int getMainFontSize() {
     public ThemePreference getThemePreference() {
         return themePreference;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        AppearancePreferences that = (AppearancePreferences) o;
+        return shouldOverrideDefaultFontSize == that.shouldOverrideDefaultFontSize &&
+                mainFontSize == that.mainFontSize &&
+                themePreference.equals(that.themePreference);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(shouldOverrideDefaultFontSize, mainFontSize, themePreference);
+    }
 }

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

@@ -1985,25 +1985,29 @@ public void storeMainTableNameFormatPreferences(MainTableNameFormatPreferences p
     // AppearancePreferences
     //*************************************************************************************************************
 
+    private AppearancePreferences createAppearancePreferences() {
+        return new AppearancePreferences(
+                getBoolean(OVERRIDE_DEFAULT_FONT_SIZE),
+                getInt(MAIN_FONT_SIZE),
+                new ThemePreference(get(FX_THEME)));
+    }
+
     @Override
     public Theme getTheme() {
         if (globalTheme == null) {
-            globalTheme = new ThemeImpl(new ThemePreference(get(FX_THEME)), this, getFileUpdateMonitor(), DefaultTaskExecutor::runInJavaFXThread);
+            globalTheme = new ThemeImpl(createAppearancePreferences(), getFileUpdateMonitor(), DefaultTaskExecutor::runInJavaFXThread);
         }
         return globalTheme;
     }
 
     @Override
     public void updateTheme() {
-        getTheme().updateThemePreference(new ThemePreference(get(FX_THEME)));
+        getTheme().updateAppearancePreferences(createAppearancePreferences());
     }
 
     @Override
     public AppearancePreferences getAppearancePreferences() {
-        return new AppearancePreferences(
-                getBoolean(OVERRIDE_DEFAULT_FONT_SIZE),
-                getInt(MAIN_FONT_SIZE),
-                globalTheme.getPreference());
+        return getTheme().getCurrentAppearancePreferences();
     }
 
     @Override