Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
7e64bc2
[tb/testing-font-color-conversion] Add regression stories
Myranae Mar 31, 2026
3e8cac7
[tb/testing-font-color-conversion] Fix test error
Myranae Mar 31, 2026
2dcfc57
[tb/testing-font-color-conversion] Color conversions
Myranae Mar 31, 2026
122de80
[tb/testing-font-color-conversion] Update regression stories
Myranae Mar 31, 2026
9aeb1ef
Revert "[tb/testing-font-color-conversion] Color conversions"
Myranae Mar 31, 2026
39416bc
[tb/testing-font-color-conversion] Linting fixes
Myranae Mar 31, 2026
a6d1e15
Reapply "[tb/testing-font-color-conversion] Color conversions"
Myranae Mar 31, 2026
47b8a24
[tb/testing-font-color-conversion] Darken the separator dot for visib…
Myranae Mar 31, 2026
f2b5e81
[tb/testing-font-color-conversion] New story covering graded colors
Myranae Mar 31, 2026
449198a
[tb/testing-font-color-conversion] Add story for incorrect state colors
Myranae Mar 31, 2026
b740a05
[tb/testing-font-color-conversion] docs(changeset): Convert font and …
Myranae Mar 31, 2026
275fefb
[tb/testing-font-color-conversion] Revert to straight token conversio…
Myranae Apr 1, 2026
4bc562d
[tb/testing-font-color-conversion] Change background to foreground color
Myranae Apr 1, 2026
9095183
[tb/testing-font-color-conversion] Change background to foreground co…
Myranae Apr 1, 2026
cd3ebbd
[tb/testing-font-color-conversion] Remove planning docs
Myranae Apr 1, 2026
6c3cdd8
[tb/testing-font-color-conversion] Revert back to background
Myranae Apr 1, 2026
9097781
[tb/testing-font-color-conversion] Merge branch 'main' into tb/testin…
Myranae Apr 1, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/nervous-moons-roll.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Convert font and colors to semantic tokens for label image
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The changeset description says "Convert font and colors to semantic tokens for label image", but this PR only converts color properties — no font-related tokens are changed anywhere. This will produce a misleading CHANGELOG entry; the description should be updated to "Convert hardcoded color values to semantic tokens for label image".

Extended reasoning...

The changeset file at .changeset/nervous-moons-roll.md (line 5) reads: Convert font and colors to semantic tokens for label image. However, examining every modified file in this PR reveals that only background color and border color properties were converted to semantic tokens — no font-related properties (font-size, font-weight, font-family, text color, etc.) were touched.

The specific color conversions made in this PR are:

  • answer-pill.tsx: backgroundColor: "#00880b"semanticColor.core.background.success.strong
  • label-image.tsx: background: "rgba(33, 36, 44, 0.32)"semanticColor.core.border.neutral.default
  • marker.tsx: backgroundColor: "#ECF3FE"semanticColor.core.background.instructive.subtle and background: "#00880b"semanticColor.core.background.success.strong

All four converted values are background or border color properties. None of them are font or typography tokens. The PR title itself confirms this scope: "[ColorSync] Convert color values to semantic tokens in label-image" — no mention of fonts.

The git branch name tb/testing-font-color-conversion and the git log entries (e.g., "Change background to foreground color", "Revert to straight token conversion vs figma color") suggest that font/foreground color conversion was originally planned or experimented with during development, but was ultimately not included in this PR. The changeset description was likely written when font conversion was still planned and never updated when that work was removed.

The impact is purely a documentation inaccuracy: the generated CHANGELOG entry for @khanacademy/perseus will claim that font tokens were converted when they were not. This could confuse consumers reviewing the changelog trying to understand the scope of this patch.

To fix this, the changeset description should be updated to accurately reflect what was done: Convert hardcoded color values to semantic tokens for label image.

Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import {generateTestPerseusItem} from "@khanacademy/perseus-core";

import {themeModes} from "../../../../../../.storybook/modes";
import {ServerItemRendererWithDebugUI} from "../../../testing/server-item-renderer-with-debug-ui";
import {
incorrectAnswerQuestion,
textQuestion,
numberline,
} from "../__tests__/label-image.testdata";

import type {Meta, StoryObj} from "@storybook/react-vite";

const meta: Meta = {
title: "Widgets/Label Image/Visual Regression Tests/Initial State",
component: ServerItemRendererWithDebugUI,
tags: ["!dev"],
parameters: {
docs: {
description: {
component:
"Regression tests for the Label Image widget that do NOT " +
"need any interactions to test.",
},
},
chromatic: {disableSnapshot: false, modes: themeModes},
},
};
export default meta;

type Story = StoryObj<typeof ServerItemRendererWithDebugUI>;

// Verifies the default unanswered state: all markers visible and pulsating,
// no answers selected, text choices.
export const DefaultUnanswered: Story = {
args: {
item: generateTestPerseusItem({question: textQuestion}),
},
};

// Verifies choices shown in the instructions section (hideChoicesFromInstructions: false),
// including TeX fraction choices and the rgba(33, 36, 44, 0.32) separator dots
// that appear between each choice.
export const WithChoicesInInstructions: Story = {
args: {
Comment on lines +41 to +44
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The WithChoicesInInstructions story comment on line 42 still reads "the rgba(33, 36, 44, 0.32) separator dots" — the exact hardcoded value this PR replaces with semanticColor.core.border.neutral.default. Update the comment to reference the semantic token instead.

Extended reasoning...

This PR replaces rgba(33, 36, 44, 0.32) with semanticColor.core.border.neutral.default in label-image.tsx (line 863). The new story file label-image-initial-state-regression.stories.tsx was written as part of this same PR to serve as a visual regression baseline, but the comment on the WithChoicesInInstructions story was never updated to reflect the token-based implementation.

The stale reference appears at lines 41-42 of the new file:

// including TeX fraction choices and the rgba(33, 36, 44, 0.32) separator dots
// that appear between each choice.

This is immediately contradicted by label-image.tsx where the instructionsChoice::after pseudo-element now uses background: semanticColor.core.border.neutral.default instead of the raw RGBA value.

The comment was almost certainly written before the color conversion was made and was not revisited. Since the file is brand new (added in this PR), there is no pre-existing history to blame — the inaccuracy was introduced the moment this PR was created.

Step-by-step proof: A developer reads WithChoicesInInstructions to understand what it tests. The comment says the separator dots use rgba(33, 36, 44, 0.32). They search the codebase for that string and find nothing, because this PR deleted it. They are now misled about what color value is used. The actual color is resolved at runtime from semanticColor.core.border.neutral.default, a value that may differ across themes.

The fix is a one-line comment update: replace rgba(33, 36, 44, 0.32) with semanticColor.core.border.neutral.default in the story comment. No functional code is affected.

item: generateTestPerseusItem({question: numberline}),
},
};

// Verifies the incorrect marker state: marker dot renders with neutral
// background (background.neutral.default) when showCorrectness is "incorrect".
// No answer pill is shown because no answer is selected in this static state.
export const IncorrectMarker: Story = {
args: {
item: generateTestPerseusItem({question: incorrectAnswerQuestion}),
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
import {generateTestPerseusItem} from "@khanacademy/perseus-core";
import {within} from "storybook/test";

import {themeModes} from "../../../../../../.storybook/modes";
import {ServerItemRendererWithDebugUI} from "../../../testing/server-item-renderer-with-debug-ui";
import {
incorrectAnswerQuestion,
mathQuestion,
shortTextQuestion,
textQuestion,
} from "../__tests__/label-image.testdata";

import type {Meta} from "@storybook/react-vite";

const meta: Meta = {
title: "Widgets/Label Image/Visual Regression Tests/Interactions",
component: ServerItemRendererWithDebugUI,
tags: ["!autodocs"],
parameters: {
chromatic: {disableSnapshot: false, modes: themeModes},
},
};

export default meta;

// Verifies the marker open/selected state: clicking a marker button opens the
// answer choices dropdown and shows the active marker styling.
export const MarkerOpened = {
args: {
item: generateTestPerseusItem({question: textQuestion}),
},
play: async ({canvasElement, userEvent}) => {
const canvas = within(canvasElement);
const marker = canvas.getByLabelText("The fourth unlabeled bar line.");
await userEvent.click(marker);
},
};

// Verifies the post-interaction marker state: after selecting an answer and
// closing the dropdown, all markers render as white circles (not the default
// pulsing blue).
export const AnswerSelected = {
args: {
item: generateTestPerseusItem({question: textQuestion}),
},
play: async ({canvasElement, userEvent}) => {
const canvas = within(canvasElement);
const marker = canvas.getByLabelText("The fourth unlabeled bar line.");
await userEvent.click(marker);

// WonderBlocks SingleSelect renders options into a React portal outside
// the canvas, so we scope to document.body.
const suvsChoice = within(document.body).getByRole("option", {
name: "SUVs",
});
await userEvent.click(suvsChoice);
},
};

// Verifies the correct answer state: after selecting the right answer and
// clicking Check, the marker and answer pill render in green (success.strong).
// Uses shortTextQuestion (single marker) to avoid needing to fill all markers.
// Check is clicked twice due to a server-side scoring quirk in Storybook.
export const CorrectAnswerGraded = {
args: {
item: generateTestPerseusItem({question: shortTextQuestion}),
},
play: async ({canvasElement, userEvent}) => {
const canvas = within(canvasElement);

const marker = canvas.getByLabelText("The fourth unlabeled bar line.");
await userEvent.click(marker);

// WonderBlocks SingleSelect renders options into a React portal outside
// the canvas, so we scope to document.body.
const suvsChoice = within(document.body).getByRole("option", {
name: "SUVs",
});
await userEvent.click(suvsChoice);

// eslint-disable-next-line testing-library/prefer-screen-queries
const checkButton = canvas.getByRole("button", {name: "Check answer"});
await userEvent.click(checkButton);
await userEvent.click(checkButton);
},
};

// Verifies the incorrect answer state: marker dot renders with neutral
// background (background.neutral.default) and the answer pill shows the wrong
// selection with the same neutral styling. Uses incorrectAnswerQuestion, which
// has showCorrectness "incorrect" pre-set on the marker in the question data,
// matching how this state is passed in from review/show-solutions contexts.
// The play function selects an answer so the pill becomes visible.
export const IncorrectAnswerWithPill = {
args: {
item: generateTestPerseusItem({question: incorrectAnswerQuestion}),
},
play: async ({canvasElement, userEvent}) => {
const canvas = within(canvasElement);

const marker = canvas.getByLabelText("The fourth unlabeled bar line.");
await userEvent.click(marker);

// WonderBlocks SingleSelect renders options into a React portal outside
// the canvas, so we scope to document.body.
const trucksChoice = within(document.body).getByRole("option", {
name: "Trucks",
});
await userEvent.click(trucksChoice);
},
};

// Verifies that math choices render correctly inside an open marker dropdown.
// The math choices are only visible after opening a marker, so we capture
// the open state here.
export const MathChoicesVisible = {
args: {
item: generateTestPerseusItem({question: mathQuestion}),
},
play: async ({canvasElement, userEvent}) => {
const canvas = within(canvasElement);
const marker = canvas.getByLabelText("The fourth unlabeled bar line.");
await userEvent.click(marker);
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,54 @@ export const numberline: PerseusRenderer = {
},
};

// Not typed as PerseusRenderer because showCorrectness is part of
// InteractiveMarkerType (runtime UI state) but not PerseusLabelImageMarker
// (the schema type). The field is supported by the widget component at runtime.
export const incorrectAnswerQuestion = {
content:
"Carol created a chart and a bar graph to show how many of each type of vehicle were in her supermarket parking lot.\n\nVehicle Type | Number in the parking lot\n:- | :-: \nTrucks| $25$ \nVans | $5$ \nCars| $40$ \nSUVs | $10$ \n\n**Label each bar on the bar graph.**\n\n[[☃ label-image 1]]\n\n",
images: {
"web+graphie://ka-perseus-graphie.s3.amazonaws.com/1e28332fd2321975639ab50c9ce442e568c18421":
{
width: 341,
height: 310,
},
},
widgets: {
"label-image 1": {
type: "label-image" as const,
alignment: "default",
static: false,
graded: true,
options: {
static: false,
choices: ["Trucks", "Vans", "Cars", "SUVs"],
imageAlt:
"A bar graph with four bar lines that shows the horizontal axis labeled Number in the parking lot and the vertical axis labeled Vehicle Type. The horizontal axis is labeled, from left to right: 0, 10, 20, 30, 40, and 50. The vertical axis has, from the bottom to the top, four unlabeled bar lines as follows: the first unlabeled bar line extends to the middle of 0 and 10, the second unlabeled bar line extends to 40, the third unlabeled bar line extends to the middle of 20 and 30, and fourth unlabeled bar line extends to 10.",
imageUrl:
"web+graphie://ka-perseus-graphie.s3.amazonaws.com/56c60c72e96cd353e4a8b5434506cd3a21e717af",
imageWidth: 415,
imageHeight: 314,
markers: [
{
answers: ["SUVs"],
label: "The fourth unlabeled bar line.",
x: 25,
y: 17.7,
showCorrectness: "incorrect",
},
],
multipleAnswers: false,
hideChoicesFromInstructions: true,
},
version: {
major: 0,
minor: 0,
},
},
},
};

export const longTextFromArticle: PerseusRenderer = {
content: "[[☃ label-image 1]]",
images: {},
Expand Down
3 changes: 1 addition & 2 deletions packages/perseus/src/widgets/label-image/answer-pill.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,7 @@ export const AnswerPill = (props: {

const styles = StyleSheet.create({
correct: {
// WB green darkened by 18%
backgroundColor: "#00880b",
backgroundColor: semanticColor.core.background.success.strong,
},
incorrect: {
backgroundColor: semanticColor.core.background.neutral.default,
Expand Down
3 changes: 2 additions & 1 deletion packages/perseus/src/widgets/label-image/label-image.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import {scoreLabelImageMarker} from "@khanacademy/perseus-score";
import Clickable from "@khanacademy/wonder-blocks-clickable";
import {View} from "@khanacademy/wonder-blocks-core";
import {semanticColor} from "@khanacademy/wonder-blocks-tokens";
import {StyleSheet, css} from "aphrodite";
import classNames from "classnames";
import * as React from "react";
Expand Down Expand Up @@ -862,7 +863,7 @@ const styles = StyleSheet.create({
marginLeft: 5,
marginRight: 5,

background: "rgba(33, 36, 44, 0.32)",
background: semanticColor.core.border.neutral.default,
Comment on lines 863 to +866
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The separator dot pseudo-element in instructionsChoice uses semanticColor.core.border.neutral.default for its CSS background property, but border tokens are semantically distinct from background tokens and may resolve to different values in non-default themes (dark mode, high-contrast). The correct token is semanticColor.core.background.neutral.default, consistent with markerIncorrect in marker.tsx and the incorrect pill in answer-pill.tsx.

Extended reasoning...

Wrong semantic token category used for separator dot background

What the bug is: In label-image.tsx line 866, the instructionsChoice style block uses semanticColor.core.border.neutral.default as the value of the CSS background property for the ::after pseudo-element that renders separator dots between instruction choices. Border tokens and background tokens are distinct categories in a design token system and are not interchangeable — they exist for different semantic purposes and can resolve to different hues, opacities, or color values across themes.

The specific code path: The instructionsChoice style defines a ::after pseudo-element that creates a 2×2px filled circle (via borderRadius: 2) used as a visual separator dot between each non-last choice item. This is purely a filled background element. The PR replaces the original rgba(33, 36, 44, 0.32) with semanticColor.core.border.neutral.default — a token that belongs to the border category, not the background category.

Why existing code does not prevent this: There is no type-level enforcement preventing a border token from being passed to a background CSS property; both are plain string values at runtime. The bug is a semantic/intent mismatch, not a type error.

Impact: In the default light theme, border and background neutral tokens may happen to be visually similar, so the bug could go unnoticed. However, in dark mode or high-contrast themes, these tokens are likely to diverge. Since the stated purpose of this PR is improved theme support via semantic token conversion, using the wrong token category directly undermines that goal. The separator dots could render with incorrect color or opacity in non-default themes.

Proof by example: Consider the pattern established within this same PR:

  • marker.tsx:286markerIncorrect style: background: semanticColor.core.background.neutral.default
  • answer-pill.tsx:91incorrect style: backgroundColor: semanticColor.core.background.neutral.default
  • marker.tsx:213markerIcon border: border: \2px solid ${semanticColor.core.border.neutral.default}`` ✅ (correct: border token for an actual CSS border)
  • answer-pill.tsx:12 — box shadow: boxShadow: \0 8px 8px ${semanticColor.core.border.neutral.default}`` ✅ (correct: border token for a shadow outline)
  • label-image.tsx:866 — separator dot: background: semanticColor.core.border.neutral.default ❌ (wrong: border token for a fill/background)

All three neutral-colored filled elements use background.neutral.default; the only other uses of border.neutral.default are for actual border and boxShadow properties. Line 866 breaks this consistent pattern.

Fix: Replace semanticColor.core.border.neutral.default with semanticColor.core.background.neutral.default at label-image.tsx:866 to match the pattern used for all other neutral-colored background elements in this widget.


borderRadius: 2,
},
Expand Down
4 changes: 2 additions & 2 deletions packages/perseus/src/widgets/label-image/marker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ const styles = StyleSheet.create({

// The learner has made a selection
markerFilled: {
backgroundColor: "#ECF3FE",
backgroundColor: semanticColor.core.background.instructive.subtle,
border: `4px solid ${semanticColor.core.border.instructive.default}`,
},

Expand All @@ -279,7 +279,7 @@ const styles = StyleSheet.create({
},

markerCorrect: {
background: "#00880b", // WB green darkened by 18%
background: semanticColor.core.background.success.strong,
},

markerIncorrect: {
Expand Down
Loading