Skip to content

Commit

Permalink
[Interactive Graph: test utils] Do not set the coords field by defaul…
Browse files Browse the repository at this point in the history 
…t in the builder (#1505)

## Summary:
The builder should not be setting the `coords` inside the graph's `correct` field by default.
The correct answer `coords` get set after the content authors interact with the graph's
"correct answer" preview.

When the coords are undefined, the graph renders with the default starting coords. If we were
to pass in the coords explicitly in the builder, it would (a) not be an accurate representation
of the initial state of a graph, and (b) we would have to make sure we use the initialization
logic to confirm that we're setting coords based on the number of segments, sides, or points
for segment graphs, polygon graphs, and point graphs respectively.

Updating the coords to be undefined by default inside the builder also fixes an issue where
the "correct answer" preview was showing the wrong graph values because the values were
set to the wrong default values in the builder.

Issue: none

## Test plan:
`yarn jest packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.test.ts`

Storybook
- Segments
  - http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--interactive-graph-segments
- Polygons
  - http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--interactive-graph-polygon

## Screenshots

| Before | After |
| --- | --- |
| <img width="923" alt="Screenshot 2024-08-08 at 4 26 16 PM" src="https://github.com/user-attachments/assets/eace2258-44b5-4d2f-a785-adbaecdd5985"> | <img width="929" alt="Screenshot 2024-08-08 at 4 26 22 PM" src="https://github.com/user-attachments/assets/6b6abe81-e19d-4342-bfcf-f84b189531c1"> |
| <img width="925" alt="Screenshot 2024-08-08 at 4 26 47 PM" src="https://github.com/user-attachments/assets/5004b44f-ad74-4dac-ba55-5d24a33dc8a3"> | <img width="922" alt="Screenshot 2024-08-08 at 4 26 53 PM" src="https://github.com/user-attachments/assets/13cbf50c-2fd6-47a4-bf7f-851c6ace18c3"> |

Author: nishasy

Reviewers: benchristel

Required Reviewers:

Approved By: benchristel

Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ gerald, ⏭️  Publish npm snapshot, ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), 🚫 Upload Coverage, ✅ gerald, ⏭️  Publish npm snapshot, 🚫 Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), 🚫 Cypress (ubuntu-latest, 20.x), 🚫 Jest Coverage (ubuntu-latest, 20.x), 🚫 Check for .changeset entries for all changed files (ubuntu-latest, 20.x), 🚫 Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1505
  • Loading branch information
@nishasy
nishasy authored Aug 9, 2024
1 parent e2b29a5 commit 3f9cc14
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 93 deletions.
5 changes: 5 additions & 0 deletions .changeset/wise-lamps-tell.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

[Interactive Graph: test utils] Do not set the coords field by default in the builder
40 changes: 0 additions & 40 deletions packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ describe("InteractiveGraphQuestionBuilder", () => {
correct: {
type: "segment",
numSegments: 1,
coords: [expect.anything()],
},
}),
);
Expand All @@ -168,7 +167,6 @@ describe("InteractiveGraphQuestionBuilder", () => {
correct: {
type: "segment",
numSegments: 2,
coords: [expect.anything(), expect.anything()],
},
}),
);
Expand Down Expand Up @@ -230,10 +228,6 @@ describe("InteractiveGraphQuestionBuilder", () => {
graph: {type: "linear"},
correct: {
type: "linear",
coords: [
[-5, 5],
[5, 5],
],
},
}),
);
Expand Down Expand Up @@ -283,16 +277,6 @@ describe("InteractiveGraphQuestionBuilder", () => {
graph: {type: "linear-system"},
correct: {
type: "linear-system",
coords: [
[
[-5, 5],
[5, 5],
],
[
[-5, -5],
[5, -5],
],
],
},
}),
);
Expand Down Expand Up @@ -366,10 +350,6 @@ describe("InteractiveGraphQuestionBuilder", () => {
graph: {type: "ray"},
correct: {
type: "ray",
coords: [
[-5, 5],
[5, 5],
],
},
}),
);
Expand Down Expand Up @@ -455,11 +435,6 @@ describe("InteractiveGraphQuestionBuilder", () => {
graph: {type: "quadratic"},
correct: {
type: "quadratic",
coords: [
[-5, 5],
[0, -5],
[5, 5],
],
},
}),
);
Expand Down Expand Up @@ -513,10 +488,6 @@ describe("InteractiveGraphQuestionBuilder", () => {
graph: {type: "sinusoid"},
correct: {
type: "sinusoid",
coords: [
[0, 0],
[3, 2],
],
},
}),
);
Expand Down Expand Up @@ -573,11 +544,6 @@ describe("InteractiveGraphQuestionBuilder", () => {
showAngles: false,
showSides: false,
snapTo: "grid",
coords: [
[3, -2],
[0, 4],
[-3, -2],
],
}),
}),
);
Expand Down Expand Up @@ -688,12 +654,6 @@ describe("InteractiveGraphQuestionBuilder", () => {
graph: {type: "angle"},
correct: {
type: "angle",
coords: [
// Default correct answer is 20 degree angle at (0, 0)
[6.994907182610915, 0],
[0, 0],
[6.5778483455013586, 2.394141003279681],
],
},
}),
);
Expand Down
70 changes: 17 additions & 53 deletions packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ interface InteractiveFigureConfig {

class SegmentGraphConfig implements InteractiveFigureConfig {
private numSegments: number;
private coords: CollinearTuple[];
private coords?: CollinearTuple[];
private startCoords?: CollinearTuple[];

constructor(options?: {
Expand All @@ -417,12 +417,7 @@ class SegmentGraphConfig implements InteractiveFigureConfig {
options?.startCoords?.length ??
options?.coords?.length ??
1;
this.coords =
options?.coords ??
repeat(this.numSegments, () => [
[-5, 5],
[5, 5],
]);
this.coords = options?.coords;
this.startCoords = options?.startCoords;
}

Expand All @@ -445,17 +440,14 @@ class SegmentGraphConfig implements InteractiveFigureConfig {

class LinearGraphConfig implements InteractiveFigureConfig {
private startCoords?: CollinearTuple;
private correctCoords: CollinearTuple;
private correctCoords?: CollinearTuple;

constructor(options?: {
coords?: CollinearTuple;
startCoords?: CollinearTuple;
}) {
this.startCoords = options?.startCoords;
this.correctCoords = options?.coords ?? [
[-5, 5],
[5, 5],
];
this.correctCoords = options?.coords;
}

correct(): PerseusGraphType {
Expand All @@ -472,23 +464,14 @@ class LinearGraphConfig implements InteractiveFigureConfig {

class LinearSystemGraphConfig implements InteractiveFigureConfig {
private startCoords?: CollinearTuple[];
private correctCoords: CollinearTuple[];
private correctCoords?: CollinearTuple[];

constructor(options?: {
coords?: CollinearTuple[];
startCoords?: CollinearTuple[];
}) {
this.startCoords = options?.startCoords;
this.correctCoords = options?.coords ?? [
[
[-5, 5],
[5, 5],
],
[
[-5, -5],
[5, -5],
],
];
this.correctCoords = options?.coords;
}

correct(): PerseusGraphType {
Expand All @@ -511,10 +494,7 @@ class RayGraphConfig implements InteractiveFigureConfig {
coords?: CollinearTuple;
startCoords?: CollinearTuple;
}) {
this.coords = options?.coords ?? [
[-5, 5],
[5, 5],
];
this.coords = options?.coords;
this.startCoords = options?.startCoords;
}

Expand Down Expand Up @@ -579,11 +559,7 @@ class QuadraticGraphConfig implements InteractiveFigureConfig {
coords?: [Coord, Coord, Coord];
startCoords?: [Coord, Coord, Coord];
}) {
this.coords = options?.coords ?? [
[-5, 5],
[0, -5],
[5, 5],
];
this.coords = options?.coords;
this.startCoords = options?.startCoords;
}

Expand All @@ -607,10 +583,7 @@ class SinusoidGraphConfig implements InteractiveFigureConfig {
coords?: [Coord, Coord];
startCoords?: [Coord, Coord];
}) {
this.coords = options?.coords ?? [
[0, 0],
[3, 2],
];
this.coords = options?.coords;
this.startCoords = options?.startCoords;
}

Expand All @@ -632,7 +605,7 @@ class PolygonGraphConfig implements InteractiveFigureConfig {
private numSides: number | "unlimited";
private showAngles: boolean;
private showSides: boolean;
private coords: Coord[];
private coords?: Coord[];
private startCoords?: Coord[];

constructor(
Expand All @@ -648,14 +621,14 @@ class PolygonGraphConfig implements InteractiveFigureConfig {
) {
this.snapTo = snapTo ?? "grid";
this.match = options?.match;
this.numSides = options?.numSides ?? 3;
this.numSides =
options?.numSides ??
options?.startCoords?.length ??
options?.coords?.length ??
3;
this.showAngles = options?.showAngles ?? false;
this.showSides = options?.showSides ?? false;
this.coords = options?.coords ?? [
[3, -2],
[0, 4],
[-3, -2],
];
this.coords = options?.coords;
this.startCoords = options?.startCoords;
}
correct(): PerseusGraphType {
Expand Down Expand Up @@ -734,12 +707,7 @@ class AngleGraphConfig implements InteractiveFigureConfig {
snapDegrees?: number;
match?: "congruent";
}) {
// Default correct answer is 20 degree angle at (0, 0)
this.coords = options?.coords ?? [
[6.994907182610915, 0],
[0, 0],
[6.5778483455013586, 2.394141003279681],
];
this.coords = options?.coords;
this.startCoords = options?.startCoords;
this.showAngles = options?.showAngles;
this.allowReflexAngles = options?.allowReflexAngles;
Expand Down Expand Up @@ -772,7 +740,3 @@ class AngleGraphConfig implements InteractiveFigureConfig {
};
}
}

function repeat<T>(n: number, makeItem: () => T): T[] {
return new Array(n).fill(null).map(makeItem);
}

0 comments on commit 3f9cc14

Please sign in to comment.