Skip to content

Commit

Permalink
[Locked Figure Labels] View locked vector labels (#1650)
Browse files Browse the repository at this point in the history
## 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)

<img width="435" alt="image" src="https://github.com/user-attachments/assets/bbad65a9-6608-44f9-ace6-b227d1d6b14f">

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: #1650
  • Loading branch information
nishasy committed Sep 24, 2024
1 parent b9d1af1 commit 03cddb6
Show file tree
Hide file tree
Showing 12 changed files with 144 additions and 36 deletions.
6 changes: 6 additions & 0 deletions .changeset/strange-houses-breathe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": minor
"@khanacademy/perseus-editor": minor
---

[Locked Figure Labels] View locked vector labels
Original file line number Diff line number Diff line change
Expand Up @@ -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"];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
}}
Expand All @@ -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,
},
},
}}
Expand All @@ -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 (
<EditorPageWithStorybookPreview
Expand All @@ -207,6 +201,7 @@ export const MafsWithLockedPointLabelsFlag = (): React.ReactElement => {
"interactive-graph-locked-features-labels": true,
"locked-point-labels": true,
"locked-line-labels": false,
"locked-vector-labels": false,
},
},
}}
Expand All @@ -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 (
<EditorPageWithStorybookPreview
Expand All @@ -233,6 +220,7 @@ export const MafsWithLockedLineLabelsFlag = (): React.ReactElement => {
"interactive-graph-locked-features-labels": true,
"locked-point-labels": false,
"locked-line-labels": true,
"locked-vector-labels": false,
},
},
}}
Expand All @@ -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 (
<EditorPageWithStorybookPreview
apiOptions={{
flags: {
mafs: {
...flags.mafs,
"interactive-graph-locked-features-labels": true,
"locked-point-labels": false,
"locked-line-labels": false,
"locked-vector-labels": true,
},
},
}}
question={segmentWithLockedFigures}
/>
);
};

export const WithSaveWarnings = (): React.ReactElement => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ describe("getDefaultFigureForType", () => {
[2, 2],
],
color: "grayH",
labels: [],
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export function getDefaultFigureForType(type: LockedFigureType): LockedFigure {
[2, 2],
],
color: DEFAULT_COLOR,
labels: [],
};
case "ellipse":
return {
Expand Down
1 change: 1 addition & 0 deletions packages/perseus/src/perseus-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
5 changes: 5 additions & 0 deletions packages/perseus/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<React.Fragment key={i}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"];

Expand All @@ -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",
},
],
},
]);
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {line as kline} from "@khanacademy/kmath";
import {vec} from "mafs";

import type {Coord} from "../../interactive2/types";
import type {
Expand All @@ -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;
Expand Down Expand Up @@ -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",
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
circleQuestionWithDefaultCorrect,
graphWithLabeledLine,
graphWithLabeledPoint,
graphWithLabeledVector,
interactiveGraphWithAriaLabel,
linearQuestion,
linearQuestionWithDefaultCorrect,
Expand Down Expand Up @@ -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, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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(
[
Expand All @@ -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(
[
Expand All @@ -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",
Expand Down Expand Up @@ -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();

0 comments on commit 03cddb6

Please sign in to comment.