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 for side navigation dark #2950

Merged
merged 9 commits into from Mar 27, 2020

Conversation

bartaz
Copy link
Contributor

@bartaz bartaz commented Mar 25, 2020

Done

Adds dark theme for side navigation component.
Updates themes' color variable names to be more consistent.
Updates dark theme colors to use opacity.
Consistently use text color for side navigation active item border (rather than brand color).

Fixes #2863

QA

Screenshot 2020-03-25 at 15 17 46

@bartaz bartaz added the Feature 🎁 New feature or request label Mar 25, 2020
@webteam-app
Copy link

webteam-app commented Mar 25, 2020

scss/_settings_colors.scss Outdated Show resolved Hide resolved
scss/_settings_colors.scss Outdated Show resolved Hide resolved
@bartaz
Copy link
Contributor Author

bartaz commented Mar 26, 2020

@lyubomir-popov Just had a look at percy, and it seems we are not really ready to replace dark theme colors with opacity based ones:
https://percy.io/vanilla-framework/vanilla-framework/builds/4652940

Screenshot 2020-03-26 at 07 39 49

For example, main navigation uses $colors--dark-theme--background-highlighted as background, rather than $colors--dark-theme--background.
So we can't change $colors--dark-theme--background-highlighted to opacity based color, as it needs to be solid dark.

To achieve what we want we would need to be much more consistent in using theme colors across components. And possibly we would need more theme color variables - for solid backgrounds, for transparent backgrounds, for borders with different steps of intensity, etc.

I'm reverting it now, so the opacity based colors will only be used in dark side navigation. But this means dark side navigation will not directly use dark theme colors, but their transparent variants. Which leads to yet another inconsistency in how we deal with themes between components.

@bartaz bartaz changed the title WIP: Dark theme for side navigation dark Dark theme for side navigation dark Mar 26, 2020
@bartaz bartaz marked this pull request as ready for review March 26, 2020 14:33
@bartaz
Copy link
Contributor Author

bartaz commented Mar 26, 2020

@lyubomir-popov

HR dark theme now with opacity based border color feels VERY subtle (barely visible) compared to light theme equivalent:

Screenshot 2020-03-26 at 15 35 04

Screenshot 2020-03-26 at 15 34 42

Copy link
Contributor

@lyubomir-popov lyubomir-popov left a comment

Choose a reason for hiding this comment

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

LGTM

@bartaz bartaz merged commit 069539b into canonical:master Mar 27, 2020
@bartaz bartaz deleted the side-navigation-dark branch March 27, 2020 09:19
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.

Build sidenav dark theme
3 participants