Updated Dark Theme UI Values #38
Conversation
|
@bstifle nice start. I like where that's going. |
There was a problem hiding this comment.
These look good - left a few comments.
I also think it'd be good to have:
ui-foreground-hover: $blk-010
ui-foreground-pressed: $blk-020
ui-foreground-dark-hover: $blk-210
ui-foreground-dark-pressed: $blk-220
(obviously can't go "step lighter" from pure white like we do with buttons, but this is a nice bit of feedback for sequential actions of hover/focus, and :active)
There are many UI elements that have hover, and pressed states, and it'd be nice to define those colors here. For instance, alerts and modal close buttons, and dropdown items:
| @@ -466,18 +466,18 @@ | |||
| "ui-red-pressed": "#7c1d13", | |||
| "ui-background": "#f3f3f3", | |||
| "ui-foreground": "#353535", | |||
There was a problem hiding this comment.
Are these correct? Isn't in "light theme" ui-foreground: $blk-000 (#fff) and ui-background: $blk-005 (#f8f8f8)
There was a problem hiding this comment.
correct... same issue. brought this up with @julio8a too.
These were already there... so existing
| $ui-red-pressed-dark: #FF6852; | ||
| $ui-background-dark: $blk-210; | ||
| $ui-foreground-dark: $blk-000; |
There was a problem hiding this comment.
I think the foreground is the wrong variable here - "dark theme" ui-foreground: $blk-200 (#2b2b2b) and ui-background: $blk-210 (#202020)
There was a problem hiding this comment.
The way we are using foreground is for text. Look at light theme foreground and background. That’s how Paul and I initially put it together.
There was a problem hiding this comment.
Yeah, that's just kind of confusing no? I'd expect background to be the ... background and foreground to... be the foreground?
Wouldn't that also limit us to one "named" text color?
I'm not sure "foreground" really means "text color" for anyone, I think it is more associated with surfaces in an app than anything else, and why it'd be nice to supplement with hover and pressed states... cc @paulcpederson for thoughts.
There was a problem hiding this comment.
I'm good to change that variable if it's confusing. Just wanted to create a var for normal body text color. Open to suggestions
There was a problem hiding this comment.
oh, if its normal body text color then we should call it that. it's misleading fo sho. if we are following #555555 for light theme body, then we should go #bfbfbf for dark theme body
There was a problem hiding this comment.
e.g.
Light theme:
#2b2b2b headers
#555555 body
Dark theme:
#ffffff headers
#BFBFBF body
There was a problem hiding this comment.
ui-foreground: $blk-000
ui-foreground-hover: $blk-010
ui-foreground-pressed: $blk-020
ui-text-1: $blk-160
ui-text-2:$blk-180
ui-text-3: $blk-200
ui-foreground-dark: $blk-200
ui-foreground-dark-hover: $blk-210
ui-foreground-dark-pressed: $blk-220
ui-text-dark-1: $blk-060
ui-text-dark-2:$blk-040
ui-text-dark-3: $blk-020
Kind of confusing to have "dark" be light and vice versa, but those are the kind of variables I'd want to use, a range of "surface" colors for default, hover, pressed, and then a few body text colors (or just one if we think three is two many)...
There was a problem hiding this comment.
Yeah, "header / body/ ?" better than levels of 1/2/3 fo sho.
There was a problem hiding this comment.
@macandcheese for this comment:
ui-foreground-hover: $blk-010
ui-foreground-pressed: $blk-020
ui-foreground-dark-hover: $blk-210
ui-foreground-dark-pressed: $blk-220
down and down. love this
|
@kyle-03674 pressed red needs to be lighter. any more saturation and we are getting too dark. red is tricky on dark backgrounds note that light and dark theme button states are different (inverted). eg. how pressed is light on dark theme, and hover is darker than default on dark theme
|
|
@bstifle right - makes sense to go lighter for pressed on dark theme. I was suggesting/asking if your red pressed dark theme color should be even lighter to be more similar to the other hues. like this |
|
and thanks for the comparison :) i think these new dark theme colors feel right in line with light theme |
|
hmm yeah! @kyle-03674 i could go for that on the pressed. @macandcheese @julio8a what do you think? |
|
looks good to me! |
|
I like it! |
|
Thanks Kyle! |
|
Cool, @bstifle should we update the dark pressed red to the hex @kyle-03674 is using in this pull request, or should me merge and open a new one? |
|
@paulcpederson @julio8a @macandcheese @kyle-03674 I updated and pushed a new commit for the pressed red #FF7465. should be ready to go! |
|
@bstifle, first PR! |
|
|
|
noob status: confirmed! |
Updated dark theme colors to new colors. Including the dark theme background and foreground