Skip to content
This repository has been archived by the owner on Nov 2, 2021. It is now read-only.

Consistent Dark Mode Scheme #1417

Closed
SensehacK opened this issue Apr 22, 2020 · 18 comments
Closed

Consistent Dark Mode Scheme #1417

SensehacK opened this issue Apr 22, 2020 · 18 comments
Assignees
Labels
Status: Abandoned issue is no longer important to the requestor and no one else has shown an interest in it Status: Completed Nothing further to be done with this issue

Comments

@SensehacK
Copy link
Contributor

SensehacK commented Apr 22, 2020

Is your feature request related to a problem? Please describe.
I love dark mode everything,
Describe the solution you'd like
Would like to propose a consistent dark mode color scheme / palette across the whole web app.
So that some pages would look consistent with the overall color schemes.

Relevant Issues:
#1385
#1416
#1400

Describe alternatives you've considered
Dark Reader extension fairly does great job of dark mode, but isn't officially supported on the web.

Additional context
We could utilize SASS variables for better scalability in terms of web development in future.

@jeremyphilemon jeremyphilemon added Status: Claimed It's clear what the subject of the issue is about, and what the resolution should be and removed Status: Review Needed labels Apr 23, 2020
@jeremyphilemon
Copy link
Collaborator

Go ahead! Let's try to keep discussions about the palette here. For now you can use the palettes that's currently being used and build upon it with a little research from looking at accessibility guides too :)

Resources:

@balrajsingh9
Copy link

balrajsingh9 commented May 7, 2020

Go ahead! Let's try to keep discussions about the palette here. For now you can use the palettes that's currently being used and build upon it with a little research from looking at accessibility guides too :)

Resources:

The current dark theme is great 👍 . Some minor suggestions I would like to posit.

Can we have something similar to what twiter has? It shows three options to the user -

  • Default (Light)
  • Dim
  • Lights out

Screenshot 2020-05-07 at 5 01 38 PM

There are some colors in the palette which are quite difficult to see. I'm comparing the visibility with the one we have in the default (light) theme. #1400

Screenshot 2020-05-07 at 5 09 14 PM

Screenshot 2020-05-07 at 5 09 39 PM

Using dev-tools I quickly changed the color of the element shown in the screenshot above and according to the Material guideline, the medium emphasis text color on a darker surface should be - rgba(255,255,255,.60) or #FFFFFF with 60% alpha.
Screenshot 2020-05-07 at 5 25 37 PM
To follow the guideline, we need to maintain colors in variables and switch according to the currently applied theme vs the common colors that we have in-place right.

So I agree with @SensehacK that we need to use variables to maintain a consistent style and to have better visibility (accessibility) of texts as compared to what we have currently.

Extending the above point, I also played with the font-weight and had an instant improvement when I reduced it and didn't touch the text-color.

Old - font-weight = 900
Screenshot 2020-05-07 at 5 09 39 PM

After reducing to font-weight = 500
Screenshot 2020-05-07 at 5 18 50 PM

Let me know your thoughts @jeremyphilemon

@SensehacK
Copy link
Contributor Author

@balrajsingh9 Nice insight on 3 options for UX.

Also font weight makes quite the difference, I'm currently swamped till next week with my final year project and thats the reason I wasn't able to put some design thinking towards this enhancement.

If you have any Color palettes in mind, you could share it using a table

  1. Light
  • Primary color 1: UIColor(rgb x,x,x)
  • Primary color 2: UIColor(rgb x,x,x)
  • Primary color 3: UIColor(rgb x,x,x)
  • Primary color 4: UIColor(rgb x,x,x)
  • Primary color 5: UIColor(rgb x,x,x)
  1. Dark
  • Primary color 1: UIColor(rgb x,x,x)
  • Primary color 2: UIColor(rgb x,x,x)
  • Primary color 3: UIColor(rgb x,x,x)
  • Primary color 4: UIColor(rgb x,x,x)
  • Primary color 5: UIColor(rgb x,x,x)
  1. Dim
  • Primary color 1: UIColor(rgb x,x,x)
  • Primary color 2: UIColor(rgb x,x,x)
  • Primary color 3: UIColor(rgb x,x,x)
  • Primary color 4: UIColor(rgb x,x,x)
  • Primary color 5: UIColor(rgb x,x,x)

So that we can all collectively agree on color scheme and have an overall consistent UX.

@balrajsingh9
Copy link

Thank you @SensehacK.
I'll read more about what color scheme is apt. for the theme variants that we're going to use and which conforms to Google's Material Guideline (Twitter's scheme is good though!). I'll check the exact color values that we can use.

Let me know if you need any help with writing code for this. I can take out some time out of my job 😆

@balrajsingh9
Copy link

Twitter has 6 primary colors and they remain the same for all the theme variants. They have chosen a very nice set of primary color scheme which doesn't look odd or creates any visibility issues.

Here is a short video that I recorded demonstrating my above point - GDrive

@SensehacK
Copy link
Contributor Author

So they have played around with opacity of every element like Basic Color (h1-h6, span, text) & just change the primary prominent feature of UI with only 5 UIColors options.

By checking your video, only thing they change of the 3 UI Modes is the background color as the basic contrast color blends well with it.
Utilizing just one of the 5 UIColor options as "Icing on the cake" personalization.

This approach is minimalist and I really appreciate it but its @jeremyphilemon decision or moderators whether we should steer in that direction.

@stale
Copy link

stale bot commented Jun 6, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added Status: Abandoned issue is no longer important to the requestor and no one else has shown an interest in it and removed Status: Abandoned issue is no longer important to the requestor and no one else has shown an interest in it labels Jun 6, 2020
@SensehacK
Copy link
Contributor Author

@balrajsingh9 Which colors are we choosing for the theme?

@balrajsingh9
Copy link

balrajsingh9 commented Jun 9, 2020

@SensehacK I would recommend going with the twitter colors as they work well with all kinds of background we would want to have, light, dim, Lights out/Pitch black, etc...

RGBs for Primary Colors -

  • rgb(29, 161, 242)
  • rgb(255, 173, 31)
  • rgb(224, 36, 94)
  • rgb(121, 75, 196)
  • rgb(244, 93, 34)
  • rgb(23, 191, 99)

RGBs/Hex for Background Colors -

  • Light - No change need, rgb(255, 255, 255)
  • Dim - We can use the current one or rgb(21, 32, 43)
  • Lights out/Pitch black - rgb(0, 0, 0)
  • Graphite/Slate/Mat Grey - #222831

More work to do on font colors. Haven't finalized yet, but we can take inspiration/copy from Twitter or any other good looking website.

We may refer to these palettes as well - https://colorhunt.co/palettes/dark

@SensehacK
Copy link
Contributor Author

@balrajsingh9 Looks great!

@balrajsingh9
Copy link

@SensehacK As you have claimed this, are you going to work on it or you want to collaborate?

@SensehacK
Copy link
Contributor Author

@balrajsingh9 So I'm new to React - I would love to collaborate as I still haven't found time to wrap my head around React state management.

Let me know what works for you.

You can raise an PR as well, I'll also work in parallel & always wanted to try Pair programming on Github fork branches? If you're up for it.

@balrajsingh9
Copy link

balrajsingh9 commented Jun 10, 2020

@SensehacK All in.
/claim

I went through the source code to check how the current theme toggle is working and found that we are using use-dark-mode lib to toggle the theme and switch the styles. This prevents us to implement a 3rd variant of background color to choose from, as we can only have either light or a dark theme and not the three variants as we discussed.

Would have worked -
The library provides us the option to pass in the config the class name for both the variants, which is classNameDark
Leveraging that config, we can show the user a modal to select the theme, we won't touch the light variant, but for the dark-variant, we can pass the class name acc to what the user chooses.

But the only problem is that we cannot update the config object after passing it initially 😞

So all in all, we can either -

  • Keep using two variants, light and dark and just change the dark theme's background-color and also fix the font styles wherever we have less visibility issue or
  • Rewrite the logic of theme toggling for using 3 background variants

@balrajsingh9
Copy link

Got a workaround. NVM 😀

@balrajsingh9
Copy link

balrajsingh9 commented Jun 20, 2020

Sneakpeak 💯 Almost ready.
Lots of lots of color modifications and easy on eyes.

Screenshot 2020-06-21 at 3 41 47 AM

Screenshot 2020-06-21 at 3 41 37 AM

@SensehacK
Copy link
Contributor Author

@balrajsingh9 Looks nice & where is the code base, I visited your fork didn't seem that you have pushed the code.

@balrajsingh9
Copy link

balrajsingh9 commented Jun 21, 2020

@balrajsingh9 Looks nice & where is the code base, I visited your fork didn't seem that you have pushed the code.

Fixing lint issues.
Pre-commit hook prevents me to commit before I fix the lint issues.

@SensehacK https://github.com/balrajsingh9/covid19india-react/tree/feat/theme-overhaul

@stale
Copy link

stale bot commented Aug 21, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Status: Abandoned issue is no longer important to the requestor and no one else has shown an interest in it label Aug 21, 2020
@stale stale bot closed this as completed Aug 28, 2020
@github-actions github-actions bot added Status: Completed Nothing further to be done with this issue and removed Status: Claimed It's clear what the subject of the issue is about, and what the resolution should be labels Aug 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Status: Abandoned issue is no longer important to the requestor and no one else has shown an interest in it Status: Completed Nothing further to be done with this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants