Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dark theme #3891

Merged
merged 8 commits into from
Apr 7, 2018
Merged

Dark theme #3891

merged 8 commits into from
Apr 7, 2018

Conversation

tobiasdiez
Copy link
Member

@tobiasdiez tobiasdiez commented Mar 27, 2018

This PR adds a dark theme. Based on the wonderful work of @halirutan, this was really easy and only a handful of colors had to be changed.
image
The highlight color is taken from the icon that won the design contest last year. In the end, the dark theme is more a Spielerei from my side to test how nicely the JavaFX styling works and I definitely think we should provide a light theme as the default.

The dark theme was also a nice test because it revealed a few 1-px wide borders, shadows and other small stuff. Based on these discoveries, I also changed the light theme a bit. These changes are just a proposal and are open for discussion. In my opinion, the light theme still needs a bit of love until it is finished (e.g. I find the side pane should be colored in a darker color to better distinguish it from the main table and the background color of the header looks a bit "dirty".)

image


  • 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?)

@tobiasdiez tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Mar 27, 2018
@halirutan
Copy link
Collaborator

That actually looks great. Especially, I like that you were stubborn enough to make the entry editor light. It really looks good. Here are some points that we definitely should address:

  1. Why is the hover effect for the icons gone? I really liked this one.

  2. The line below the main-table header and the All entries Group header cannot be of different width.

  3. Can we manage anyhow that these two lines are on the same level? This doesn't look good
    img

  4. We need a general decision about the hover/select highlight colors for main-table, group entries, and menu bar. I don't insist on my initial choice of yellow, but the blue tones introduce some issues. First, the gray text and the hover-blue don't work. The contrast is too low
    img
    The same is true for the color markings in the front of the main-table. They won't work together. I am still voting for having a very light hover and slightly darker select color. Especially, in the select state, the text should not be inverted. Even a gray-tone works very well.

  5. The main menu entries hovering is not correct. It changes the color of the text instead of the background. This is unreadable for me when I hover over an entry (when you open a menu, and hover the list, it is correct on the sub-items though)

  6. I am still very much for not using the big circles as row indicators in the groups view. I see now that they can be colored and my alternative choice of a small triangle was too small. How about the PLAY icon. It gives a much better impression on the indent-level and is slightly larger

grp

  1. When I hover a line on the main-table, all the small icons for read-status, print-status, etc are always visible, no matter their state

  2. Why is the "rank" icon in the main-table not simply the little star that we use in the header? This icon is neither flat nor can do I have a clue what it should be.

  3. The hover/select status in the main-table has currently 3 states: hover (light), click (darker), double click (really dark). Why is that?

  4. The bottom bar of the groups-view is white and has a separator at the very bottom.
    img
    The separator doesn't look good and when we stick to having this bottom bar white, it should be clearly separated to another side panel (which has a gray header).

  5. I liked my version of the selected tab-pane entry better, where even the text is colored in the theme color. There is no reason that the little icon in front of each entry editor tab is blue but the text is not. Additionally, hovering the little closing "X" throws a CSS error.

  6. The little date-selector icon in the entry editor does not have theme color. Can we fix this?

  7. Can we ensure that the highlight colors (like for priority, read status, etc) use the colors that are defined with the theme? (I haven't even looked where this is defined).

This is a pretty long list, but it covers most things that I saw.

@stefan-kolb
Copy link
Member

Looks nice 👍

@Siedlerchr
Copy link
Member

I like the yellow highlighting. Regarding the group expand /collapse the play icon is not a good choice as it does not indicate the status. In every Tree control you have such an icon.
It's called caret

@lenhard
Copy link
Member

lenhard commented Mar 28, 2018

Personally, I hate dark themes, but I acknowledge that this is what many users want these days. Offering this theme would certainly be a coolness-boost for JabRef.

How do I switch between themes UI-wise? Switching the css files is not what most users would do. Some dialog in the preferences would be cool (but is not a must).

@halirutan
Copy link
Collaborator

halirutan commented Mar 28, 2018

Some more points that apply only to the light theme:

  1. Right now, we use a grayish tone for all fonts. This is defined in -fx-mid-text-color. It is a bit too bright and should be derive(-jr-base, -70%)
  2. More importantly, we then need to use the same color for the glyphs in the main-table (attachments, priority, etc..)
  3. The same text-color has to be used in the BibTeX code editor.
  4. The menu items change its text-color when I hover them. Currently, the font gets 's lighter which does not make sense. The color should be the same on hover and only the background should be highlighted.
    I have derived (in HSB color space) two colors from the main JabRef color we use as the theme that work more unobtrusively for the tables and the menu
    scr
    The color definitions for these are:
    -jr-theme: #50618F;
    -jr-hover-color: #F0F2F7;
    -jr-select-color: #D4DDF7;
    -jr-accent: -jr-theme;
  1. Using the theme color to highlight the currently selected text box is a bit heavy and too dark. I suggest, we use the brighter -jr-select-color above. It still clearly indicates which box is selected.

@Siedlerchr
Copy link
Member

As you are in fixing some small glitches as well:
Opening the event log produces some css warnings:

javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-fx-info' while resolving lookups for '-fx-text-fill' from rule '*.log' in stylesheet file:/E:/workspace/JabRef/jabref/bin/main/org/jabref/gui/errorconsole/errorconsole.css
Mar 28, 2018 11:00:32 AM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-fx-info' while resolving lookups for '-fx-text-fill' from rule '*.log' in stylesheet file:/E:/workspace/JabRef/jabref/bin/main/org/jabref/gui/errorconsole/errorconsole.css
Mar 28, 2018 11:00:32 AM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-fx-info' while resolving lookups for '-fx-text-fill' from rule '*.log' in stylesheet file:/E:/workspace/JabRef/jabref/bin/main/org/jabref/gui/errorconsole/errorconsole.css
Mar 28, 2018 11:00:33 AM javafx.scene.CssStyleHelper calculateValue
WARNING: Could not resolve '-fx-info' while resolving lookups for '-fx-text-fill' from rule '*.log' in stylesheet file:/E:/workspace/JabRef/jabref/bin/main/org/jabref/gui/errorconsole/errorconsole.css

Base.css:
- Introduce explicit colors for hover/select of table. This needs to be
adjust for dark theme (-jr-hover-color, -jr-select-color)
- Additional color for hovered menu text. No influence on light theme but
might be needed in the dark theme -jr-menu-foreground-active
- A slightly darker gray for main text
- reenabled hover effect for icons
- reenabled theme color for tab labels. If required for dark theme lets
introduce a separate color for it
- fixed tab close button
- in light theme we need a visible border for the split pane. Otherwise e.g.
main table and preview have no visible border.
- removed that a selected entry in table has a different color when the maintable
is not focused.
- fixed hover highlighting for menu

GroupTree.css
- the bottom border is not required, but with the search bar in white,
we need a visible border to the group tree itself

ErrorConsole.css
- fixed the style

EntryEditor.css
- Same color for code area then the rest

Entry Editor code area uses the same font color as the rest of the
@tobiasdiez
Copy link
Member Author

tobiasdiez commented Mar 28, 2018

I made some further changes to the light theme. Biggest discussion point is probably the colored toolbar/menu. If this is white (or very light gray) I find it too boring.
image
I'm quite satisfied with the general look but not completely sold on the colors.

@halirutan you changed the hover color for the menu to be the selected color. Was this on purpose? In general, I don't understand the introduction of the variables -jr-hover-color and -jr-select-color since they seem to be the same as the same variables without the color-suffix.
We should also find a compromise for the group icon. Right now we are constantly reverting it back and forth ;-). I would prefer a slightly smaller filled circle but I'm open for everything as long as the color is clearly visible.

- Make separator in groups panel the same as in entry editor and main table
- Replace shadow effect on hover by button-like outline
- Sightly darker selected and hovered colors
- Color toolbar and menu
- Make focus outline of text fields consistent
@halirutan
Copy link
Collaborator

halirutan commented Mar 29, 2018

@tobiasdiez We should get the general feedback from all developers to decide the main discussion points.

General information

We have reached a state, where UI colors can easily be adjusted. But there are some general points we need to decide. I admit that I have a strong opinion about most of my design choices which is why I'd like to capture some feedback. We will compare specific points of the two versions of the light theme we have right now:

I will ask for feedback on some specific points and give some insight into why I feel my choice is better :)
Each point is formulated so that choice 1 means that my argumentation is better and choice 2 means Tobias version is more sound.

To give feedback, you can use the following Doodle poll

If you click an item it means you vote for choice 1.

Overall look

Which version do you prefer?

  1. Yes, I like Patrick's version better
  2. No, I like Tobias' version better

Toolbar/Menubar coloring

The question is, should the toolbar fit into the rest of the UI or should it stick out by having a color? My vote here is clearly for having it fit into the UI. Color only, where color is necessary because we should aim for an unobtrusive UI. It doesn't mean, that the toolbar must have the same gray-tone as the menubar like in my screenshot. It can stick out by being a bit brighter (which was my initial choice before it was changed in this branch).

My argument against the light blue of Tobias version is that it visibly decreases the contrast of the icons. Using a completely different color will make the UI even more inconsistent.

Tobi's response: The blue in the above screenshot is only an example. It is easy to choose a combination of toolbar icons and background color that has a high enough contrast. Note that the contrast is also a problem when we use a darker gray as a background.

  1. Yes, we only use UI gray tones for the background of the toolbar/menubar
  2. No, we colorize them

Group-View Background

My version uses the same bright background color for the group-view that is used in the main-table and in the editor. My argument for this is that no portion of the UI should draw attention to a region when there is no reason for it. The group-view is certainly not more important than the main-table but making it dark, pushes a lot of visual weight to this side panel.

Tobi's response: The navigation bar of an application should have a different background than the content to give the a visual hierarchy (I read this somewhere in the UI design guidelines of Microsoft, but cannot find the link right now). In our case, the groups determine what is shown in the main table and thus serve as a navigation. Moreover, all apps I've installed use a different color for the navigation: Spotify, OneNote, Outlook, Sublime,....

  1. Yes, the group-view should have the same bright background as other panels too
  2. No, the group-view should be dark

Hover and Select colors

I derived these two colors in the following way: The select entry color has the same hue (color value) than the theme color but is brighter and less saturated. The hover color is the select color but even more desaturated, however, it has the same brightness. From the screenshot, you might believe that the hover color creates too less highlighting but when actually using it, this is not the case.
I'm not sure how Tobias picked his hover color but for me it doesn't fit well into the rest.

The important point here is that I'd like to use the darker select color for hovering over menus and indicate which text-area is selected as well as other things. It gives a consistent look and feel. Opposed to that, I believe the dark theme color that is used in Tobias' version to color the frame of the selected text-area in the Entry Editor is too much.

Tobi's response: I'm not really satisfied with the specific color choice in both screenshot. Patrick's is a bit to light for my taste (especially the focused text box in the entry editor, but also the row hover) and mine acknowledged has the problem of too many colors that don't really harmonize.

  1. Yes, I like Patrick's version better
  2. No, I like Tobias' version better

Group Item Indicator

A small point that is highly discussed. The group item indicator is the little icon that appears in front of each entry in the group view. I don't like the big gray circle. Instead, I like something that helps the user to visually to identify the indentation level. The triangle I used achieves that by being vertical on the left side and giving the user a vertical line to follow. Additionally, it is a bit smaller than the circles.

Tobi's response: I agree that the circle might be a bit to large. However, we should choose some icon that is filled enough so that colored icons can easily be distinguished from gray ones. In Patrick's screenshot you have problems noticing the blue icon, while in mine even the lighter green and purple ones are clearly noticeable.

  1. Yes, I too like the triangles better
  2. No, ugly circles are perfect.

Behavior and consistency of icon buttons

I created a hover effect for all icons to help users identify which things "can be pressed".

hover mp4

It works on all icons in the toolbar, entry editor, group view. It even works on the little close "X" for tabs
and the >> button when the toolbar is too small to show all icons. I would like to have this consistently on all icon buttons if possible.

Tobi's response: I don't know how to create a fancy gif but the hover effect I use is taken from Material Design Specs. See here for a working example.. Since the icon really appears as a small button on hover, it makes it clear to the user that this something to click on. Moreover, the same effect can be used for flat text buttons as well. Moreover, I don't like that the dropshadow of Patrick's solutions blurs the icon so much (especially noticeable for the paste icon).

  1. Yes, this is a good idea
  2. No, it's not a good idea

Let's ping some people

@koppor
@stefan-kolb
@Siedlerchr
@lenhard
@AEgit

@stefan-kolb
Copy link
Member

Answered your poll, but I'm not sure about the hover effect. Need to try this out, tho I don't know where I can do this right now.

@halirutan
Copy link
Collaborator

@stefan-kolb

Need to try this out, tho I don't know where I can do this right now.

Use this branch and go one commit back to ef8c2b0

@tobiasdiez
Copy link
Member Author

I've added a few remarks above as a response to have a more balanced argumentation in both directions. I also want to add that it is helpful if each developer not only answers the poll but also outlines a few points that he likes and dislikes about each solution. In my opinion, most of these points above are not solved really well in both versions.

You can also checkout Patrick's version in the current maintable-beta branch, while mine is here on darkTheme (@stefan-kolb). This is probably better than just judging based on screenshots.

Let's begin the battle 😄

@AEgit
Copy link

AEgit commented Mar 29, 2018

@tobiasdiez and @halirutan : Both of you did a great job, so it was hard to decide. I will give some minor comments below to explain my Doodle Poll choices. Keep in mind that I just decided based on the screenshots, as I currently do not have enough time for proper testing. Furthermore, my decision is solely based on the light theme version. The dark theme looks awesome, but I don't know, whether I would use it during my actual work (maybe yes, maybe not - I really don't know) - anyway, there does not exist a dark version of Patrick's attempt, so I choose to compare just the light themes.

  1. Overall look: I prefer Patrick's version. There appear to be too many different colours in Tobi's version.
  2. Toolbar/Menubar coloring: Same reasoning as for 1). Patrick's version looks less "crowded". But, as Tobi explains this might be different with a different colour scheme and/or if using the dark theme.
  3. Group view background: Very difficult decision. I like both - I think I would prefer Tobi's version, but it is a bit too dark for me (= too much visual weight) so I went with Patrick's. But I could see Tobi's version working, if it is slightly less dark.
  4. Hover and select colors: I think Tobi's version is better as the colours used by Patrick appear too light.
  5. Group item indicator: I agree that the circle is too large and I guess with a deeply nested structure (not shown in the screenshot) it will look worse. To be honest, I also do not like the triangle that much, but I cannot come up with a better icon.
  6. Behaviour and consistency of icon buttons: I like both, so this could go either way. It would be interesting to see, how this works with the dark theme. If Patrick's version works without any adjustments also with the dark theme, I have a very slight inclination for his version. Otherwise Tobi's version is probably better.

Overall both look great, so I will probably be happy with any of the two approaches.

@Siedlerchr
Copy link
Member

I really lke Patricks Version, I am not a big fan of dark themes.

  • Icon Hover: As long as the tooltips are shown I don't care
  • Group: Circles or Triangles in front: I would prefer just indentation, I don't think there is an icon necessary. Would like to see a screenshot
  • Group background: Agree with @AEgit, Here I would prefer that version of tobias from Dark theme #3891 (comment) (e.g. just the typical "control color" background

@stefan-kolb
Copy link
Member

  1. Toolbar/Menubar coloring: Toolbar should not use another color, I'm going with Patrick.
  2. Group view background: I think Tobias version let's the main panel stand out more which is desirable for me as it is the main working area or area of information. I think it is fine if it is a little bit in the background color wise, needn't be that dark tho.
  3. Hover and select colors: Tobias version is better as it is easier to use another color style and also I think the effect of shadows does not stand out enough for the buttons (altho it is pretty)
  4. Group item indicator: Going for Patricks version

@Siedlerchr
Copy link
Member

There is still an odd behavior with the expansion: Xubuntu 16.10 with 2560x1335 (27" screen)

First of all, I would expect that as long as I have space, the icons fitting on the screen are displayed:
Second: If I click on that icon I would suspect that the icons are then moved to the left or shown below the search bar etc.
At least not outside the frame I have never seen that in any application.

Another little odd behaviour: When I click in the search field, it get's focus and expands a few cm to the right. When I then leave the field, the search bar goes back to its original size,

And there is too much space between the icons and the searchbar. The icons should be more closer to the search bar.

oddscreemshot

@Siedlerchr
Copy link
Member

And Here is the screenshot with the search + icons
jabreftoolbariconstowide

@Siedlerchr
Copy link
Member

Another issue which should be treated in both themes:
The arrows and the X in the side pane are missing tooltipps. ( e.g. Move Panel up/down=

@halirutan
Copy link
Collaborator

I made some adjustments that include more prominent select/hover colors, gray group view, and icons additionally change their background when you hover them. I explain a bit further what I did in this video:

https://youtu.be/DJAJyr_6XQY

I won't push it right now, as I had to make changes that will definitely break the dark theme of Tobi.

@tobiasdiez I still don't know how I can test your dark theme! Simply using the Dark.css obviously doesn't work. Can you explain how you included the Dark.css so that it overwrites settings from Base.css?

@stefan-kolb
Copy link
Member

stefan-kolb commented Mar 31, 2018

Nice video 😄
Don't think we need to make the toolbar stand out in any way, as we already have the icons and the hovering animation. Also, while the brownish highlighting color is sound, I personally like the lighter blue color more. Reason: I think highlighting is a useful information but not that important that it needs to stick out that much.

Anyway, our UI really looks great compared to what we have right now. This is a huge step!

halirutan and others added 2 commits April 1, 2018 18:38
Buttons now have a simply hover effect as suggested by Tobi. To make this
work consistently, I *did not* use a color for the effect. Instead,
simple black (or white for the dark-scheme) is used with a high value of
alpha. This works independent of the underlying background by just darkening
the area a bit.

I removed the second hover color and replaced it by alpha darkening as well.

Other small fixes include the toolbar context menu and adjustments to
group view and entry editor.

Finally, I tried to bring the changes to the dark version.
- Slightly increase size of menu (touch-friendlier)
- Make sidebar slightly darker
- Color icons as text in the same place would be colored
@tobiasdiez
Copy link
Member Author

Thanks @halirutan for the update.

I now finished implementing the suggestions and the feedback we got on our two versions. Notably, I changed the sidepane color to a very light gray as suggested by @stefan-kolb and @AEgit.
image

In my opinion this works better than a pure white side bar without being too dark (as my version above was).
image

So, in summary, this version looks really nice in my opinion. For the sake of moving forward, I'll merge this now. Of course, we can improve and change the look and feel in the future again. (For example, I'm not sure if the contrast on the selected row with blue background is good enough; moreover the current icon for the groups is to small to show the color).

We also need to add a preference to switch to the dark theme (as remarked by @lenhard), which for the moment is only possible from the command line. I don't have motivation right now to implement this and leave it for a new PR.

@tobiasdiez tobiasdiez merged commit 439530a into maintable-beta Apr 7, 2018
@tobiasdiez tobiasdiez deleted the darkTheme branch April 7, 2018 10:44
@AEgit
Copy link

AEgit commented Apr 7, 2018

Looks really nice! thumbsup

@halirutan
Copy link
Collaborator

halirutan commented Apr 7, 2018

I changed the sidepane color to a very light gray

Well, it was a light gray before. Now, it is the darkest gray we have in the application and we can no longer distinguish the group header and the entry editor from the contents of the group view. Simply changing the tool-bar color and the group view color to the same value than the menu bar would have solved all issues. It would have given the group view a darker gray and makes it notably different from the headers.

But now that it is merged without discussion, I won't fiddle around any longer.

img

@tobiasdiez
Copy link
Member Author

@halirutan Sorry that I didn't noticed the light gray, but I'm pretty sure that I couldn't distinguish between the sidepane and the maintable in the previous version.

Anyway, the reason of why I merge this PR is because its aim was to add a dark theme and

The dark theme was also a nice test because it revealed a few 1-px wide borders, shadows and other small stuff.

Since this is accomplished now and I got positive feedback on it, I'd merged it. (Similarly, how we merged your recent PR on the subject because it accomplished its goal to use consistent jr- colors everywhere). So this merge should definitely not be interpreted as the hammer marking the endpoint of design discussions. In the contrast, there are still many open points that needs to be fixed and discussed (e.g. every javafx dialog) and in the process of course also other design changes are possible. I'm sorry if my words and action made a different impression.

I really enjoyed your feedback - it lead to a huge improvement over all my proposals so far! I'm really happy that you put so much energy and work in the design and hope to see further improvements coming from you in this regard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants