diff --git a/.changeset/tricky-rice-nail.md b/.changeset/tricky-rice-nail.md new file mode 100644 index 0000000000..ff6b820670 --- /dev/null +++ b/.changeset/tricky-rice-nail.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": minor +--- + +Make the `mafs.point` flag control whether point graphs with fixed numbers of points should use Mafs. Previously, turning on the `mafs.point` flag would enable Mafs for point graphs with unlimited points as well. diff --git a/packages/perseus/src/widgets/interactive-graph.test.tsx b/packages/perseus/src/widgets/interactive-graph.test.tsx index efa77e5479..9303a1b772 100644 --- a/packages/perseus/src/widgets/interactive-graph.test.tsx +++ b/packages/perseus/src/widgets/interactive-graph.test.tsx @@ -1,7 +1,15 @@ import invariant from "tiny-invariant"; -import InteractiveGraph, {type Rubric} from "./interactive-graph"; +import InteractiveGraph, { + type Rubric, + shouldUseMafs, +} from "./interactive-graph"; +import type { + PerseusGraphTypeLinear, + PerseusGraphTypePoint, + PerseusGraphTypePolygon, +} from "../perseus-types"; import type {PerseusGraphType} from "@khanacademy/perseus"; function createRubric(graph: PerseusGraphType): Rubric { @@ -182,3 +190,137 @@ describe("InteractiveGraph.validate on a segment question", () => { ]); }); }); + +describe("shouldUseMafs", () => { + it("is false given no mafs flags", () => { + const graph: PerseusGraphTypeLinear = { + type: "linear", + }; + const mafsFlags = undefined; + + expect(shouldUseMafs(mafsFlags, graph)).toBe(false); + }); + + it("is false when mafs flags is a boolean", () => { + // boolean values aren't valid; we expect the mafs flags to be an + // object. + const graph: PerseusGraphTypeLinear = { + type: "linear", + }; + const mafsFlags = true; + + expect(shouldUseMafs(mafsFlags, graph)).toBe(false); + }); + + it("is false for a point graph when the feature flag is off", () => { + const graph: PerseusGraphTypePoint = { + type: "point", + numPoints: 42, + }; + const mafsFlags = {}; + + expect(shouldUseMafs(mafsFlags, graph)).toBe(false); + }); + + it("is true for a point graph when the `point` feature flag is on", () => { + const graph: PerseusGraphTypePoint = { + type: "point", + numPoints: 42, + }; + const mafsFlags = { + point: true, + }; + + expect(shouldUseMafs(mafsFlags, graph)).toBe(true); + }); + + it("is false for a point graph with numPoints = 'unlimited'", () => { + const graph: PerseusGraphTypePoint = { + type: "point", + numPoints: "unlimited", + }; + const mafsFlags = { + point: true, + }; + + expect(shouldUseMafs(mafsFlags, graph)).toBe(false); + }); + + it("is true for a point graph without numPoints set when the feature flag is on", () => { + // numPoints defaults to 1 + const graph: PerseusGraphTypePoint = { + type: "point", + }; + const mafsFlags = { + point: true, + }; + + expect(shouldUseMafs(mafsFlags, graph)).toBe(true); + }); + + it("is false for a polygon graph when the feature flag is off", () => { + const graph: PerseusGraphTypePolygon = { + type: "polygon", + numSides: 3, + }; + const mafsFlags = {}; + + expect(shouldUseMafs(mafsFlags, graph)).toBe(false); + }); + + it("is true for a polygon graph when the feature flag is on", () => { + const graph: PerseusGraphTypePolygon = { + type: "polygon", + numSides: 3, + }; + const mafsFlags = { + polygon: true, + }; + + expect(shouldUseMafs(mafsFlags, graph)).toBe(true); + }); + + it("is false for a polygon graph when numSides is 'unlimited'", () => { + const graph: PerseusGraphTypePolygon = { + type: "polygon", + numSides: "unlimited", + }; + const mafsFlags = { + polygon: true, + }; + + expect(shouldUseMafs(mafsFlags, graph)).toBe(false); + }); + + it("is true for a polygon graph when numSides is not set", () => { + // numSides defaults to 3 + const graph: PerseusGraphTypePolygon = { + type: "polygon", + }; + const mafsFlags = { + polygon: true, + }; + + expect(shouldUseMafs(mafsFlags, graph)).toBe(true); + }); + + it("is false for a linear graph when the feature flag is off", () => { + const graph: PerseusGraphTypeLinear = { + type: "linear", + }; + const mafsFlags = {}; + + expect(shouldUseMafs(mafsFlags, graph)).toBe(false); + }); + + it("is true for a linear graph when the feature flag is on", () => { + const graph: PerseusGraphTypeLinear = { + type: "linear", + }; + const mafsFlags = { + linear: true, + }; + + expect(shouldUseMafs(mafsFlags, graph)).toBe(true); + }); +}); diff --git a/packages/perseus/src/widgets/interactive-graph.tsx b/packages/perseus/src/widgets/interactive-graph.tsx index 00e6c9f8bb..2b4c8c39c7 100644 --- a/packages/perseus/src/widgets/interactive-graph.tsx +++ b/packages/perseus/src/widgets/interactive-graph.tsx @@ -15,18 +15,18 @@ import Util from "../util"; import KhanColors from "../util/colors"; import { angleMeasures, + canonicalSineCoefficients, ccw, collinear, + getLineEquation, + getLineIntersection, intersects, lawOfCosines, magnitude, rotate, - vector, sign, - getLineEquation, - getLineIntersection, - canonicalSineCoefficients, similar, + vector, } from "../util/geometry"; import GraphUtils from "../util/graph-utils"; import {polar} from "../util/graphie"; @@ -53,8 +53,8 @@ import type { } from "../types"; import type { QuadraticCoefficient, - SineCoefficient, Range, + SineCoefficient, } from "../util/geometry"; const {DeprecationMixin} = Util; @@ -1815,20 +1815,9 @@ class InteractiveGraph extends React.Component { } render() { - if ( - this.props.graph.type === "polygon" && - this.props.graph.numSides === "unlimited" - ) { - return ( - - ); - } - // Mafs shim - if (this.props.apiOptions?.flags?.["mafs"]?.[this.props.graph.type]) { + const mafsFlags = this.props.apiOptions?.flags?.["mafs"]; + if (shouldUseMafs(mafsFlags, this.props.graph)) { const box = getInteractiveBoxFromSizeClass( this.props.containerSizeClass, ); @@ -2675,6 +2664,37 @@ class InteractiveGraph extends React.Component { } } +// exported for testing +export function shouldUseMafs( + mafsFlags: Record | undefined | boolean, + graph: PerseusGraphType, +): boolean { + if (typeof mafsFlags === "boolean" || typeof mafsFlags === "undefined") { + return false; + } + + switch (graph.type) { + case "point": + if (graph.numPoints === UNLIMITED) { + // TODO(benchristel): add a feature flag for the "unlimited" + // case once we've implemented point graphs with unlimited + // points + return false; + } + return Boolean(mafsFlags["point"]); + case "polygon": + if (graph.numSides === UNLIMITED) { + // TODO(benchristel): add a feature flag for the "unlimited" + // case once we've implemented polygon graphs with unlimited + // sides + return false; + } + return Boolean(mafsFlags["polygon"]); + default: + return Boolean(mafsFlags[graph.type]); + } +} + export default { name: "interactive-graph", displayName: "Interactive graph",