Skip to content

Commit

Permalink
Fix theme switching exception (#8939)
Browse files Browse the repository at this point in the history
  • Loading branch information
@HoussemNasri
HoussemNasri committed Jul 3, 2022
1 parent eb1d7a9 commit 050cd3f
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 41 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We fixed an issue where the exception that there are invalid characters in filename. [#8786](https://github.com/JabRef/jabref/issues/8786)
- When the proxy configuration removed the proxy user/password, this change is applied immediately.
- We fixed an issue where removing several groups deletes only one of them. [#8390](https://github.com/JabRef/jabref/issues/8390)
- We fixed a bug where switching between themes will cause an error/exception. [#8939](https://github.com/JabRef/jabref/pull/8939)

### Removed

Expand Down
10 changes: 0 additions & 10 deletions src/main/java/org/jabref/gui/JabRefDialogService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ public JabRefDialogService(Window mainWindow, Pane mainPane, ThemeManager themeM

private FXDialog createDialog(AlertType type, String title, String content) {
FXDialog alert = new FXDialog(type, title, true);
themeManager.installCss(alert.getDialogPane().getScene());
alert.setHeaderText(null);
alert.setContentText(content);
alert.getDialogPane().setMinHeight(Region.USE_PREF_SIZE);
Expand Down Expand Up @@ -113,7 +112,6 @@ protected Node createDetailsButton() {

// Reset the dialog graphic using the default style
alert.getDialogPane().setGraphic(graphic);
themeManager.installCss(alert.getDialogPane().getScene());
alert.setHeaderText(null);
alert.setContentText(content);
alert.getDialogPane().setMinHeight(Region.USE_PREF_SIZE);
Expand All @@ -138,7 +136,6 @@ public <T> Optional<T> showChoiceDialogAndWait(String title, String content, Str
choiceDialog.setTitle(title);
choiceDialog.setContentText(content);
choiceDialog.initOwner(mainWindow);
themeManager.installCss(choiceDialog.getDialogPane().getScene());
return choiceDialog.showAndWait();
}

Expand All @@ -148,7 +145,6 @@ public Optional<String> showInputDialogAndWait(String title, String content) {
inputDialog.setHeaderText(title);
inputDialog.setContentText(content);
inputDialog.initOwner(mainWindow);
themeManager.installCss(inputDialog.getDialogPane().getScene());
return inputDialog.showAndWait();
}

Expand All @@ -158,7 +154,6 @@ public Optional<String> showInputDialogWithDefaultAndWait(String title, String c
inputDialog.setHeaderText(title);
inputDialog.setContentText(content);
inputDialog.initOwner(mainWindow);
themeManager.installCss(inputDialog.getDialogPane().getScene());
return inputDialog.showAndWait();
}

Expand Down Expand Up @@ -186,7 +181,6 @@ public void showErrorDialogAndWait(String message, Throwable exception) {
exceptionDialog.getDialogPane().setMaxWidth(mainWindow.getWidth() / 2);
exceptionDialog.setHeaderText(message);
exceptionDialog.initOwner(mainWindow);
themeManager.installCss(exceptionDialog.getDialogPane().getScene());
exceptionDialog.showAndWait();
}

Expand All @@ -196,7 +190,6 @@ public void showErrorDialogAndWait(String title, String content, Throwable excep
exceptionDialog.setHeaderText(title);
exceptionDialog.setContentText(content);
exceptionDialog.initOwner(mainWindow);
themeManager.installCss(exceptionDialog.getDialogPane().getScene());
exceptionDialog.showAndWait();
}

Expand Down Expand Up @@ -266,7 +259,6 @@ public Optional<ButtonType> showCustomDialogAndWait(String title, DialogPane con
alert.getDialogPane().setMinHeight(Region.USE_PREF_SIZE);
alert.setResizable(true);
alert.initOwner(mainWindow);
themeManager.installCss(alert.getDialogPane().getScene());
return alert.showAndWait();
}

Expand Down Expand Up @@ -294,7 +286,6 @@ public <V> void showProgressDialog(String title, String content, Task<V> task) {
task.cancel();
progressDialog.close();
});
themeManager.installCss(progressDialog.getDialogPane().getScene());
progressDialog.initOwner(mainWindow);
progressDialog.show();
}
Expand All @@ -319,7 +310,6 @@ public <V> Optional<ButtonType> showBackgroundProgressDialogAndWait(String title
alert.getDialogPane().setMinHeight(Region.USE_PREF_SIZE);
alert.setResizable(true);
alert.initOwner(mainWindow);
themeManager.installCss(alert.getDialogPane().getScene());

stateManager.getAnyTasksThatWillNotBeRecoveredRunning().addListener((observable, oldValue, newValue) -> {
if (!newValue) {
Expand Down
52 changes: 28 additions & 24 deletions src/main/java/org/jabref/gui/theme/ThemeManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.WeakHashMap;
import java.util.function.Consumer;
Expand Down Expand Up @@ -46,7 +47,7 @@ public class ThemeManager {
private final StyleSheet baseStyleSheet;
private Theme theme;

private final Set<Scene> scenes = Collections.newSetFromMap(new WeakHashMap<>());
private Scene mainWindowScene;
private final Set<WebEngine> webEngines = Collections.newSetFromMap(new WeakHashMap<>());

public ThemeManager(AppearancePreferences appearancePreferences,
Expand Down Expand Up @@ -88,8 +89,7 @@ private void updateThemeSettings() {
}

private void updateFontSettings() {
DefaultTaskExecutor.runInJavaFXThread(() ->
updateRunner.accept(() -> scenes.forEach(this::updateFontStyle)));
DefaultTaskExecutor.runInJavaFXThread(() -> updateRunner.accept(() -> getMainWindowScene().ifPresent(this::updateFontStyle)));
}

private void removeStylesheetFromWatchList(StyleSheet styleSheet) {
Expand Down Expand Up @@ -117,12 +117,10 @@ private void baseCssLiveUpdate() {
if (baseStyleSheet.getSceneStylesheet() == null) {
LOGGER.error("Base stylesheet does not exist.");
} else {
LOGGER.debug("Updating base CSS for {} scenes", scenes.size());
LOGGER.debug("Updating base CSS for main window scene");
}

DefaultTaskExecutor.runInJavaFXThread(() ->
updateRunner.accept(() -> scenes.forEach(this::updateBaseCss))
);
DefaultTaskExecutor.runInJavaFXThread(() -> updateRunner.accept(this::updateBaseCss));
}

private void additionalCssLiveUpdate() {
Expand All @@ -131,11 +129,11 @@ private void additionalCssLiveUpdate() {
return styleSheet.getWebEngineStylesheet();
}).orElse("");

LOGGER.debug("Updating additional CSS for {} scenes and {} web engines", scenes.size(), webEngines.size());
LOGGER.debug("Updating additional CSS for main window scene and {} web engines", webEngines.size());

DefaultTaskExecutor.runInJavaFXThread(() ->
updateRunner.accept(() -> {
scenes.forEach(this::updateAdditionalCss);
updateAdditionalCss();

webEngines.forEach(webEngine -> {
// force refresh by unloading style sheet, if the location hasn't changed
Expand All @@ -148,17 +146,19 @@ private void additionalCssLiveUpdate() {
);
}

private void updateBaseCss(Scene scene) {
List<String> stylesheets = scene.getStylesheets();
if (!stylesheets.isEmpty()) {
stylesheets.remove(0);
}
private void updateBaseCss() {
getMainWindowScene().ifPresent(scene -> {
List<String> stylesheets = scene.getStylesheets();
if (!stylesheets.isEmpty()) {
stylesheets.remove(0);
}

stylesheets.add(0, baseStyleSheet.getSceneStylesheet().toExternalForm());
stylesheets.add(0, baseStyleSheet.getSceneStylesheet().toExternalForm());
});
}

private void updateAdditionalCss(Scene scene) {
scene.getStylesheets().setAll(List.of(
private void updateAdditionalCss() {
getMainWindowScene().ifPresent(scene -> scene.getStylesheets().setAll(List.of(
baseStyleSheet.getSceneStylesheet().toExternalForm(),
appearancePreferences.getTheme()
.getAdditionalStylesheet().map(styleSheet -> {
Expand All @@ -170,21 +170,21 @@ private void updateAdditionalCss(Scene scene) {
}
})
.orElse("")
));
)));
}

/**
* Installs the base css file as a stylesheet in the given scene. Changes in the css file lead to a redraw of the
* scene using the new css file.
*
* @param scene the scene to install the css into
* @param mainWindowScene the scene to install the css into
*/
public void installCss(Scene scene) {
public void installCss(Scene mainWindowScene) {
Objects.requireNonNull(mainWindowScene, "scene is required");
updateRunner.accept(() -> {
if (this.scenes.add(scene)) {
updateBaseCss(scene);
updateAdditionalCss(scene);
}
this.mainWindowScene = mainWindowScene;
updateBaseCss();
updateAdditionalCss();
});
}

Expand Down Expand Up @@ -223,4 +223,8 @@ public void updateFontStyle(Scene scene) {
public Theme getActiveTheme() {
return this.theme;
}

public Optional<Scene> getMainWindowScene() {
return Optional.ofNullable(mainWindowScene);
}
}
7 changes: 0 additions & 7 deletions src/main/java/org/jabref/gui/util/BaseDialog.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import java.util.Optional;

import javafx.scene.Node;
import javafx.scene.control.Button;
import javafx.scene.control.ButtonType;
import javafx.scene.control.Dialog;
Expand All @@ -15,8 +14,6 @@
import org.jabref.gui.keyboard.KeyBinding;
import org.jabref.gui.keyboard.KeyBindingRepository;

import com.tobiasdiez.easybind.EasyBind;

public class BaseDialog<T> extends Dialog<T> implements org.jabref.gui.Dialog<T> {

protected BaseDialog() {
Expand All @@ -39,10 +36,6 @@ protected BaseDialog() {

setDialogIcon(IconTheme.getJabRefImage());
setResizable(true);

EasyBind.wrapNullable(dialogPaneProperty())
.mapObservable(Node::sceneProperty)
.subscribeToValues(scene -> Globals.getThemeManager().installCss(scene));
}

private Optional<Button> getDefaultButton() {
Expand Down

0 comments on commit 050cd3f

Please sign in to comment.