From 2d3c3b49a652020e5bf662b7b19682fa94212755 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Fri, 26 Apr 2024 14:01:58 -0700 Subject: [PATCH] Limit colorset for locked figures (#1210) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: Limit locked figures colorset as per this slack conversation. Issue: https://khanacademy.atlassian.net/browse/LEMS-1931 ## Test plan: `yarn jest packages/perseus/src/widgets/__tests__/interactive-graph.test.ts` Storybook pages http://localhost:6006/?path=/docs/perseus-editor-widgets-interactive-graph-editor--docs http://localhost:6006/?path=/docs/perseus-widgets-interactive-graph--docs Screenshot 2024-04-24 at 10 38 53 AM Author: nishasy Reviewers: jeremywiebe, handeyeco, nishasy, mark-fitzgerald Required Reviewers: Approved By: jeremywiebe, handeyeco Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage, ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Extract i18n strings (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1210 --- .changeset/gorgeous-phones-float.md | 6 ++ .../__stories__/color-select.stories.tsx | 7 ++- .../__stories__/color-swatch.stories.tsx | 3 +- .../__tests__/locked-point-settings.test.tsx | 2 +- .../src/components/__tests__/util.test.ts | 8 +-- .../src/components/locked-line-settings.tsx | 4 +- .../src/components/locked-point-settings.tsx | 2 +- .../perseus-editor/src/components/util.ts | 6 +- .../interactive-graph-editor.stories.tsx | 15 ++--- packages/perseus/src/perseus-types.ts | 12 ---- .../interactive-graph.testdata.ts | 58 +++++++++---------- .../interactive-graph.test.ts.snap | 16 ++--- .../__tests__/interactive-graph.test.ts | 28 ++++----- 13 files changed, 82 insertions(+), 85 deletions(-) create mode 100644 .changeset/gorgeous-phones-float.md diff --git a/.changeset/gorgeous-phones-float.md b/.changeset/gorgeous-phones-float.md new file mode 100644 index 0000000000..4002ab185d --- /dev/null +++ b/.changeset/gorgeous-phones-float.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/perseus": minor +"@khanacademy/perseus-editor": minor +--- + +Limit color set for locked figures in Interactive Graph diff --git a/packages/perseus-editor/src/components/__stories__/color-select.stories.tsx b/packages/perseus-editor/src/components/__stories__/color-select.stories.tsx index 56a6b34fce..d8afb0edb4 100644 --- a/packages/perseus-editor/src/components/__stories__/color-select.stories.tsx +++ b/packages/perseus-editor/src/components/__stories__/color-select.stories.tsx @@ -1,6 +1,7 @@ import * as React from "react"; import ColorSelect from "../color-select"; +import {getDefaultFigureForType} from "../util"; import type {LockedFigureColor} from "@khanacademy/perseus"; import type {Meta} from "@storybook/react"; @@ -14,17 +15,19 @@ export const Default = (args): React.ReactElement => { return ; }; +const defaultColor = getDefaultFigureForType("point").color; + // Set the default values in the control panel. Default.args = { id: "color-select", - selectedValue: "blue", + selectedValue: defaultColor, onChange: () => {}, }; export const Controlled = { render: function Render() { const [selectedValue, setSelectedValue] = - React.useState("blue"); + React.useState(defaultColor); const handleColorChange = (color: LockedFigureColor) => { setSelectedValue(color); diff --git a/packages/perseus-editor/src/components/__stories__/color-swatch.stories.tsx b/packages/perseus-editor/src/components/__stories__/color-swatch.stories.tsx index 9486ab2e22..b74582c6eb 100644 --- a/packages/perseus-editor/src/components/__stories__/color-swatch.stories.tsx +++ b/packages/perseus-editor/src/components/__stories__/color-swatch.stories.tsx @@ -1,6 +1,7 @@ import * as React from "react"; import ColorSwatch from "../color-swatch"; +import {getDefaultFigureForType} from "../util"; import type {Meta} from "@storybook/react"; @@ -15,6 +16,6 @@ export const Default = (args): React.ReactElement => { // Set the default values in the control panel. Default.args = { - color: "blue", + color: getDefaultFigureForType("point").color, filled: true, }; diff --git a/packages/perseus-editor/src/components/__tests__/locked-point-settings.test.tsx b/packages/perseus-editor/src/components/__tests__/locked-point-settings.test.tsx index 8226bda209..9ef9b16fc8 100644 --- a/packages/perseus-editor/src/components/__tests__/locked-point-settings.test.tsx +++ b/packages/perseus-editor/src/components/__tests__/locked-point-settings.test.tsx @@ -18,7 +18,7 @@ describe("LockedPointSettings", () => { ); const titleText = screen.getByText("Point (0, 0)"); - const colorSwatch = screen.getByLabelText("Color: blue, filled"); + const colorSwatch = screen.getByLabelText("Color: green, filled"); // Assert expect(titleText).toBeInTheDocument(); diff --git a/packages/perseus-editor/src/components/__tests__/util.test.ts b/packages/perseus-editor/src/components/__tests__/util.test.ts index 03f5e428d0..3b2a1ef083 100644 --- a/packages/perseus-editor/src/components/__tests__/util.test.ts +++ b/packages/perseus-editor/src/components/__tests__/util.test.ts @@ -44,7 +44,7 @@ describe("getDefaultFigureForType", () => { expect(figure).toEqual({ type: "point", coord: [0, 0], - color: "blue", + color: "green", filled: true, }); }); @@ -58,17 +58,17 @@ describe("getDefaultFigureForType", () => { { type: "point", coord: [0, 0], - color: "blue", + color: "green", filled: true, }, { type: "point", coord: [2, 2], - color: "blue", + color: "green", filled: true, }, ], - color: "blue", + color: "green", lineStyle: "solid", showArrows: false, showStartPoint: false, diff --git a/packages/perseus-editor/src/components/locked-line-settings.tsx b/packages/perseus-editor/src/components/locked-line-settings.tsx index 4e5915badb..4e7445ec13 100644 --- a/packages/perseus-editor/src/components/locked-line-settings.tsx +++ b/packages/perseus-editor/src/components/locked-line-settings.tsx @@ -35,7 +35,7 @@ const LockedLineSettings = (props: Props) => { const { kind, points, - color: lineColor = "blue", + color: lineColor, lineStyle = "solid", showArrows, showStartPoint, @@ -133,7 +133,7 @@ const LockedLineSettings = (props: Props) => { {/* Line color settings */} diff --git a/packages/perseus-editor/src/components/locked-point-settings.tsx b/packages/perseus-editor/src/components/locked-point-settings.tsx index 7c0eb80a98..2f404acd8d 100644 --- a/packages/perseus-editor/src/components/locked-point-settings.tsx +++ b/packages/perseus-editor/src/components/locked-point-settings.tsx @@ -170,7 +170,7 @@ const LockedPointSettings = (props: Props) => { <> diff --git a/packages/perseus-editor/src/components/util.ts b/packages/perseus-editor/src/components/util.ts index 7837b1701a..b97be1b93f 100644 --- a/packages/perseus-editor/src/components/util.ts +++ b/packages/perseus-editor/src/components/util.ts @@ -52,6 +52,8 @@ export function getValidNumberFromString(value: string) { return isNaN(parsed) ? 0 : parsed; } +const DEFAULT_COLOR = "green"; + export function getDefaultFigureForType(type: "point"): LockedPointType; export function getDefaultFigureForType(type: "line"): LockedLineType; export function getDefaultFigureForType(type: LockedFigureType): LockedFigure; @@ -61,7 +63,7 @@ export function getDefaultFigureForType(type: LockedFigureType): LockedFigure { return { type: "point", coord: [0, 0], - color: "blue", + color: DEFAULT_COLOR, filled: true, }; case "line": @@ -75,7 +77,7 @@ export function getDefaultFigureForType(type: LockedFigureType): LockedFigure { coord: [2, 2], }, ], - color: "blue", + color: DEFAULT_COLOR, lineStyle: "solid", showArrows: false, showStartPoint: false, diff --git a/packages/perseus-editor/src/widgets/__stories__/interactive-graph-editor.stories.tsx b/packages/perseus-editor/src/widgets/__stories__/interactive-graph-editor.stories.tsx index d32d0002fa..708d40aec5 100644 --- a/packages/perseus-editor/src/widgets/__stories__/interactive-graph-editor.stories.tsx +++ b/packages/perseus-editor/src/widgets/__stories__/interactive-graph-editor.stories.tsx @@ -1,6 +1,7 @@ import * as React from "react"; import {flags} from "../../__stories__/flags-for-api-options"; +import {getDefaultFigureForType} from "../../components/util"; import InteractiveGraphEditor from "../interactive-graph-editor"; import InteractiveGraphEditorArgTypes from "./interactive-graph-editor.argtypes"; @@ -19,11 +20,7 @@ const mafsOptions = { }, }; -const defaultPointProps = { - type: "point", - color: "blue", - filled: true, -}; +const defaultPointProps = getDefaultFigureForType("point"); export default { title: "PerseusEditor/Widgets/Interactive Graph Editor", @@ -156,7 +153,7 @@ export const WithLockedLines: StoryComponentType = { {...defaultPointProps, coord: [0, 2]}, {...defaultPointProps, coord: [2, 3]}, ], - color: "blue", + color: "green", lineStyle: "solid", showArrows: false, showStartPoint: false, @@ -179,10 +176,10 @@ export const WithLockedLines: StoryComponentType = { type: "line", kind: "segment", points: [ - {...defaultPointProps, color: "green", coord: [0, -2]}, - {...defaultPointProps, color: "green", coord: [4, 0]}, + {...defaultPointProps, color: "grayH", coord: [0, -2]}, + {...defaultPointProps, color: "grayH", coord: [4, 0]}, ], - color: "green", + color: "grayH", lineStyle: "solid", showArrows: true, showStartPoint: true, diff --git a/packages/perseus/src/perseus-types.ts b/packages/perseus/src/perseus-types.ts index 9fac08c8dd..a396227aaf 100644 --- a/packages/perseus/src/perseus-types.ts +++ b/packages/perseus/src/perseus-types.ts @@ -655,33 +655,21 @@ export type PerseusInteractiveGraphWidgetOptions = { lockedFigures?: ReadonlyArray; }; -// TODO: If/when these colors are added to Wonder Blocks, we should remove -// them from here and use Wonder Blocks everywhere else instead. const lockedFigureColorNames = [ - "blue", "green", - "gray", "grayH", - "grayI", "purple", - "purpleD", "pink", - "orange", "red", ] as const; export type LockedFigureColor = (typeof lockedFigureColorNames)[number]; export const lockedFigureColors: Record = { - blue: "#3D7586", green: "#447A53", - gray: "#5D5F66", grayH: "#3B3D45", - grayI: "#21242C", purple: "#594094", - purpleD: "#8351E8", pink: "#B25071", - orange: "#946700", red: "#D92916", } as const; diff --git a/packages/perseus/src/widgets/__testdata__/interactive-graph.testdata.ts b/packages/perseus/src/widgets/__testdata__/interactive-graph.testdata.ts index 9ad7c9dbc6..62f4489060 100644 --- a/packages/perseus/src/widgets/__testdata__/interactive-graph.testdata.ts +++ b/packages/perseus/src/widgets/__testdata__/interactive-graph.testdata.ts @@ -225,19 +225,19 @@ export const linearQuestionWithLockedPoints: PerseusRenderer = { { type: "point", coord: [5, 3], - color: "blue", + color: "green", filled: true, }, { type: "point", coord: [4, -4], - color: "blue", + color: "green", filled: true, }, { type: "point", coord: [7, -3], - color: "blue", + color: "green", filled: true, }, ], @@ -968,13 +968,13 @@ export const segmentWithLockedPointsQuestion: PerseusRenderer = { { type: "point", coord: [-7, -7], - color: "blue", + color: "green", filled: true, }, { type: "point", coord: [2, -5], - color: "blue", + color: "green", filled: false, }, ], @@ -1086,17 +1086,17 @@ export const segmentWithLockedLineQuestion: PerseusRenderer = { { type: "point", coord: [-7, -7], - color: "purple", + color: "green", filled: true, }, { type: "point", coord: [2, -5], - color: "purple", + color: "green", filled: false, }, ], - color: "purple", + color: "green", lineStyle: "solid", showArrows: false, showStartPoint: true, @@ -1109,17 +1109,17 @@ export const segmentWithLockedLineQuestion: PerseusRenderer = { { type: "point", coord: [-7, -6], - color: "green", + color: "grayH", filled: false, }, { type: "point", coord: [2, -4], - color: "green", + color: "grayH", filled: true, }, ], - color: "green", + color: "grayH", lineStyle: "solid", showArrows: false, showStartPoint: true, @@ -1495,17 +1495,17 @@ export const segmentWithAllLockedLineSegmentVariations: PerseusRenderer = { { type: "point", coord: [-7, -6], - color: "blue", + color: "grayH", filled: true, }, { type: "point", coord: [2, -4], - color: "purple", + color: "grayH", filled: false, }, ], - color: "purple", + color: "grayH", lineStyle: "dashed", showArrows: false, showStartPoint: true, @@ -1519,17 +1519,17 @@ export const segmentWithAllLockedLineSegmentVariations: PerseusRenderer = { { type: "point", coord: [-7, -7], - color: "blue", + color: "pink", filled: true, }, { type: "point", coord: [2, -5], - color: "blue", + color: "pink", filled: false, }, ], - color: "blue", + color: "pink", lineStyle: "solid", showArrows: true, showStartPoint: false, @@ -1611,17 +1611,17 @@ export const segmentWithAllLockedLineVariations: PerseusRenderer = { { type: "point", coord: [-7, -6], - color: "blue", + color: "grayH", filled: true, }, { type: "point", coord: [2, -4], - color: "purple", + color: "grayH", filled: false, }, ], - color: "purple", + color: "grayH", lineStyle: "dashed", showArrows: false, showStartPoint: true, @@ -1635,17 +1635,17 @@ export const segmentWithAllLockedLineVariations: PerseusRenderer = { { type: "point", coord: [-7, -7], - color: "blue", + color: "pink", filled: true, }, { type: "point", coord: [2, -5], - color: "blue", + color: "pink", filled: false, }, ], - color: "blue", + color: "pink", lineStyle: "solid", showArrows: true, showStartPoint: false, @@ -1727,17 +1727,17 @@ export const segmentWithAllLockedRayVariations: PerseusRenderer = { { type: "point", coord: [-7, -6], - color: "purple", + color: "grayH", filled: true, }, { type: "point", coord: [2, -4], - color: "purple", + color: "grayH", filled: false, }, ], - color: "purple", + color: "grayH", lineStyle: "dashed", showArrows: false, showStartPoint: true, @@ -1751,17 +1751,17 @@ export const segmentWithAllLockedRayVariations: PerseusRenderer = { { type: "point", coord: [-7, -7], - color: "blue", + color: "pink", filled: true, }, { type: "point", coord: [2, -5], - color: "blue", + color: "pink", filled: false, }, ], - color: "blue", + color: "pink", lineStyle: "solid", showArrows: true, showStartPoint: false, diff --git a/packages/perseus/src/widgets/__tests__/__snapshots__/interactive-graph.test.ts.snap b/packages/perseus/src/widgets/__tests__/__snapshots__/interactive-graph.test.ts.snap index f0c26fb7e0..a2bccbb3c9 100644 --- a/packages/perseus/src/widgets/__tests__/__snapshots__/interactive-graph.test.ts.snap +++ b/packages/perseus/src/widgets/__tests__/__snapshots__/interactive-graph.test.ts.snap @@ -4151,7 +4151,7 @@ exports[`snapshots should render correctly with locked lines 1`] = ` { // Assert expect(points[0]).toHaveStyle({ - fill: lockedFigureColors.blue, - stroke: lockedFigureColors.blue, + fill: lockedFigureColors.green, + stroke: lockedFigureColors.green, }); expect(points[1]).toHaveStyle({ fill: wbColor.white, - stroke: lockedFigureColors.blue, + stroke: lockedFigureColors.green, }); }); }); @@ -316,12 +316,12 @@ describe("locked layer", () => { // Assert expect(points[0]).toHaveStyle({ - fill: lockedFigureColors.blue, - stroke: lockedFigureColors.blue, + fill: lockedFigureColors.green, + stroke: lockedFigureColors.green, }); expect(points[1]).toHaveStyle({ fill: wbColor.white, - stroke: lockedFigureColors.blue, + stroke: lockedFigureColors.green, }); }); @@ -394,8 +394,8 @@ describe("locked layer", () => { // Assert expect(lines).toHaveLength(2); - expect(lines[0]).toHaveStyle({stroke: lockedFigureColors.purple}); - expect(lines[1]).toHaveStyle({stroke: lockedFigureColors.green}); + expect(lines[0]).toHaveStyle({stroke: lockedFigureColors.green}); + expect(lines[1]).toHaveStyle({stroke: lockedFigureColors.grayH}); expect(ray).toHaveStyle({stroke: lockedFigureColors.pink}); }); @@ -419,20 +419,20 @@ describe("locked layer", () => { expect(linePoints).toHaveLength(4); // Two points for each line expect(linePoints[0]).toHaveStyle({ - fill: lockedFigureColors.purple, - stroke: lockedFigureColors.purple, + fill: lockedFigureColors.green, + stroke: lockedFigureColors.green, }); expect(linePoints[1]).toHaveStyle({ fill: wbColor.white, - stroke: lockedFigureColors.purple, + stroke: lockedFigureColors.green, }); expect(linePoints[2]).toHaveStyle({ fill: wbColor.white, - stroke: lockedFigureColors.green, + stroke: lockedFigureColors.grayH, }); expect(linePoints[3]).toHaveStyle({ - fill: lockedFigureColors.green, - stroke: lockedFigureColors.green, + fill: lockedFigureColors.grayH, + stroke: lockedFigureColors.grayH, }); expect(rayPoints[0]).toHaveStyle({ fill: wbColor.white,