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 #4702

Merged
merged 16 commits into from
Sep 15, 2015
Merged

Dark theme #4702

merged 16 commits into from
Sep 15, 2015

Conversation

BigFunger
Copy link
Contributor

Implements the dark theme

Fixes #2906

  1. Moves hard-coded values in less files into named variables
  2. Creates theme switching and adds a dark theme
    • Adds default value to 'theme' config values
    • Adds logic to the application chrome that sets a class on the application node
    • Adds a new variable file and css overrides for the dark theme

Notes: This is a first attempt at the dark theme. I think that the implementation of the theme itself may be a bit heavy handed, as it duplicates a lot of existing css/less. Also, the color decisions are my first attempt and should be easy enough to tweak.

@glaszig
Copy link

glaszig commented Aug 19, 2015

👍 thank you for your effort. this needs to happen.

@tbragin
Copy link
Contributor

tbragin commented Aug 26, 2015

Really well done!

A couple of comments:

  • I was a bit surprised to see the theme only apply to dashboards; I figured it'd apply to other screens as well.
  • I was also surprised that the time picker background wasn't dark, even on the dashboard screen.
  • I don't love the pink color for hyperlinks; I think they are too bright.
  • Some white lines for dividers seem unexpectedly bright as well (e.g. the one at the top dividing the navigation/filter elements from charts, the ones inside the pie charts dividing the slices)

screen shot 2015-08-25 at 11 01 18 pm

screen shot 2015-08-25 at 11 07 46 pm

screen shot 2015-08-25 at 11 07 38 pm

@spalger
Copy link
Contributor

spalger commented Aug 26, 2015

@tbragin we did explore theming everything, the solution was going to be pretty intense to implement and we decided that it was probably smarter to wait until we are ready to do a UX/UI deep-dive.

@tbragin
Copy link
Contributor

tbragin commented Aug 26, 2015

@spalger If our intent is to only darken the Dashboard, let's make it clear in Settings. Right now it says:

theme (Default: bright) 
Application theme. Valid values are "dark" and "bright"

When I changed the setting from "bright" to "dark", I expected my UI to flip to dark. It didn't happen, so I thought something was broken. By luck I eventually clicked to Dashboard and saw that it was dark, but I was about to file a bug otherwise.

Also, can we call the themes "light" and "dark", as we did in Kibana 3? I'm not sure what "bright" is - I would expect it to be hot-pink ;)

cc: @alt74

@spalger
Copy link
Contributor

spalger commented Aug 26, 2015

Hahaha, I totally agree. The config description should definitely be updated.

@rashidkpc
Copy link
Contributor

I'd prefer to add a toggle on the dashboard and save this on a per-dashboard basis. The global setting could be treated as a default, but this needs to be toggle-able adhoc

@BigFunger BigFunger assigned BigFunger and unassigned alt74 Aug 31, 2015
@rashidkpc
Copy link
Contributor

Few other comments:

  • The table is a different color, but nothing else is. Makes the table look like its in a container within the panel container, but everything else is edge-to-edge
  • The spy arrow also looks like it was optimized to blend with the map, but maps are rare on dashboards. I'd leave the spy arrow tab the same color as the panel background

@BigFunger BigFunger assigned lukasolson and unassigned BigFunger Sep 2, 2015
dash: function (savedDashboards, config) {
return savedDashboards.get()
.then(function (dash) {
dash.darkTheme = !!config.get('dashboard:defaultDarkTheme');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be implemented in the savedDashboards service, or specifically in the SavedDashboard constructor See #4702 (comment), this should probably be removed

@alt74
Copy link

alt74 commented Sep 4, 2015

Looks great! We just walked through the entire set up. great job @BigFunger I wish we could change the timeline drawer background from white to dark as well. Maybe next release :) @rashidkpc

@tbragin
Copy link
Contributor

tbragin commented Sep 10, 2015

LGTM! Very nice work, @BigFunger

@spalger
Copy link
Contributor

spalger commented Sep 10, 2015

jenkins, test this

@rashidkpc rashidkpc added v4.2.0 and removed v4.3.0 labels Sep 14, 2015
@BigFunger
Copy link
Contributor Author

Most recent screenshots:

Main Layout
Main Layout

Add Visualization Options
Add Visualization Options

Toggle Theme
Toggle Theme

Add Filter Prompt
Add Filter Prompt

Filter List
Filter List

@BigFunger
Copy link
Contributor Author

@alt74 Please see the above screenshots and advise on colors. Thanks!

@alt74
Copy link

alt74 commented Sep 14, 2015

dark theme
Quick preview of updated dark theme. @rashidkpc please review...

kibana-logobg

I removed the background behind the Kibana logo in the new dark theme. Lot's of grays right next to each other. In the light theme the Discover tab and Logo background almost match.

@BigFunger
Copy link
Contributor Author

Applied what changes that I could out of those specified. The result is below. I can't do anything to elements that are outside of the 'application' node with the functionality as it stands now.
@rashidkpc, comments?

dark-theme-2014-09-14

@BigFunger
Copy link
Contributor Author

Most recent iteration:

brighter panel headings
light-dark-2

darker panel headings
light-dark-3

@BigFunger
Copy link
Contributor Author

After discussion with @alt74
Current Dark Theme

@BigFunger
Copy link
Contributor Author

After discussion with @rashidkpc
Current Dark Theme

@BigFunger
Copy link
Contributor Author

Slightly darker text in panel

Current Dark Theme

@BigFunger
Copy link
Contributor Author

Darker text still

Current Dark Theme

BigFunger added a commit that referenced this pull request Sep 15, 2015
@BigFunger BigFunger merged commit e22009b into elastic:master Sep 15, 2015
@spalger
Copy link
Contributor

spalger commented Sep 15, 2015

🎊 🎊 🎊 🎊 🎊
💃 💃 💃 💃 💃

@hackforfood-uk
Copy link

Hello,

This is just what I am looking for, without sounding silly how do I apply this?

Cheers
Pete.

@tbragin
Copy link
Contributor

tbragin commented Nov 19, 2015

@petegriggs Check the "Dashboard" section in the docs:
https://www.elastic.co/guide/en/kibana/current/dashboard.html#dashboard-getting-started

screen shot 2015-11-19 at 5 57 40 am

@ltagliamonte
Copy link

Is it possible to cherry pick this in Kibana 4.1.3 i can't update Kibana because of the ES version. I'm using AWS ES service.

@davison
Copy link

davison commented Dec 8, 2015

I was about to make exactly the same request as it's going to take us a while to migrate our ES to 2.

Is it "backportable" to 4.1 in some way (even if it means a bit of a hack)?

@spalger
Copy link
Contributor

spalger commented Dec 8, 2015

@ltagliamonte @davison I just did a quick check and it looks like 104 files are conflicting. If you would like to take a stab at backporting the changes though you can get started with:

# checkout the kibana source repo somewhere
git checkout git@github.com:elastic/kibana.git
cd kibana
curl https://patch-diff.githubusercontent.com/raw/elastic/kibana/pull/4702.patch | git apply

edit: added a bit about checking out the repo, if you don't have Kibana setup for development you probably want to check out the dev environment setup guide

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants