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

A few more dark mode fixes #1700

Merged
merged 7 commits into from
Mar 8, 2019
Merged

A few more dark mode fixes #1700

merged 7 commits into from
Mar 8, 2019

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Mar 8, 2019

A follow-up to @snide's dark colors update.

Big-ish change

The lightest three colors euiColorDarkShade, euiColorDarkestShade, euiColorFullShade were way too similar that they were indistinguishable from each other (especially on the dark background). I altered the dark and darkest shades to be a tad darker (still using the same hex values from the light theme).

I noticed the difference when looking at subdued text, which didn't look subdued at all. So now it does.

Badges

@snide Can you handle the update of these? You can push directly to this branch.

screen shot 2019-03-07 at 21 54 55 pm

But I did fix the notification badge:

screen shot 2019-03-07 at 22 25 46 pm --> screen shot 2019-03-07 at 22 25 14 pm


Other changes

Tooltips
before
screen shot 2019-03-07 at 23 07 05 pm
after
screen shot 2019-03-07 at 22 34 04 pm

Callout titles
They were just a tad on the saturated side, so I tinted them ever so slightly.

before
screen shot 2019-03-07 at 22 35 12 pm
after
screen shot 2019-03-07 at 22 35 19 pm

Bottom bar

Added a shadow to try to distinguish it from the background ever so slightly.

before
screen shot 2019-03-07 at 22 37 17 pm

after
screen shot 2019-03-07 at 22 38 48 pm

it does also effect light theme
screen shot 2019-03-07 at 22 38 59 pm

Loading chart

before
screen shot 2019-03-07 at 22 50 35 pm

after
screen shot 2019-03-07 at 22 50 46 pm

Sidenav underline color

I fixed the focus/hover underline color of the side nav

before
screen shot 2019-03-07 at 23 15 54 pm

after
screen shot 2019-03-07 at 23 16 10 pm


Still needs attention

Like I said, badges, but also the text colors of accent, warning and danger are soo similar, it's really hard to distinguish. Is it possible to go a littler more yellow with the warning color?

screen shot 2019-03-07 at 22 53 43 pm

@ryankeairns It seems something is not working with the subdued stat color?

screen shot 2019-03-07 at 22 52 30 pm

Checklist

  • [ ] This was checked in mobile
  • [ ] This was checked in IE11
  • This was checked in dark mode
  • [ ] Any props added have proper autodocs
  • [ ] Documentation examples were added
  • A changelog entry exists and is marked appropriately
  • [ ] This was checked for breaking changes and labeled appropriately
  • [ ] Jest tests were updated or added to match the most common scenarios
  • [ ] This was checked against keyboard-only and screenreader scenarios
  • [ ] This required updates to Framer X components

@cchaos cchaos requested review from snide and ryankeairns March 8, 2019 04:17
@ryankeairns
Copy link
Contributor

ryankeairns commented Mar 8, 2019

The changes thus far look great! I'll take a look at EuiStat.

As a future item (beyond this PR), the bottom bar could afford to be even more prominent in dark mode and better aligned with light mode. For example, light mode uses a dark background which provides strong contrast, but then that feels incongruent with dark on dark mode. Perhaps a full color bottom bar would achieve both - better visibility and consistency across themes.

screenshot 2019-03-08 09 47 55

screenshot 2019-03-08 09 48 34

Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

👍 LGTM

Created a one-line PR against your fork that fixes the EuiStat subdued color.
cchaos#17

@snide
Copy link
Contributor

snide commented Mar 8, 2019

Changed the warning color to be more yellow. Also fixed the badges.

image

image

image

Copy link
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

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

OK. Think we're good here.

@cchaos cchaos merged commit 288ddc6 into elastic:master Mar 8, 2019
@cchaos cchaos deleted the dark-fixes branch March 8, 2019 17:55
chandlerprall pushed a commit to chandlerprall/eui that referenced this pull request Mar 11, 2019
chandlerprall added a commit that referenced this pull request Mar 11, 2019
* Update react-datepicker time selector to not _always_ scroll to preSelection time

* Update react-datepicker time selection scroll-into-view onMount logic

* revert props default changes I made for testing

* fix scroll issue

* fix ie issue

* A few more dark mode fixes (#1700)

* 9.2.0

* Updated documentation.

* Make EuiPopover's repositionOnScroll prop optional in TS (#1705)

* Make EuiPopover's repositionOnScroll prop optional in TS

* changelog

* fix range coloring

* Fix scrollTop target

* changelog
Shigawire pushed a commit to Shigawire/eui that referenced this pull request May 10, 2019
* Update react-datepicker time selector to not _always_ scroll to preSelection time

* Update react-datepicker time selection scroll-into-view onMount logic

* revert props default changes I made for testing

* fix scroll issue

* fix ie issue

* A few more dark mode fixes (elastic#1700)

* 9.2.0

* Updated documentation.

* Make EuiPopover's repositionOnScroll prop optional in TS (elastic#1705)

* Make EuiPopover's repositionOnScroll prop optional in TS

* changelog

* fix range coloring

* Fix scrollTop target

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

Successfully merging this pull request may close these issues.

3 participants