[ColorSync][math-input] Convert math-input font and colors to semantic tokens#3744
[ColorSync][math-input] Convert math-input font and colors to semantic tokens#3744Myranae wants to merge 13 commits into
Conversation
|
Size Change: -55 B (-0.01%) Total Size: 508 kB 📦 View Changed
ℹ️ View Unchanged
|
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (895a39c) and published it to npm. You Example: pnpm add @khanacademy/perseus@PR3744If you are working in Khan Academy's frontend, you can run the below command. ./dev/tools/bump_perseus_version.ts -t PR3744If you are working in Khan Academy's webapp, you can run the below command. ./dev/tools/bump_perseus_version.js -t PR3744 |
…ed colors and fonts in math-input components to semantic tokens (cursor-handle, keypad-button, navigation-button)
github-actions
Bot
added
schema-change
Myranae
changed the title
|
@claude review once |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — mechanical color/font token migration with visual regression coverage.
Extended reasoning...
Overview
This PR is part of the ColorSync migration and converts hardcoded color/font values in the math-input package to WonderBlocks semantic tokens. Changes touch the cursor handle SVG fill/stroke, keypad button pressed state (border + background + font family), and navigation button pressed state. It also adds three new visual regression stories (CursorHandleVisible, KeypadButtonPressed, NavigationButtonPressed) plus a MathQuill cursor-blink stabilization decorator for deterministic snapshots. Snapshot updates are purely class-name churn from the aphrodite style hash changing.
Security risks
None. The changes are color/font constant replacements and additive Storybook stories — no auth, data handling, or input parsing is touched.
Level of scrutiny
Low. This is a presentation-only token swap following an established migration pattern (matches prior ColorSync PRs). The author has chromatic diffs reviewed and called out the three expected visual changes; the snapshot churn is the expected mechanical side effect. The TODO comment on the unreachable action branch in keypad-button.tsx is a helpful annotation, not a behavior change.
Other factors
- No bugs found by the bug hunting system.
- The MathQuill blink-decorator in
expression-interactions-regression.stories.tsxis well-commented (explains the@layer shared!important inversion) and scoped to the story file, so it does not affect production code. - Bundle size delta is -55 B, consistent with the simplification of the pressed-state gradient into a single token.
|
|
||
| const meta: Meta<typeof CursorHandle> = { | ||
| title: "Math Input/Components/Visual Regression Tests/Initial State", | ||
| component: CursorHandle, |
There was a problem hiding this comment.
Would this still be considered general math input initial state regression story if the main component being tested is CursorHandle? I almost feel like this should be cursor-handle-regression.stories.tsx instead?
|
|
||
| const meta: Meta<typeof NavigationPad> = { | ||
| title: "Math Input/Components/Visual Regression Tests/Interactions", | ||
| component: NavigationPad, |
There was a problem hiding this comment.
Same comment here but for NavigationPad.
| border: "2px solid #1B50B3", | ||
| border: `2px solid ${semanticColor.core.border.instructive.strong}`, | ||
| padding: 0, | ||
| background: `linear-gradient(0deg, rgba(24, 101, 242, 0.32), rgba(24, 101, 242, 0.32)), ${semanticColor.core.background.base.default}`, |
There was a problem hiding this comment.
oh whoa, what was this linear gradient doing before?
| decorators: [ | ||
| (Story) => { | ||
| // MathQuill toggles `mq-blink` via setInterval, making snapshots | ||
| // non-deterministic. The blink rules in main.css use `!important` |
There was a problem hiding this comment.
I love a good use of non-deterministic, but can we say flakey here so it's more immediately obvious what the problem is?
Or maybe just add a sentence above this that says "text field cursor blinking can make snapshots flakey"
|
|
||
| // This component is touch-only in production and never reached via mouse | ||
| // interactions, so it is tested here in isolation. | ||
| export const CursorHandleVisible: Story = { |
There was a problem hiding this comment.
I don't think I see this in the UI Tests check on your PR?
| }, | ||
| }; | ||
|
|
||
| export const KeypadButtonPressed: Story = { |
There was a problem hiding this comment.
I don't think I see this in the UI Tests check on your PR either?
2 participants
Summary:
Part of the ColorSync migration. Converts hardcoded color and font values in the math-input package to WonderBlocks semantic tokens. See images below for color comparisons and affected widget areas.
Main Changes Visible
Keypad Button Pressed (outline and background)
Cursor Handle
Navigation Button Pressed
Issue: LEMS-4244
Test plan:
CursorHandleVisible(new regression story, cursor handle fill/stroke)KeypadButtonPressed(pressed state background + border)NavigationButtonPressed(pressed state border)