Let user load customized CSS #6036

nilsstre · 2020-02-27T14:51:50Z

I have added functionality that lets the user import and export CSS files, this pull request is related to issue #5790

I have added a button in the PreferencesDialog that lets the user import a CSS file, the user then has to set the radio button in the Appearance tab -> Visual theme to custom theme and restart for the changes to take effect.

I have also added a button in the PreferencesDialog that lets the user export either the base, the dark or the custom theme (if previously imported) as CSS. The reason for adding this button was that I thought it would be easier for the user to create a custom theme if they could use one of the originals as a base.

I added a third radio button for toggling the custom theme to the Visual theme section in the Appearance tab, the button is disabled if the user has not imported a CSS file

Change in CHANGELOG.md described (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for bigger UI changes)
Checked documentation: Is the information available and up to date? If not: Issue created at https://github.com/JabRef/user-documentation/issues.

New radio and import/export buttons

Dialogue box for selecting which theme to export

JabRef with the root theme CSS property changed to green

JabRef with the root theme CSS property changed to red

closes #5790

… CSS file, started on import functionality

…d of string concatenation

…ve since we don't need it

* Add CSS file type, add button in preferences to import a custom CSS file, started on import functionality * Change so that the log uses format specifiers instead of string concatenation * Add RadioButton for toggling custom theme * Add preference for setting the path to custom CSS theme * Load custom CSS if toggled * Add missing language keys * Remove check if the current theme is applied again, the check is removed since we don't need it * Save path to the custom CSS file in program preferences

… once, disable custom theme radio button i no custom theme has been imported

* Add method for saving theme to file * Add modal for selection witch theme to export as CSS

nilsstre · 2020-02-27T16:43:00Z

Created an issue for the missing documentation: JabRef/user-documentation#254

Also created a pull request that adds the documentation: JabRef/user-documentation#255

nilsstre · 2020-02-27T16:48:39Z

Could you take a look? @tobiasdiez 😃

koppor · 2020-02-27T19:23:44Z

Please ensure that you loaded the correct IntelliJ configuration and the you pressed Ctrl+Alt+O to Optimize the imports. Then everyhting is sorted correctly

 [ant:checkstyle] [ERROR] /home/runner/work/jabref/jabref/src/main/java/org/jabref/gui/preferences/ExportThemeDialog.java:5: Wrong order for 'javafx.beans.property.SimpleStringProperty' import. [ImportOrder]

koppor

Some nitpicks, but looks good otherwise ^^

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

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

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

src/main/resources/l10n/JabRef_en.properties

koppor · 2020-02-27T21:07:37Z

How can I add a CSS? I would have expected an import button next to "Custom theme"

I would also expect an "Export" button there.

"Import theme" and "Export theme" at the lower left corner is not intuitive for me: The context of the "Visual Theme" heading indicates that everything what has to do with the Theme is found there. Just move the buttons here below the radio group.

nilsstre · 2020-02-27T21:09:42Z

How can I add a CSS? I would have expected an import button next to "Custom theme"

I would also expect an "Export" button there.

"Import theme" and "Export theme" at the lower left corner is not intuitive for me: The context of the "Visual Theme" heading indicates that everything what has to do with the Theme is found there. Just move the buttons here below the radio group.

Okay I will move the buttons, I thought that it would be a good idea to group them with the other import/export buttons

nilsstre · 2020-02-27T22:18:01Z

New button design

tobiasdiez · 2020-02-28T07:30:51Z

@koppor what's the use case for the export theme? The way the themes work, you don't need to change an existing theme file but specify only the differences (ie you build a top of the current light theme). Moreover, both source files for the light and dark theme are in this repository...

koppor · 2020-02-28T07:37:52Z

My assumptions are:

This is the only way where I get information about the available CSS classes.
As a user not familiar with GitHub, I am still familiar with text editors. Thus, import/export is an easy way for me to edit the CSS.

koppor · 2020-02-28T07:38:23Z

@nilsstre Are this unused imports real? --> https://app.codacy.com/gh/JabRef/jabref/pullRequest?prid=5083392

calixtus · 2020-02-28T07:45:41Z

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

+            dialogService.showWarningDialogAndWait(Localization.lang("Import CSS"),
+                    Localization.lang("You must restart JabRef for this to come into effect."));


Why don't you deal with this by restartWarnings.add()?

calixtus · 2020-02-28T07:55:27Z

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

+import javafx.scene.control.TableRow;
+import javafx.scene.control.TableView;
+import javafx.scene.control.cell.PropertyValueFactory;
+import javafx.scene.input.KeyCode;
+
+import org.jabref.JabRefException;
+import org.jabref.gui.DialogService;
+import org.jabref.gui.util.BaseDialog;
+import org.jabref.gui.util.FileDialogConfiguration;
+import org.jabref.gui.util.ThemeLoader;
+import org.jabref.logic.l10n.Localization;
+import org.jabref.logic.util.StandardFileType;
+import org.jabref.preferences.JabRefPreferences;
+
+import com.airhacks.afterburner.views.ViewLoader;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ExportThemeDialog extends BaseDialog<Void> {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(ExportThemeDialog.class);
+
+    @FXML
+    private TableView<Theme> table;
+    @FXML
+    private TableColumn<Theme, String> columnName;
+    @FXML
+    private TableColumn<Theme, String> columnPath;
+
+    private JabRefPreferences preferences;
+    private DialogService dialogService;
+
+    public ExportThemeDialog(DialogService dialogService, JabRefPreferences preferences) {
+        this.dialogService = dialogService;
+        this.preferences = preferences;
+
+        ViewLoader
+                .view(this)
+                .load()
+                .setAsDialogPane(this);
+
+        this.setTitle(Localization.lang("Export Theme"));
+    }
+
+    @FXML
+    public void initialize() {
+        columnName.setCellValueFactory(new PropertyValueFactory<>("name"));
+        columnPath.setCellValueFactory(new PropertyValueFactory<>("path"));
+
+        ObservableList<Theme> data =
+                FXCollections.observableArrayList(new Theme("Light theme", ThemeLoader.MAIN_CSS), new Theme("Dark theme", ThemeLoader.DARK_CSS));
+
+        if (!(ThemeLoader.getCustomCss().isBlank())) {
+            data.add(new Theme("Custom theme", ThemeLoader.getCustomCss()));
+        }
+
+        table.setItems(data);
+
+        table.setOnKeyPressed(event -> {
+            TablePosition tablePosition;
+            if (event.getCode().equals(KeyCode.ENTER)) {
+                tablePosition = table.getFocusModel().getFocusedCell();
+                final int row = tablePosition.getRow();
+                ObservableList<Theme> list = table.getItems();
+                Theme theme = list.get(row);
+                exportCSSFile(theme.getPath());
+            }
+        });
+
+        table.setRowFactory(tv -> {
+            TableRow<Theme> row = new TableRow<>();
+            row.setOnMouseClicked(event -> handleSelectedRowEvent(row));
+            return row;
+        });
+    }
+
+    private void handleSelectedRowEvent(TableRow<Theme> row) {
+        if (!row.isEmpty()) {
+            exportCSSFile(row.getItem().getPath());
+        }
+    }
+
+    private void exportCSSFile(String theme) {
+        FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder()
+                .addExtensionFilter(StandardFileType.CSS)
+                .withDefaultExtension(StandardFileType.CSS)
+                .withInitialDirectory(preferences.setLastPreferencesExportPath())
+                .build();
+
+        dialogService.showFileSaveDialog(fileDialogConfiguration)
+                     .ifPresent(exportFile -> {
+                         try {
+                             preferences.exportTheme(exportFile.getFileName(), theme);
+                         } catch (JabRefException ex) {
+                             LOGGER.warn(ex.getMessage(), ex);
+                             dialogService.showErrorDialogAndWait(Localization.lang("Export theme"), ex);
+                         }
+                     });
+    }
+}


This class can be divided according to the mvvm-pattern.
This means: put exportCSSFile, handleSelectedRowEvent and the logic to initialize the list into a ViewModel class und communicate between the View and the ViewModel via Properties. Basic idea is that the ViewModel should be useable, even if no gui is loaded.
Have a look at the implementation of other PreferencesTabs or dialogs.
It's very easy.

calixtus · 2020-02-28T08:04:09Z

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

+    private void writeThemeToFile(Path path, OutputStream os) throws IOException {
+        Files.copy(path, os);
+    }


In this case, writeThemeToFile is just an alias for Files.copy with the exact arguments. Just inline it. "exportTheme" should be clear enough.

tobiasdiez · 2020-02-28T10:22:33Z

@koppor In that case, I would prefer to point users to themes.jabref.org which points to a new repository where we then explain a bit how themes are designed (and maybe let users upload new themes as PRs?). The normal user will not touch the css but might download it from someone else. And for this normal user flow, no export functionality is needed.

tobiasdiez

Thanks a lot for your contribution. Overall the code looks good and I've only a few comments. The are concerned mainly with simplifying the user experience and streamline the code flow.
Concerning the later, the flow of information should be like this: preference dialog sets preference object, theme loader sets correct theme based on preferences. There should be no direct interaction between the preference dialog and the theme loader.

Finally, I'm still of the opinion that the export functionality is not necessary. Instead I would add a sentence like "You can find custom themes and information about how to create them on themes.jabref.org" below the "Custom theme option". On this site we can then give more detailed information on how to create custom themes and have a collection of user-provided ones. I let @koppor decide on this one.

tobiasdiez · 2020-02-28T14:04:48Z

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

 import org.jabref.gui.util.TaskExecutor;
+import org.jabref.logic.l10n.Localization;


These new imports don't seem to be necessary.

tobiasdiez · 2020-02-28T14:10:14Z

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

@@ -29,4 +31,18 @@
    <Label styleClass="sectionHeader" text="%Visual theme"/>
    <RadioButton fx:id="themeLight" text="%Light theme" toggleGroup="$theme"/>
    <RadioButton fx:id="themeDark" text="%Dark theme" toggleGroup="$theme"/>
+    <RadioButton fx:id="customTheme" text="%Custom theme" toggleGroup="$theme"/>
+
+    <Label styleClass="sectionHeader" text="%Import custom theme" />


I would prefer if the "Import custom CSS file" button is directly grouped with the above radio button option (since it doesn't make sense to import a custom theme but don't select it). Moreover, I would add a text field showing the currently selected custom css file. So in summary it should look like "Execute program" under "External programs" (also in the preferences).

Finally, I think "Select/Choose custom theme" is more appropriate. Import suggest that I can delete the file afterwards because JabRef imported it.

tobiasdiez · 2020-02-28T14:22:14Z

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

@@ -38,6 +39,7 @@

    public static final String MAIN_CSS = "Base.css";
    public static final String DARK_CSS = "Dark.css";
+    private static String CUSTOM_CSS = "";


This field is not really required for this class. I would prefer if only the preferences know about the current custom theme (in form of the FX_Theme field).

tobiasdiez · 2020-02-28T14:23:27Z

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

@@ -403,6 +405,8 @@
    private static final int EXPORTER_FILENAME_INDEX = 1;
    private static final int EXPORTER_EXTENSION_INDEX = 2;

+    private static final String PATH_TO_CUSTOM_THEME = "pathToCustomTheme";


Is this field really required? Can we not simply point FX_Theme to the external css file?

koppor · 2020-03-06T10:02:28Z

@nilsstre I finally brought up https://themes.jabref.org. Is it possible for you to address the comments by @tobiasdiez? We would really like to get this in.

koppor

Please include the link to http://themes.jabref.org/ and address the review comments.

tobiasdiez · 2020-04-10T15:10:00Z

@nilsstre Any update on this? Would be nice to gives this PR a final polish. Otherwise we have to close your PR unmerged and your work would be lost - which would be very sad for the time and energy you already spent on this. Thanks!

koppor · 2020-07-07T07:43:14Z

@nilsstre We would really like to see that this features comes into JabRef. May I ask whether you can find some time to finish this PR? It seems to be close to get it done. We have resources to provide timely feedback.

calixtus · 2020-07-30T22:05:47Z

As this PR is abandoned, I'm going to finish this in #6725 and I'm closing this PR.
Thanks for your contribution so far, your work is marked by your name in the git log and you should be mentioned in future releases of JabRef.

nilsstre added 13 commits February 26, 2020 11:45

feat/#1 Add CSS file type, add button in preferences to import custom…

f402fd2

… CSS file, started on import functionality

feat/#1 Change so that the log messages uses format specifiers instea…

7bf2fed

…d of string concatenation

feat/#1 Add RadioButton for toggling custom theme

b27f26d

feat/#1 Add preference for setting path to custom CSS theme

8cb3a50

feat/#1 Load custom CSS if toggled

bd6c561

feat/#1 Add missing language keys

6a5f73c

feat/#1 Remove check if current theme is applied again, check is remo…

b5b56bf

…ve since we don't need it

feat/#1 Save path to custom CSS file in program preferences

99d9768

fix/#5 Add checks so that the theme change notification is only shown…

f2fc2ab

… once, disable custom theme radio button i no custom theme has been imported

Fix merge conflict, fixes #5

582c9d5

Remove added stuff from merge conflict

198be47

Add export current theme JabRef#5790

00eac4e

* Add method for saving theme to file * Add modal for selection witch theme to export as CSS

nilsstre mentioned this pull request Feb 27, 2020

Allow to load customized css #5790

Closed

nilsstre added 2 commits February 27, 2020 16:26

Add missing language lines

9fc3382

Merge branch 'master' into addCSSCustomisation

90fc805

This was referenced Feb 27, 2020

Documentation for loading customised CSS JabRef/user-documentation#254

Open

Add documentation for customising JabRef using CSS, fixes #254 JabRef/user-documentation#255

Closed

nilsstre added 2 commits February 27, 2020 17:55

Add information about import/export of themes, JabRef#5790

4f0db24

Fix CodaCy and checkstyle issues, JabRef#5790

a8feb94

koppor requested changes Feb 27, 2020

View reviewed changes

nilsstre added 2 commits February 27, 2020 21:14

Add fixes from code review, JabRef#5790

8c1e2c2

Remove unused import JabRef#5790

e4a4f68

nilsstre requested a review from koppor February 27, 2020 20:29

Move the import/export buttons to the Appearance tab JabRef#5790

00d8ef6

koppor approved these changes Feb 27, 2020

View reviewed changes

calixtus reviewed Feb 28, 2020

View reviewed changes

tobiasdiez approved these changes Feb 28, 2020

View reviewed changes

koppor force-pushed the master branch 5 times, most recently from b8ef7b7 to 21c6e5e Compare March 4, 2020 17:02

koppor changed the title ~~Let user load customized css~~ [WIP] Let user load customized CSS Mar 6, 2020

koppor requested changes Mar 17, 2020

View reviewed changes

tobiasdiez mentioned this pull request Mar 28, 2020

Allow user to use system theme #6195

Closed

calixtus added the status: devcall label Apr 11, 2020

koppor removed the status: devcall label Apr 14, 2020

tobiasdiez marked this pull request as draft April 17, 2020 15:57

koppor changed the title ~~[WIP] Let user load customized CSS~~ Let user load customized CSS Apr 23, 2020

koppor mentioned this pull request Jul 7, 2020

Fonts problem of JabRef 5.1 #6645

Closed

calixtus mentioned this pull request Jul 30, 2020

Add CSS Customisation #6725

Merged

5 tasks

calixtus closed this Jul 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Let user load customized CSS #6036

Let user load customized CSS #6036

nilsstre commented Feb 27, 2020 •

edited by calixtus

Loading

nilsstre commented Feb 27, 2020

nilsstre commented Feb 27, 2020 •

edited

Loading

koppor commented Feb 27, 2020

koppor left a comment

koppor commented Feb 27, 2020

nilsstre commented Feb 27, 2020 •

edited

Loading

nilsstre commented Feb 27, 2020

tobiasdiez commented Feb 28, 2020

koppor commented Feb 28, 2020

koppor commented Feb 28, 2020

calixtus Feb 28, 2020

calixtus Feb 28, 2020

calixtus Feb 28, 2020

tobiasdiez commented Feb 28, 2020

tobiasdiez left a comment •

edited

Loading

tobiasdiez Feb 28, 2020

tobiasdiez Feb 28, 2020

tobiasdiez Feb 28, 2020

tobiasdiez Feb 28, 2020

koppor commented Mar 6, 2020

koppor left a comment

tobiasdiez commented Apr 10, 2020

koppor commented Jul 7, 2020

calixtus commented Jul 30, 2020

		dialogService.showWarningDialogAndWait(Localization.lang("Import CSS"),
		Localization.lang("You must restart JabRef for this to come into effect."));

		import org.jabref.gui.util.TaskExecutor;
		import org.jabref.logic.l10n.Localization;

Let user load customized CSS #6036

Let user load customized CSS #6036

Conversation

nilsstre commented Feb 27, 2020 • edited by calixtus Loading

nilsstre commented Feb 27, 2020

nilsstre commented Feb 27, 2020 • edited Loading

koppor commented Feb 27, 2020

koppor left a comment

Choose a reason for hiding this comment

koppor commented Feb 27, 2020

nilsstre commented Feb 27, 2020 • edited Loading

nilsstre commented Feb 27, 2020

tobiasdiez commented Feb 28, 2020

koppor commented Feb 28, 2020

koppor commented Feb 28, 2020

calixtus Feb 28, 2020

Choose a reason for hiding this comment

calixtus Feb 28, 2020

Choose a reason for hiding this comment

calixtus Feb 28, 2020

Choose a reason for hiding this comment

tobiasdiez commented Feb 28, 2020

tobiasdiez left a comment • edited Loading

Choose a reason for hiding this comment

tobiasdiez Feb 28, 2020

Choose a reason for hiding this comment

tobiasdiez Feb 28, 2020

Choose a reason for hiding this comment

tobiasdiez Feb 28, 2020

Choose a reason for hiding this comment

tobiasdiez Feb 28, 2020

Choose a reason for hiding this comment

koppor commented Mar 6, 2020

koppor left a comment

Choose a reason for hiding this comment

tobiasdiez commented Apr 10, 2020

koppor commented Jul 7, 2020

calixtus commented Jul 30, 2020

nilsstre commented Feb 27, 2020 •

edited by calixtus

Loading

nilsstre commented Feb 27, 2020 •

edited

Loading

nilsstre commented Feb 27, 2020 •

edited

Loading

tobiasdiez left a comment •

edited

Loading