Consistent FX color scheme for JabRef #3839

halirutan · 2018-03-12T15:11:06Z

The PR aims to make the look and feel of JabRef more consistent by providing a well-chosen set of base colors and deriving other colors from it. At the moment, I used a flat and bright scheme with the original JabRef blue as the theme color.

In addition to the changes in the FX CSS files, I provide an additional true type font for the application icons that are not available in the Material Design Icon set. It can be loaded and used in exactly the same way as the Material Design Icons and only small changes to the code where necessary. As soon as we agreed on using this, I can make further adjustments and additions (currently, WinEd is missing) to the icons so that they fit perfectly into our toolbar. The one-colored icons should represent the original icon as much as possible and should be recognizable. The will look like this

For those who would like to help to work on this:

Our base style is modena.css and to make our restyling easy to use and to understand, we should try to redefine the color variables of modena.css as much as possible instead of explicitly introducing and using our own variables. This means, don't use explicit color settings in the definition of styles for a control. If it is a commonly used property e.g. the border color, try to find and overwrite the appropriate modena-variable. If you indeed need a specific color, then re-use the ones defined at the top of Main.css.

Remember that this is unfinished at the moment and I have rather commented out lines than simply deleting it.

Change in CHANGELOG.md described
Tests created for changes
Manually tested changed features in running JabRef
Screenshots added in PR description (for bigger UI changes)
Ensured that the git commit message is a good one
Check documentation status (Issue created for outdated help page at help.jabref.org?)

# Conflicts: # src/main/java/org/jabref/gui/maintable/MainTable.css

… few base colors

…signIcon

# Conflicts: # src/main/java/org/jabref/gui/maintable/MainTable.css

lenhard · 2018-03-17T19:45:37Z

I played around with the UI a little and I like it. I like the fact that the menu and tool bars are a little less color-intense than in the current maintable-beta.

You have my +1 for continuing with this line of work.

In the main toolbar, I changed the "New File" icon because it was an outlined version while Open and Save is a filled version. For the Open icon I would like to have the "folder-open", but that is currently not included in our version. I changed the Add Entry icon to a filled version as well, since it better fits the trash. Unfortunately, there is no filled version of Copy and Paste and the positively filled version of GitHub looks just awful. I changed the Up, Down, and Close icon for the sidepanel and the entry editor to versions that look visually more similar in size. Especially, the up and down was too small. After correspondence with a friend, I changed the default list item before the groups into something not so heavy. The small triangle with its vertical side on the left gives additionally a better impression of the indentation which is the key here. The Add Group icon was changed into a heavier circled version. It now matches the style of the close button of the sidepanel and entry editor. All icons of "sub toolbars" in the entry editor and the side panel have now an equal size of 16px. The preferred height of the side-panel header and the header of the tab-pane was adjusted so that they match and there is no jump between main-table and side-panel anymore. There might be a better way to do this. Finally, the color of the overflow buttons for toolbar and tab-pane was adjusted.

Slightly reducing this color still looks very nice and it gives a better impression when text is marked.

Additionally, an alternative version for Vim was added and slight improvements on other icons.

halirutan · 2018-03-22T03:44:14Z

I made further improvements to the custom icons and they look at least acceptable now if put on the toolbar:

For more information on this view issue #3868. I pulled the latest changes from maintable-beta. I'm giving this free for review now. Note that there are still a lot of commented lines in the code to easily see what I have adjusted. Once we agree on something, I'll clean everything up.

tobiasdiez

Thanks again for your work. I really like the central place for colors, makes it way easier to read and understand the rest of the css file. I have a few remarks, which are mostly "please reactivate this code you commented-out". I think, you can cleanup the code and then we can merge. I'll try to design a dark theme but that may take some while.

tobiasdiez · 2018-03-23T08:59:55Z

src/main/java/org/jabref/gui/IconTheme.java

@@ -60,6 +61,8 @@
    }

    private static InputStream getMaterialDesignIconsStream() {
+        // TODO: The next line loads the additional JabRef-Icons. Most certainly wrong at this place


How is font-loading handled in the jensd.fx.glyphs library? I would follow a similar strategy as them...Another idea is to create a new static method and call it during application initialization.

I refactored this and used the same approach we (and Material Design Icons as well) use. It's now loaded the static initializer of the IconTheme class itself.

tobiasdiez · 2018-03-23T09:03:49Z

src/main/java/org/jabref/gui/IconTheme.java

-        APPLICATION_WINEDT(IconTheme.getImage("winedt")),
+        FILE_OPENOFFICE(JabRefMaterialDesignIcon.OPEN_OFFICE),
+        APPLICATION_EMACS(JabRefMaterialDesignIcon.EMACS),
+        APPLICATION_LYX(JabRefMaterialDesignIcon.LYX),


Are there other places that still use IconTheme.getImage? If not this method can be removed as well as InternalFileIcon.

Refactored this out. I initially left it because maybe someone still needs a pixel icon in future. But since it is really easy now to add SVG icons if we indeed don't find anything appropriate in the existing ones, this can be removed.

tobiasdiez · 2018-03-23T09:05:51Z

src/main/java/org/jabref/gui/Main.css

+    -jr-base: #f0f0f0;
+
+    -jr-white: #ffffff;
+    -jr-gray-00: #f5f5f5;


what's the reason for the 00 suffix?

That's what happens if you still need a lighter gray than -jr-gray-0 :) I fixed it.

tobiasdiez · 2018-03-23T09:08:06Z

src/main/java/org/jabref/gui/Main.css

+    -jr-tooltip-bg: -jr-info;
+    -jr-tooltip-fg: -jr-black;
+
+    /* Consistent size for headers of tab-pane and side-panels*/


Move this down to the end, since right now it appears in the middle of color definitions?

tobiasdiez · 2018-03-23T09:13:55Z

src/main/java/org/jabref/gui/Main.css

-.menu-bar > .container > .menu-button > .label {
-    -fx-padding: 0.41777em 0.41777em 0.41777em 0.41777em; /* 5 5 5 5 */
-}
+/*.menu-bar > .container > .menu-button > .label {*/


What's the reason for uncommenting these menu bar customization? If I remember correctly, they make the menu items a bit bigger to align with the MaterialDesing specifications.

It doesn't look good. Even the default padding is larger than it is in other applications, but it looks OK. Compare these two images that show additionally the header of the mail program. First, as I have it now

And here with the additional padding

The menu items look lost in the menu bar. The spacing is far too large. I really would like to keep the default setting.

Btw, in the colorful version, we have now on maintable-beta, this kind of made sense because it tried to mimic the look of today's web-pages with their very large top menus. If you decide to work on a different scheme, the large padding might be an option again. But in this scheme, it doesn't work and I'd like to keep it simple and unsurprising for the user.

I still like the second screenshot more and find it more touch-friendly. But, well, we just have different tastes and I definitely understand your viewpoint.
Probably every dev should create an own theme and we then vote which should be the default 😸

tobiasdiez · 2018-03-23T09:36:25Z

src/main/java/org/jabref/gui/groups/GroupTree.css

    -fx-table-cell-border-color: transparent; /* hide grid lines */
 }

 .tree-table-cell {
-    -fx-font-size: 105%;
+    /*-fx-font-size: 105%;*/


That's probably personal preference, but I prefer the bigger font size (and smaller for hits).

I'm against this. Although the font is only one pixel larger on my system and the anti-aliasing is also OK, it introduces a very notable font-difference to the main-table where there should be consistency. I would accept if this is wanted by most devs, but from my point of view, it is not OK.

Ok, convinced. The base font size should be the same. But I still think the hit-number can be smaller because this is kind of unimportant info.

By the way, did you change the padding of the tree node items? They have a bit tighter feeling than on the current master/maintable-beta, which probably results not from the difference of font-sizes.

tobiasdiez · 2018-03-23T09:37:29Z

src/main/java/org/jabref/gui/groups/GroupTree.css

 }

 .disclosureNodeColumn {
    -fx-alignment: top-right;
 }

 .tree-table-row-cell:dragOver-bottom {
-    -fx-border-color: #eea82f;
+    -fx-border-color: -jr-yellow;


this should reuse the same active/selected colors as for the main table, right?

Good catch. I introduced a color for it in Main.css and set it to our orange. This looks decent and the indicator is visible when you drag it over a marked entry.

tobiasdiez · 2018-03-23T09:38:46Z

src/main/java/org/jabref/gui/groups/GroupTree.css

-    -fx-background-color: white;
-}
+/*.tree-table-row-cell:selected > .tree-table-cell > .tree-disclosure-node > .arrow {*/
+    /*-fx-background-color: -jr-white;*/


In your screenshot, the arrow is never visible accept on hover because it is also white...

In my tests, the commented section is the color of the small arrow, when the group is selected. This setting is not required anymore as far as I understand it because the arrow has not the same color as the icons and our markup color is bright. It looks currently like this

So for me, the arrow is always visible and looks OK even when the entry is selected.

tobiasdiez · 2018-03-23T09:40:27Z

src/main/java/org/jabref/gui/maintable/MainTable.css

 }

 .table-view .column-header .glyph-icon {
    -fx-alignment: baseline-center;
-    -fx-text-fill: -fx-unimportant;
-    -fx-fill: -fx-unimportant;
+    -fx-text-fill: -jr-icon;


use fx-mid-text instead?

It looks pretty decent to me with the icon color and since the table header entry can be pressed to invoke the sorting action, it made sense to me to indicate this. But this might also a very personal preference.

tobiasdiez · 2018-03-23T09:42:04Z

src/main/java/org/jabref/gui/maintable/MainTable.css

-.table-row-cell:odd {
-    -fx-background: -fx-control-inner-background;
-}
+/*.table-row-cell:odd {*/


Please reactivate this, because otherwise the rows are styled in a (ugly) strip-pattern.

I disagree. This is a usabiltily enhancement to difference between odd and even rows. Makes it much easier to see which entry is in which row. So called "Zebra Striping"
http://uxmovement.com/content/9-design-techniques-for-user-friendly-tables/
https://alistapart.com/article/zebrastripingmoredataforthecase

See also here for some general ideas about desinging data tables:
https://uxdesign.cc/design-better-data-tables-4ecc99d23356

I disagree as well. For the eye, it is so much help if the table has an alternating color. It goes along the same line, why we use fonts with serifs when reading a book. The eye needs guidance.

This was not an accident that I commented it out and introduced the odd/even coloring. Additionally, I made the color difference very, very small so that it is barely noticeable but still has the visual guidance.

Well, I don't want to start a rant about zebras (they are quite beautiful animals), but anyway here is the rant:
First of all, there are indeed a few studies (like this one) which show that zebra strips help users in some cases. One should note that the improvement in performance is relatively small (a few 1-3%) and only when the user is required to move vertically in a row to look for data. It is also still controversial whether the better performance comes from a superior design or just because testers where used to the striped style. On the other hand, zebra strips actually lead to worse performance when the task is to quickly identify a desired row containing given data because the eye takes these lines as fix points instead of scan quickly through the dataset.

So zebra strips may have a place in bigger datasheets where it is required to jump in a specific row and the data spans multiple columns. We have, on the other hand, a very small dataset with like 20 entries on display and 5 or so columns. Moreover, and this is probably the biggest point, a row is highlighted when selected or hovered and thus you get all the benefits of the zebra style without the clean look of the plain table.

Sorry for the rant but this is one of the things where people just follow the general advice in the form "zebra strips help to navigate in data table" without asking what are the boundary conditions for the performance increase and if they apply in the present case. This leads to a lot of documents /blog posts citing this as a always-good style to present data in a table format.

lets ask the other @JabRef/developers on their opinion on this. I currently find it very hard to identify an entry in the list. Especially, if you have maybe some entries in which not all required fields are visible. Compare this to the screenshot in the first post. The version from @halirutan looks much better with the stripes.
This is also the case when you have multiple files in the list view.

I give my support for using zebra stripping.

halirutan

I have reacted to all but 2 things:

Make it possible to adjust the main-menu. I support this but there are more settings than only the colors that need to be adjustable. This should be done carefully when working on a scheme that needs these adjustments.
Making the entry editor background white. I'm a bit unsure if I support this visually, but I'm not generally against it. However, a white background color requires further changes because, with an entry editor that is white, we need a consistent, flat-colored border around each of the used controls. I have too less experience to implement this correctly and it would take me considerable time. Is it possible that someone else looks over this when it is really wanted to make the entry editor white?

halirutan · 2018-03-23T16:31:54Z

src/main/java/org/jabref/gui/groups/GroupTree.css

 }

 .disclosureNodeColumn {
    -fx-alignment: top-right;
 }

 .tree-table-row-cell:dragOver-bottom {
-    -fx-border-color: #eea82f;
+    -fx-border-color: -jr-yellow;


Good catch. I introduced a color for it in Main.css and set it to our orange. This looks decent and the indicator is visible when you drag it over a marked entry.

halirutan · 2018-03-23T16:38:00Z

src/main/java/org/jabref/gui/groups/GroupTree.css

-    -fx-background-color: white;
-}
+/*.tree-table-row-cell:selected > .tree-table-cell > .tree-disclosure-node > .arrow {*/
+    /*-fx-background-color: -jr-white;*/


In my tests, the commented section is the color of the small arrow, when the group is selected. This setting is not required anymore as far as I understand it because the arrow has not the same color as the icons and our markup color is bright. It looks currently like this

So for me, the arrow is always visible and looks OK even when the entry is selected.

halirutan · 2018-03-23T16:44:19Z

src/main/java/org/jabref/gui/maintable/MainTable.css

 }

 .table-view .column-header .glyph-icon {
    -fx-alignment: baseline-center;
-    -fx-text-fill: -fx-unimportant;
-    -fx-fill: -fx-unimportant;
+    -fx-text-fill: -jr-icon;


It looks pretty decent to me with the icon color and since the table header entry can be pressed to invoke the sorting action, it made sense to me to indicate this. But this might also a very personal preference.

halirutan · 2018-03-23T16:48:25Z

src/main/java/org/jabref/gui/maintable/MainTable.css

-.table-row-cell:odd {
-    -fx-background: -fx-control-inner-background;
-}
+/*.table-row-cell:odd {*/


I disagree as well. For the eye, it is so much help if the table has an alternating color. It goes along the same line, why we use fonts with serifs when reading a book. The eye needs guidance.

This was not an accident that I commented it out and introduced the odd/even coloring. Additionally, I made the color difference very, very small so that it is barely noticeable but still has the visual guidance.

halirutan · 2018-03-23T17:12:36Z

src/main/java/org/jabref/gui/Main.css

-    -fx-background-color: -fx-light-background;
-    -fx-border-color: -fx-active-background;
+    /*-fx-background-color: -fx-control-inner-background;*/
+    -fx-border-color: -jr-theme;


OK. It's only a redefinition, because the only thing I set is -fx-accent, but I introduced a jr variable for it.

halirutan · 2018-03-23T17:18:48Z

src/main/java/org/jabref/gui/Main.css

+    -jr-tooltip-bg: -jr-info;
+    -jr-tooltip-fg: -jr-black;
+
+    /* Consistent size for headers of tab-pane and side-panels*/


halirutan · 2018-03-23T17:28:26Z

src/main/java/org/jabref/gui/Main.css

    -fx-opacity: 0.2;
    -fx-background-radius: 0em;
 }

 .scroll-bar:horizontal .thumb,
 .scroll-bar:vertical .thumb {
-    -fx-background-color: derive(#757575, 60%);
+    -fx-background-color: -fx-outer-border;


halirutan · 2018-03-24T00:46:20Z

src/main/java/org/jabref/gui/SidePaneComponent.java

-        container.setCenter(new Label(title));
+//        container.setLeft(graphic);
+        final Label label = new Label(title);
+        label.getStyleClass().add("sidePaneComponentHeader");


…o consistent-fxcolors

tobiasdiez · 2018-03-26T21:39:42Z

I've now merged this PR. We can fine-tune the design later and I'll provide a proposal for a dark theme in a new PR.

…drop * upstream/maintable-beta: (48 commits) Clean unused imports Fix missing icons and wrong package for custom icons Consistent FX color scheme for JabRef (#3839) javafx replacement for file dialog (#3005) Reenable closing of entry preview by pressing Esc (#3883) Load all field editors using ViewLoader Use JabRef icons in FXML Move icon stuff to new package gui.icon Load EntryEditor using new ViewLoader Improve tooltip tests Fix import thread problem Don't use null as parameter in DialogService Make it easier to create FXML dialogs (#3880) update slf4j from 1.8.0-beta1 -> 1.8.0-beta2 New translations JabRef_en.properties (Tagalog) New translations JabRef_en.properties (Italian) New translations Menu_en.properties (Italian) New translations JabRef_en.properties (Indonesian) New translations JabRef_en.properties (Greek) New translations Menu_en.properties (Greek) ... # Conflicts: # src/main/java/org/jabref/gui/actions/CleanupAction.java # src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java # src/main/java/org/jabref/gui/groups/GroupTreeController.java # src/main/java/org/jabref/gui/maintable/MainTable.java # src/main/java/org/jabref/logic/cleanup/MoveFilesCleanup.java # src/main/java/org/jabref/logic/cleanup/RenamePdfCleanup.java # src/test/java/org/jabref/logic/cleanup/MoveFilesCleanupTest.java # src/test/java/org/jabref/logic/cleanup/RenamePdfCleanupTest.java

…gsjavafx * upstream/maintable-beta: (188 commits) Fix checkstyle Exchange Ignore by Disabled (#3912) Remove all @author comments and empty method/class comments Clean unused imports Fix missing icons and wrong package for custom icons Consistent FX color scheme for JabRef (#3839) javafx replacement for file dialog (#3005) Reenable closing of entry preview by pressing Esc (#3883) Load all field editors using ViewLoader Use JabRef icons in FXML Move icon stuff to new package gui.icon Load EntryEditor using new ViewLoader Improve tooltip tests Fix import thread problem Don't use null as parameter in DialogService Make it easier to create FXML dialogs (#3880) update slf4j from 1.8.0-beta1 -> 1.8.0-beta2 New translations JabRef_en.properties (Tagalog) New translations JabRef_en.properties (Italian) New translations Menu_en.properties (Italian) ... # Conflicts: # src/main/java/org/jabref/gui/JabRefFrame.java

halirutan added 9 commits February 21, 2018 05:14

Separate colors

bfe7532

More consistent layout

09705b8

Merge branch 'maintable-beta' into consistent-layout

6a99dd1

# Conflicts: # src/main/java/org/jabref/gui/maintable/MainTable.css

Remove many explicit color settings and make the style dependent on a…

e470bf7

… few base colors

Remove color gradients for controls

22cb08a

Add custom application icons in the same style as existing MaterialDe…

f8bab4a

…signIcon

Merge branch 'maintable-beta' into consistent-fxcolors

a8d5b57

Merge branch 'maintable-beta' into consistent-fxcolors

fdd70ce

# Conflicts: # src/main/java/org/jabref/gui/maintable/MainTable.css

Fix transparent cell borders for main-table

a4cf911

halirutan added 5 commits March 18, 2018 16:09

Merge branch 'maintable-beta' into consistent-fxcolors

20cb036

Merge branch 'maintable-beta' into consistent-fxcolors

aba3dc3

Reduce the brightness of highlighted menu, marked text, etc

7e1439a

Slightly reducing this color still looks very nice and it gives a better impression when text is marked.

Fix icon sizes of the custom TTF

3f41151

halirutan mentioned this pull request Mar 21, 2018

Community feedback: Custom icons for external applications #3868

Closed

halirutan added 2 commits March 22, 2018 04:34

Fix OO and Emacs icon.

2c68d79

Additionally, an alternative version for Vim was added and slight improvements on other icons.

Merge branch 'maintable-beta' into consistent-fxcolors

7b02ca0

halirutan added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 22, 2018

Fix size of Copy and Paste icon to original one.

36303bc

tobiasdiez requested changes Mar 23, 2018

View reviewed changes

halirutan commented Mar 26, 2018

View reviewed changes

halirutan and others added 2 commits March 26, 2018 07:41

Fix several style issues as reaction to the comments of the PR

8ba9dc7

Merge branch 'maintable-beta' of https://github.com/JabRef/jabref int…

97f33d0

…o consistent-fxcolors

tobiasdiez merged commit 6a37f2f into maintable-beta Mar 26, 2018

tobiasdiez deleted the consistent-fxcolors branch March 26, 2018 21:38

koppor mentioned this pull request Sep 30, 2024

Update main table colors #11579

Merged

6 tasks

Consistent FX color scheme for JabRef #3839

Consistent FX color scheme for JabRef #3839

Conversation

halirutan commented Mar 12, 2018

lenhard commented Mar 17, 2018

halirutan commented Mar 22, 2018

tobiasdiez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

halirutan left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasdiez commented Mar 26, 2018

halirutan left a comment •

edited

Loading