From 03cddb6c39570e87ff2437273eb1287ff1417eec Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Tue, 24 Sep 2024 09:52:50 -0700 Subject: [PATCH] [Locked Figure Labels] View locked vector labels (#1650) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: - Add feature flag for locked vector labels - Add `labels` to LockedVectorType - Update builder - Update stories and tests Issue: https://khanacademy.atlassian.net/browse/LEMS-2348 ## Test plan: - Go to http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--mafs-with-locked-vector-labels-flag - Confirm that the visible label is on the locked vector _only_ - Go to the point and line stories and confirm that the locked vector does not have a visible label (make sure the flags are separating the behaviors correctly) image Author: nishasy Reviewers: nishasy, benchristel, catandthemachines, anakaren-rojas Required Reviewers: Approved By: benchristel Checks: ✅ gerald, ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check builds for changes in size (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), ✅ Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1650 --- .changeset/strange-houses-breathe.md | 6 +++ .../src/__stories__/flags-for-api-options.ts | 1 + .../interactive-graph-editor.stories.tsx | 43 +++++++++-------- .../locked-figures/util.test.ts | 1 + .../locked-figures/util.ts | 1 + packages/perseus/src/perseus-types.ts | 1 + packages/perseus/src/types.ts | 5 ++ .../graph-locked-labels-layer.tsx | 6 ++- ...interactive-graph-question-builder.test.ts | 47 ++++++++++++++++++- .../interactive-graph-question-builder.ts | 22 ++++++--- .../interactive-graph.test.tsx | 29 ++++++++++++ .../interactive-graph.testdata.ts | 18 +++++-- 12 files changed, 144 insertions(+), 36 deletions(-) create mode 100644 .changeset/strange-houses-breathe.md diff --git a/.changeset/strange-houses-breathe.md b/.changeset/strange-houses-breathe.md new file mode 100644 index 0000000000..139c7d48c1 --- /dev/null +++ b/.changeset/strange-houses-breathe.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/perseus": minor +"@khanacademy/perseus-editor": minor +--- + +[Locked Figure Labels] View locked vector labels diff --git a/packages/perseus-editor/src/__stories__/flags-for-api-options.ts b/packages/perseus-editor/src/__stories__/flags-for-api-options.ts index f2b5306796..3232b091a6 100644 --- a/packages/perseus-editor/src/__stories__/flags-for-api-options.ts +++ b/packages/perseus-editor/src/__stories__/flags-for-api-options.ts @@ -18,6 +18,7 @@ export const flags = { "interactive-graph-locked-features-labels": true, "locked-point-labels": true, "locked-line-labels": true, + "locked-vector-labels": true, }, } satisfies APIOptions["flags"]; diff --git a/packages/perseus-editor/src/__stories__/interactive-graph-editor.stories.tsx b/packages/perseus-editor/src/__stories__/interactive-graph-editor.stories.tsx index 557e3706da..24f8ac5b46 100644 --- a/packages/perseus-editor/src/__stories__/interactive-graph-editor.stories.tsx +++ b/packages/perseus-editor/src/__stories__/interactive-graph-editor.stories.tsx @@ -155,6 +155,7 @@ export const MafsWithLockedFiguresCurrent = (): React.ReactElement => { "interactive-graph-locked-features-labels": false, "locked-point-labels": false, "locked-line-labels": false, + "locked-vector-labels": false, }, }, }} @@ -181,6 +182,7 @@ export const MafsWithLockedLabelsFlag = (): React.ReactElement => { "interactive-graph-locked-features-labels": true, "locked-point-labels": false, "locked-line-labels": false, + "locked-vector-labels": false, }, }, }} @@ -189,14 +191,6 @@ export const MafsWithLockedLabelsFlag = (): React.ReactElement => { ); }; -MafsWithLockedLabelsFlag.parameters = { - chromatic: { - // Disabling because this isn't visually testing anything on the - // initial load of the editor page. - disable: true, - }, -}; - export const MafsWithLockedPointLabelsFlag = (): React.ReactElement => { return ( { "interactive-graph-locked-features-labels": true, "locked-point-labels": true, "locked-line-labels": false, + "locked-vector-labels": false, }, }, }} @@ -215,14 +210,6 @@ export const MafsWithLockedPointLabelsFlag = (): React.ReactElement => { ); }; -MafsWithLockedPointLabelsFlag.parameters = { - chromatic: { - // Disabling because this isn't visually testing anything on the - // initial load of the editor page. - disable: true, - }, -}; - export const MafsWithLockedLineLabelsFlag = (): React.ReactElement => { return ( { "interactive-graph-locked-features-labels": true, "locked-point-labels": false, "locked-line-labels": true, + "locked-vector-labels": false, }, }, }} @@ -241,12 +229,23 @@ export const MafsWithLockedLineLabelsFlag = (): React.ReactElement => { ); }; -MafsWithLockedLineLabelsFlag.parameters = { - chromatic: { - // Disabling because this isn't visually testing anything on the - // initial load of the editor page. - disable: true, - }, +export const MafsWithLockedVectorLabelsFlag = (): React.ReactElement => { + return ( + + ); }; export const WithSaveWarnings = (): React.ReactElement => { diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.test.ts b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.test.ts index d52040ebbb..760193e1a8 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.test.ts +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.test.ts @@ -50,6 +50,7 @@ describe("getDefaultFigureForType", () => { [2, 2], ], color: "grayH", + labels: [], }); }); diff --git a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.ts b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.ts index be2804d9f3..2b2cb8102d 100644 --- a/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.ts +++ b/packages/perseus-editor/src/widgets/interactive-graph-editor/locked-figures/util.ts @@ -57,6 +57,7 @@ export function getDefaultFigureForType(type: LockedFigureType): LockedFigure { [2, 2], ], color: DEFAULT_COLOR, + labels: [], }; case "ellipse": return { diff --git a/packages/perseus/src/perseus-types.ts b/packages/perseus/src/perseus-types.ts index 9f2bd9861c..6163e7bc32 100644 --- a/packages/perseus/src/perseus-types.ts +++ b/packages/perseus/src/perseus-types.ts @@ -742,6 +742,7 @@ export type LockedVectorType = { type: "vector"; points: [tail: Coord, tip: Coord]; color: LockedFigureColor; + labels: LockedLabelType[]; }; export type LockedFigureFillType = "none" | "white" | "translucent" | "solid"; diff --git a/packages/perseus/src/types.ts b/packages/perseus/src/types.ts index 0ca8712620..41e7813ac5 100644 --- a/packages/perseus/src/types.ts +++ b/packages/perseus/src/types.ts @@ -165,6 +165,11 @@ export const InteractiveGraphLockedFeaturesFlags = [ * updated Interactive Graph widget. */ "locked-line-labels", + /** + * enables/disables the labels associated with locked vectors in the + * updated Interactive Graph widget. + */ + "locked-vector-labels", ] as const; /** diff --git a/packages/perseus/src/widgets/interactive-graphs/graph-locked-labels-layer.tsx b/packages/perseus/src/widgets/interactive-graphs/graph-locked-labels-layer.tsx index 579e267b1f..54cf2995de 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graph-locked-labels-layer.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graph-locked-labels-layer.tsx @@ -23,7 +23,11 @@ export default function GraphLockedLabelsLayer(props: Props) { (flags?.["mafs"]?.["locked-point-labels"] && figure.type === "point") || // Line flag + line type - (flags?.["mafs"]?.["locked-line-labels"] && figure.type === "line") + (flags?.["mafs"]?.["locked-line-labels"] && + figure.type === "line") || + // Vector flag + vector type + (flags?.["mafs"]?.["locked-vector-labels"] && + figure.type === "vector") ) { return ( diff --git a/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.test.ts b/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.test.ts index 91c8f4a0f4..42da9ff003 100644 --- a/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.test.ts +++ b/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.test.ts @@ -977,13 +977,17 @@ describe("InteractiveGraphQuestionBuilder", () => { [3, 4], ], color: "grayH", + labels: [], }, ]); }); - it("adds a locked vector with a specified color", () => { + it("adds a locked vector with options and minimal label", () => { const question: PerseusRenderer = interactiveGraphQuestionBuilder() - .addLockedVector([1, 2], [3, 4], "green") + .addLockedVector([1, 2], [3, 4], { + color: "green", + labels: [{text: "a label"}], + }) .build(); const graph = question.widgets["interactive-graph 1"]; @@ -995,6 +999,45 @@ describe("InteractiveGraphQuestionBuilder", () => { [3, 4], ], color: "green", + labels: [ + { + type: "label", + text: "a label", + coord: [2, 3], + color: "green", + size: "medium", + }, + ], + }, + ]); + }); + + it("adds a locked vector with options and specific label", () => { + const question: PerseusRenderer = interactiveGraphQuestionBuilder() + .addLockedVector([1, 2], [3, 4], { + color: "green", + labels: [{text: "a label", coord: [9, 9], size: "small"}], + }) + .build(); + const graph = question.widgets["interactive-graph 1"]; + + expect(graph.options.lockedFigures).toEqual([ + { + type: "vector", + points: [ + [1, 2], + [3, 4], + ], + color: "green", + labels: [ + { + type: "label", + text: "a label", + coord: [9, 9], + color: "green", + size: "small", + }, + ], }, ]); }); diff --git a/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.ts b/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.ts index 8feeb5c842..1ed32c8919 100644 --- a/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.ts +++ b/packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.ts @@ -1,4 +1,4 @@ -import {line as kline} from "@khanacademy/kmath"; +import {vec} from "mafs"; import type {Coord} from "../../interactive2/types"; import type { @@ -17,7 +17,7 @@ import type { PerseusGraphType, PerseusRenderer, } from "../../perseus-types"; -import type {Interval, vec} from "mafs"; +import type {Interval} from "mafs"; export type LockedFunctionOptions = { color?: LockedFigureColor; @@ -325,9 +325,7 @@ class InteractiveGraphQuestionBuilder { lineStyle: options?.lineStyle ?? "solid", labels: (options?.labels ?? []).map((label) => ({ type: "label", - coord: - label.coord ?? - ([...kline.midpoint([point1, point2])] as Coord), + coord: label.coord ?? vec.midpoint(point1, point2), text: label.text, color: options?.color ?? "grayH", size: label.size ?? "medium", @@ -354,12 +352,22 @@ class InteractiveGraphQuestionBuilder { addLockedVector( tail: vec.Vector2, tip: vec.Vector2, - color?: LockedFigureColor, + options?: { + color?: LockedFigureColor; + labels?: LockedFigureLabelOptions[]; + }, ): InteractiveGraphQuestionBuilder { const vector: LockedVectorType = { type: "vector", - color: color ?? "grayH", + color: options?.color ?? "grayH", points: [tail, tip], + labels: (options?.labels ?? []).map((label) => ({ + type: "label", + coord: label.coord ?? vec.midpoint(tail, tip), + text: label.text, + color: options?.color ?? "grayH", + size: label.size ?? "medium", + })), }; this.addLockedFigure(vector); return this; diff --git a/packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx b/packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx index 870388397c..1ac24812b3 100644 --- a/packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/interactive-graph.test.tsx @@ -24,6 +24,7 @@ import { circleQuestionWithDefaultCorrect, graphWithLabeledLine, graphWithLabeledPoint, + graphWithLabeledVector, interactiveGraphWithAriaLabel, linearQuestion, linearQuestionWithDefaultCorrect, @@ -1040,6 +1041,34 @@ describe("locked layer", () => { expect(point2Label).toHaveTextContent("point B"); }); + it("should render a locked label within a locked vector", async () => { + // Arrange + const {container} = renderQuestion(graphWithLabeledVector, { + flags: { + mafs: { + segment: true, + "interactive-graph-locked-features-labels": true, + "locked-vector-labels": true, + }, + }, + }); + + // Act + // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access + const labels = container.querySelectorAll(".locked-label"); + const label = labels[0]; + + // Assert + expect(labels).toHaveLength(1); + expect(label).toHaveTextContent("C"); + expect(label).toHaveStyle({ + color: lockedFigureColors["grayH"], + fontSize: "16px", + left: "280px", + top: "180px", + }); + }); + it("should have an aria-label and description if they are provided", async () => { // Arrange const {container} = renderQuestion(interactiveGraphWithAriaLabel, { diff --git a/packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts b/packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts index de6f8921b3..4dddd58e5a 100644 --- a/packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts +++ b/packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts @@ -707,7 +707,7 @@ export const segmentWithAllLockedRayVariations: PerseusRenderer = export const segmentWithLockedVectors: PerseusRenderer = interactiveGraphQuestionBuilder() .addLockedVector([0, 0], [2, 2]) - .addLockedVector([2, 2], [-2, 4], "green") + .addLockedVector([2, 2], [-2, 4], {color: "green"}) .build(); export const segmentWithLockedEllipses: PerseusRenderer = @@ -836,7 +836,10 @@ export const segmentWithLockedFigures: PerseusRenderer = showPoint2: true, labels: [{text: "B"}], }) - .addLockedVector([0, 0], [8, 2], "purple") + .addLockedVector([0, 0], [8, 2], { + color: "purple", + labels: [{text: "C"}], + }) .addLockedEllipse([0, 5], [4, 2], {angle: Math.PI / 4, color: "blue"}) .addLockedPolygon( [ @@ -863,7 +866,7 @@ export const staticGraphQuestion: PerseusRenderer = interactiveGraphQuestionBuilder() .addLockedPointAt(-7, -7) .addLockedLine([-7, -5], [2, -3]) - .addLockedVector([0, 0], [8, 2], "purple") + .addLockedVector([0, 0], [8, 2], {color: "purple"}) .addLockedEllipse([0, 5], [4, 2], {angle: Math.PI / 4, color: "blue"}) .addLockedPolygon( [ @@ -882,7 +885,7 @@ export const staticGraphQuestionWithAnotherWidget: () => PerseusRenderer = const result = interactiveGraphQuestionBuilder() .addLockedPointAt(-7, -7) .addLockedLine([-7, -5], [2, -3]) - .addLockedVector([0, 0], [8, 2], "purple") + .addLockedVector([0, 0], [8, 2], {color: "purple"}) .addLockedEllipse([0, 5], [4, 2], { angle: Math.PI / 4, color: "blue", @@ -968,3 +971,10 @@ export const graphWithLabeledLine: PerseusRenderer = labels: [{text: "B"}], }) .build(); + +export const graphWithLabeledVector: PerseusRenderer = + interactiveGraphQuestionBuilder() + .addLockedVector([0, 0], [8, 2], { + labels: [{text: "C"}], + }) + .build();