diff --git a/.changeset/sweet-seas-dance.md b/.changeset/sweet-seas-dance.md new file mode 100644 index 0000000000..f797de7265 --- /dev/null +++ b/.changeset/sweet-seas-dance.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +Bug fix to ensure that new angle graphs are scored correctly. diff --git a/packages/perseus/src/widgets/interactive-graph.tsx b/packages/perseus/src/widgets/interactive-graph.tsx index ebaefa0ed8..0b647e3dc8 100644 --- a/packages/perseus/src/widgets/interactive-graph.tsx +++ b/packages/perseus/src/widgets/interactive-graph.tsx @@ -143,6 +143,10 @@ type DefaultProps = { graph: Props["graph"]; }; +// (LEMS-2190): Move the Mafs Angle Graph coordinate reversal logic in interactive-graph-state.ts +// to this file when we remove the legacy graph. This logic allows us to support bi-directional angles +// for the new (non-reflexive) Mafs graphs, while maintaining the same scoring behaviour as the legacy graph. +// Once the legacy graph is removed, we should move this logic directly into the validate function below. class LegacyInteractiveGraph extends React.Component { static contextType = PerseusI18nContext; declare context: React.ContextType; diff --git a/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-state.test.ts b/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-state.test.ts index bfad73f1bc..3ecb2f9422 100644 --- a/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-state.test.ts +++ b/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-state.test.ts @@ -1,8 +1,27 @@ +import invariant from "tiny-invariant"; + import {getGradableGraph} from "./interactive-graph-state"; import type {InteractiveGraphState} from "../types"; import type {PerseusGraphType} from "@khanacademy/perseus"; +const defaultAngleState: InteractiveGraphState = { + type: "angle", + coords: [ + [5, 0], + [0, 0], + [5, 5], + ], + hasBeenInteractedWith: true, + range: [ + [-10, 10], + [-10, 10], + ], + snapStep: [1, 1], + showAngles: true, + allowReflexAngles: false, +}; + describe("getGradableGraph", () => { /** * Originally `getGradableGraph` was returning a PerseusGraphType with just a @@ -28,4 +47,74 @@ describe("getGradableGraph", () => { expect(result).toEqual(initialGraph); }); + + // (LEMS-2190): This test is to ensure that the new Mafs graph is reversing coordinates when the angle graph + // is reflexive in a clockwise direction in order to temporarily maintain the same angle scoring with the + // legacy graph. This logic (& the tests) should be moved to the scoring function after removing the legacy graph. + it("returns reversed coordinates if the angle graph is reflexive when not allowed", () => { + const state: InteractiveGraphState = { + ...defaultAngleState, + coords: [ + [-5, 0], + [0, 0], + [5, 5], + ], + hasBeenInteractedWith: true, + }; + const initialGraph: PerseusGraphType = { + type: "angle", + }; + const result = getGradableGraph(state, initialGraph); + invariant(result.type === "angle"); + expect(result.coords).toEqual([ + [5, 5], + [0, 0], + [-5, 0], + ]); + }); + + it("does not reverse coordinates if the angle graph is not reflexive", () => { + const state: InteractiveGraphState = { + ...defaultAngleState, + coords: [ + [5, 0], + [0, 0], + [5, 5], + ], + hasBeenInteractedWith: true, + }; + const initialGraph: PerseusGraphType = { + type: "angle", + }; + const result = getGradableGraph(state, initialGraph); + invariant(result.type === "angle"); + expect(result.coords).toEqual([ + [5, 0], + [0, 0], + [5, 5], + ]); + }); + + it("does not reverse coordinates if the angle graph is allowed to be reflexive", () => { + const state: InteractiveGraphState = { + ...defaultAngleState, + coords: [ + [5, 0], + [0, 0], + [-5, -5], + ], + hasBeenInteractedWith: true, + allowReflexAngles: true, + }; + const initialGraph: PerseusGraphType = { + type: "angle", + }; + const result = getGradableGraph(state, initialGraph); + invariant(result.type === "angle"); + expect(result.coords).toEqual([ + [5, 0], + [0, 0], + [-5, -5], + ]); + }); }); diff --git a/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-state.ts b/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-state.ts index 7f47d171c1..987fb0b1c8 100644 --- a/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-state.ts +++ b/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-state.ts @@ -1,5 +1,7 @@ +import {clockwise} from "../../../util/geometry"; + import type {CircleGraphState, InteractiveGraphState} from "../types"; -import type {PerseusGraphType} from "@khanacademy/perseus"; +import type {Coord, PerseusGraphType} from "@khanacademy/perseus"; export function getGradableGraph( state: InteractiveGraphState, @@ -77,9 +79,24 @@ export function getGradableGraph( } if (state.type === "angle" && initialGraph.type === "angle") { + // We're going to reverse the coords for scoring if the angle is clockwise and + // we don't allow reflex angles. This is because the angle is largely defined + // by the order of the points in the coords array, and we want to maintain + // the same angle scoring with the legacy graph (which had a bug). + // (LEMS-2190): When we remove the legacy graph, move this logic to the scoring function. + const areClockwise = clockwise([ + state.coords[0], + state.coords[2], + state.coords[1], + ]); + const shouldReverseCoords = areClockwise && !state.allowReflexAngles; + const coords: [Coord, Coord, Coord] = shouldReverseCoords + ? (state.coords.slice().reverse() as [Coord, Coord, Coord]) + : state.coords; + return { ...initialGraph, - coords: state.coords, + coords, allowReflexAngles: state.allowReflexAngles, }; }