-
Notifications
You must be signed in to change notification settings - Fork 10
[WB-1896] Tokens: Add thunderblocks
theme to Wonder Blocks
#2562
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 3e9dfeb The changes in this PR will be included in the next version bump. This PR includes changesets to release 26 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Size Change: 0 B Total Size: 103 kB ℹ️ View Unchanged
|
thunderblocks
theme to Wonder Blocks.thunderblocks
theme to Wonder Blocks
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (4c87148) and published all packages with changesets to npm. You can install the packages in webapp by running: ./services/static/dev/tools/deploy_wonder_blocks.js --tag="PR2562" Packages can also be installed manually by running: pnpm add @khanacademy/wonder-blocks-<package-name>@PR2562 |
// Black | ||
black_100: black, | ||
black_90: fade(black, 0.9), | ||
black_80: fade(black, 0.8), | ||
black_70: fade(black, 0.7), | ||
black_60: fade(black, 0.6), | ||
black_50: fade(black, 0.5), | ||
black_40: fade(black, 0.4), | ||
black_30: fade(black, 0.3), | ||
black_20: fade(black, 0.2), | ||
black_10: fade(black, 0.1), | ||
// White | ||
white_100: white, | ||
white_90: fade(white, 0.9), | ||
white_80: fade(white, 0.8), | ||
white_70: fade(white, 0.7), | ||
white_60: fade(white, 0.6), | ||
white_50: fade(white, 0.5), | ||
white_40: fade(white, 0.4), | ||
white_30: fade(white, 0.3), | ||
white_20: fade(white, 0.2), | ||
white_10: fade(white, 0.1), | ||
white_01: fade(white, 0.01), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: This might change based on a convo with Caitlyn.
A new build was pushed to Chromatic! 🚀https://5e1bf4b385e3fb0020b7073c-jopopylqbb.chromatic.com/ Chromatic results:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me! It's a big piece I was missing for typography theming.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! So exciting to see the theme colors get updated when we toggle the theme 😄
{ | ||
value: "thunderblocks", | ||
icon: "lightning", | ||
title: "Thunder Blocks (Classroom)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we also want to include OG
in the title for the default theme? In case others are referring to the default theme as OG
!
import {fade} from "../../../util/utils"; | ||
|
||
// Base colors | ||
const black = "#191918"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My one thought on working through some font theming is: should the default theme be put into these theme folders to keep similar objects near each other in the codebase? I think it could help reduce some cognitive complexity for theming.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great question! My main reasoning for not putting the default color primitives was because those tokens are public (TB primitive colors are not). I left color.ts
and spacing.ts
inside the tokens package as the goal is to mark them as deprecated at some point in the future (or OG colors are going to be marked as internal at some point).
OG colors are public mostly for historic reasons (we didn't use semanticColor
tokens before), and the overall goal is to make these colors private at some point, but I'm not fully sure if we are going to be able to do it. That will happen for sure after completing Polaris and doing all the cleanup work from legacy UIs that won't be used by the Classic Experience anymore (aka things that are deprecated from ka.org and moved to Polaris).... same applies with Spacing -> Sizing
.
That said, if you think that it is better to move OG colors to the theme
folder, then I can do it without any problem :) (at the end, it is an internal thing, and the public export remains the same).
wdyt?
@jandrade FYI I think this one might need to be updated with |
Summary:
Adds the new
thunderblocks
theme to support the new classroom experience.This new theme is based on the existing default theme (OG) but it uses its own
primitive colors.
Also added theming support in Chromatic modes, so that the new theme can be
used for visual regression testing. Note that this was only added to the
IconButton snapshots for now to verify that the new theme works as expected.
More changes will come in future PRs to start using all the correct tokens in
all the required components (including
sizing
,borders
,offsets
,typography
, etc).Issue: WB-1896
Test plan:
Verify that the new theme is available in the Storybook via the "Theme" toolbar.
Verify that the component colors change when the theme is switched to
thunderblocks
./?path=/docs/foundations-using-color--docs&globals=theme:thunderblocks
Also verify that the
IconButton
snapshots now include thethunderblocks
theme.