[Design PR - not for merging] Theming design pass #2906
Conversation
There was a problem hiding this comment.
We'll want to merge this into the theming branch rather than theming-v0.4, which is a cut branch for that beta release.
This LGTM once the snapshots are updated and tests are fixed to remove references to secondaryColor. Happy to take over this PR and push it past the finish line if you'd like.
Note that there may be a ton more snapshots to update after the current theming branch is merged into this one as karrie recently changed a lot of existing e2e tests to take snapshots in both light and dark mode.
| _create_option( | ||
| "theme.secondaryColor", | ||
| description=""" | ||
| Used to style secondary interface elements. It provides more ways to | ||
| accent and distinguish your app. Having it is optional. | ||
| """, |
There was a problem hiding this comment.
nit: We'll also want to get rid of the secondary_color proto field in NewReport.proto
| // XXX Are these supposed to be semantic names? Or color names? | ||
|
|
||
| fadedText10: transparentize(genericColors.bodyText, 0.9), // Mostly used for 1px lines. | ||
| fadedText40: transparentize(genericColors.bodyText, 0.6), // Backgrounds. | ||
| fadedText60: transparentize(genericColors.bodyText, 0.4), // Secondary text. | ||
|
|
||
| darkenedBgMix15: hasLightBg ? darken(bgMix, 0.075) : lighten(bgMix, 0.15), // Widget details, focus. | ||
| darkenedBgMix60: hasLightBg ? darken(bgMix, 0.3) : lighten(bgMix, 0.6), // Icons. | ||
|
|
||
| lightenedBg05: lighten(genericColors.bgColor, 0.025), // Button, checkbox, radio background. |
There was a problem hiding this comment.
The object returned here ends up containing both since genericColors gets copied into it via the spread operator above (and genericColors itself contains all of the color primitives from primitives/colors.ts), but I believe everything defined at this point is a semantic name.
How all of these colors are organized unfortunately needs to be reworked pretty badly as it seems like the exact meanings of things started getting more and more muddled as development went on,
tvst
changed the title
|
Going to close this one as all of the changes made here now live in #2921 |
2 participants
I did a design pass on our themes, and in the process of cleaning up our colors Abhi and I realized that it would be better if we replaced
secondaryColorwith a set of calculated colors. So what this PR does is:secondaryColor.You can see the end results here
NOTE:
/src/theme/utils.tsline 292. I left a question in there about what variable names we should use.