Paths

Table of Contentst

[native] introduce new shadesBlack90
ClosedPublic
Actions

Authored by ginsu on Nov 8 2023, 2:40 PM.

Details

Reviewers

atul
inka
rohan
ashoat

Commits

rCOMMfa42aa2dfeff: [native] introduce new shadesBlack90

Summary

This diff introduces a new shadesBlack90. The #191919 color is right now only used in one place so it never got a shade; however, the keyserver selection bottom sheet will use this + a lot of the web app redesign is going to use this color, so I think it makes sense to make this an official shade now.

Here is a screenshot to the new shades of black in the 2023 design system:

Screenshot 2023-11-08 at 6.22.00 PM.png (624×1 px, 51 KB)

Here is the figma doc: https://www.figma.com/file/fwKJ6eNgfxlx8a7WxSyAZL/2023-Design-System?type=design&node-id=1-426&mode=design

I still need to update the figma, but I want to do this after this diff gets accepted in case I need to make changes to this, but here is a task to track that
https://linear.app/comm/issue/DES-159/update-shades-of-black-in-figma

Test Plan

flow + went through the app to confirm that there were no regressions

This diff was handled using my code editor find + replace feature. Read through every value and all shades except for shadesBlack100 should be decremented by 10

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Nov 8 2023, 2:40 PM

Herald added subscribers: will, tomek, ashoat. · View Herald TranscriptNov 8 2023, 2:40 PM

ginsu edited the summary of this revision. (Show Details)Nov 8 2023, 2:55 PM

ginsu edited the test plan for this revision. (Show Details)

ginsu added reviewers: atul, inka, rohan.

ginsu attached a referenced file: F866271: Screenshot 2023-11-08 at 5.45.36 PM.png. (Show Details)

ginsu edited the summary of this revision. (Show Details)Nov 8 2023, 2:58 PM

ginsu added inline comments.

native/themes/colors.js
239 ↗	(On Diff #32980)	This is now the new `shadesBlack90`

ginsu added a child revision: D9790: [web] introduce new --shades-black-90.Nov 8 2023, 3:10 PM

ginsu edited the summary of this revision. (Show Details)Nov 8 2023, 3:23 PM

ginsu attached a referenced file: F866340: Screenshot 2023-11-08 at 6.22.00 PM.png. (Show Details)

ginsu published this revision for review.Nov 8 2023, 3:27 PM

Harbormaster completed remote builds in B23937: Diff 32980.Nov 8 2023, 5:16 PM

It seems like "90" and "80" are super close. Are you sure we need this new color? Can you explain / justify its introduction a bit more?
Is 50 / 60 / 70 / 80 / 90 actually arithmetically accurate? I have a suspicion it's not. If we actually took real mathematic percentages of pure black, what would it really be?

ashoat requested changes to this revision.Nov 9 2023, 3:44 AM

ashoat mentioned this in D9790: [web] introduce new --shades-black-90.

This revision now requires changes to proceed.Nov 9 2023, 3:45 AM

It seems like "90" and "80" are super close. Are you sure we need this new color? Can you explain / justify its introduction a bit more?

In the app there are a couple of places that use both "90" and "80":
The community drawer (90 is the darker color in the opened community and 80 is the drawer background)

Screenshot 2023-11-09 at 10.57.49 AM.png (1×1 px, 725 KB)

The keyserver selection modal (90 is the darker accent color around the keyserver details and 80 is the modal bottomsheet backgroud)

Screenshot 2023-11-09 at 11.01.39 AM.png (1×1 px, 737 KB)

We will also have this new 90 color in a lot of the web app redesign (the red is just to show where this new color value would be placed):

Screenshot 2023-11-09 at 11.12.43 AM.png (1×1 px, 345 KB)

Is 50 / 60 / 70 / 80 / 90 actually arithmetically accurate? I have a suspicion it's not. If we actually took real mathematic percentages of pure black, what would it really be?

I am not too sure about this. I just took the color value that was already in Figma. Picking the exact arithmetically accurate color value is a bit outside my design expertise, and I wasn't sure if I should spend a lot of time on this, but can do so if we think it's important.

If we still feel strongly about not wanting to introduce this color, the best way forward will probably be to migrate the new 90 value to use the current black 100; however, things will look like this (which I don't think visually look great):

Screenshot 2023-11-09 at 11.16.23 AM.png (1×1 px, 737 KB)

Screenshot 2023-11-09 at 11.16.17 AM.png (1×1 px, 729 KB)

ginsu attached a referenced file: F867377: Screenshot 2023-11-09 at 11.16.17 AM.png. (Show Details)Nov 9 2023, 8:19 AM

ginsu attached a referenced file: F867378: Screenshot 2023-11-09 at 11.16.23 AM.png. (Show Details)

ginsu attached a referenced file: F867373: Screenshot 2023-11-09 at 11.12.43 AM.png. (Show Details)

ginsu attached a referenced file: F867337: Screenshot 2023-11-09 at 11.01.39 AM.png. (Show Details)

ginsu attached a referenced file: F867334: Screenshot 2023-11-09 at 10.57.49 AM.png. (Show Details)

Thanks for explaining. Let's keep the new color, but I'd like an answer to my question about the arithmetic. You're making an opinionated change here by changing our existing colors (eg. 80 -> 70) – that change should be an informed one.

Picking the exact arithmetically accurate color value is a bit outside my design expertise

It really should not be so difficult for you to calculate. If you're confused about it, feel free to ask @atul. This should take you like 5min.

Is 50 / 60 / 70 / 80 / 90 actually arithmetically accurate? I have a suspicion it's not. If we actually took real mathematic percentages of pure black, what would it really be?

As it stands right now the mathematic percentages of pure black for our shadesBlack variables are:

shadesBlack100 (#0a0a0a): ~96%
shadesBlack90 (#191919): ~90%
shadesBlack80 (#1f1f1f): ~88%
shadesBlack70 (#404040): ~75%
shadesBlack60 (#666666): 60%
shadesBlack50 (#808080): ~50%

Screenshot 2023-11-09 at 5.15.29 PM.png (558×320 px, 55 KB)

Based on this, the only color variables that I updated that are not arithmetically accurate are shadesBlack80 and shadesBlack70 (I did not touch shadesBlack100 in this diff). I think the best way to move forward is that we should update shadesBlack80 => shadesBlack85 and shadesBlack70 => shadesBlack75. (If we do this it might also be good to update shadesBlack100 => shadesBlack95).

Another option is that we could also update the hex value to be closer to the names, but I think this could be pretty risky and could introduce a bunch of regressions with our colors in the app if we are not super careful

ginsu attached a referenced file: F867980: Screenshot 2023-11-09 at 5.15.29 PM.png. (Show Details)Nov 9 2023, 2:34 PM

Oh whoa, so the original change actually makes shadesBlack60 and some others more accurate.

I think the best way to move forward is that we should update shadesBlack80 => shadesBlack85 and shadesBlack70 => shadesBlack75. (If we do this it might also be good to update shadesBlack100 => shadesBlack95).

Makes sense to me! Let's do it.