From 0bec013e89c70dd431f86b4872dd3378ed29e110 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Tue, 6 Aug 2024 09:56:38 -0700 Subject: [PATCH] [Hint Mode: Start Coords] Add start coords UI for polygon graphs (snap to grid only) (#1488) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: Add the UI to specify start coords for Polygon graph type (not unlimited, snap to grid only) - Add the polygon graph type go start-coords-settings.tsx - No need to create a separate start-coord-polygon.tsx file because it's identical to start-coord-point.tsx. Reusing that component. - Update the selection logic in interactive-graph-editor.tsx so it properly updates the graph with the new snapTo value. (It was stuck on the previously selected value before.) - Add the start coords UI polygon flag - Update the flag tests so they actually test what they're supposed to Issue: https://khanacademy.atlassian.net/browse/LEMS-2072 ## Test plan: `yarn jest` Storybook - Polygon story without default graph settings - http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--interactive-graph-polygon - For default graph settings, go to a different story and select polygon from the dropdown - http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--interactive-graph-point Screenshot 2024-08-05 at 5 29 35 PM Author: nishasy Reviewers: nishasy, Myranae Required Reviewers: Approved By: Myranae Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (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), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1488 --- .changeset/unlucky-lamps-draw.md | 6 ++ .../src/__stories__/flags-for-api-options.ts | 1 + .../__tests__/start-coords-settings.test.tsx | 86 +++++++++++++++++++ .../src/components/__tests__/util.test.ts | 39 +++++++++ .../src/components/start-coords-settings.tsx | 8 +- .../perseus-editor/src/components/util.ts | 19 ++++ .../interactive-graph-editor.test.tsx | 70 ++++++++++++++- .../src/widgets/interactive-graph-editor.tsx | 5 +- packages/perseus/src/index.ts | 1 + packages/perseus/src/types.ts | 5 ++ .../reducer/initialize-graph-state.ts | 2 +- 11 files changed, 235 insertions(+), 7 deletions(-) create mode 100644 .changeset/unlucky-lamps-draw.md diff --git a/.changeset/unlucky-lamps-draw.md b/.changeset/unlucky-lamps-draw.md new file mode 100644 index 0000000000..e08481ca5a --- /dev/null +++ b/.changeset/unlucky-lamps-draw.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/perseus": minor +"@khanacademy/perseus-editor": minor +--- + +[Hint Mode: Start Coords] Add start coords UI for polygon graphs (snap to grid only) 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 de0311ec20..7b454dd203 100644 --- a/packages/perseus-editor/src/__stories__/flags-for-api-options.ts +++ b/packages/perseus-editor/src/__stories__/flags-for-api-options.ts @@ -21,6 +21,7 @@ export const flags = { "start-coords-ui-phase-1": true, "start-coords-ui-phase-2": true, "start-coords-ui-point": true, + "start-coords-ui-polygon": true, }, } satisfies APIOptions["flags"]; diff --git a/packages/perseus-editor/src/components/__tests__/start-coords-settings.test.tsx b/packages/perseus-editor/src/components/__tests__/start-coords-settings.test.tsx index f92a25c273..fd169b014b 100644 --- a/packages/perseus-editor/src/components/__tests__/start-coords-settings.test.tsx +++ b/packages/perseus-editor/src/components/__tests__/start-coords-settings.test.tsx @@ -606,4 +606,90 @@ describe("StartCoordSettings", () => { }, ); }); + + describe("polygon graph", () => { + test("shows the start coordinates UI: 3 sides (default)", () => { + // Arrange + + // Act + render( + {}} + />, + {wrapper: RenderStateRoot}, + ); + + // Assert + expect(screen.getByText("Start coordinates")).toBeInTheDocument(); + expect(screen.getByText("Point 1:")).toBeInTheDocument(); + expect(screen.getByText("Point 2:")).toBeInTheDocument(); + expect(screen.getByText("Point 3:")).toBeInTheDocument(); + }); + + test("shows the start coordinates UI: 6 sides", () => { + // Arrange + + // Act + render( + {}} + />, + {wrapper: RenderStateRoot}, + ); + + // Assert + expect(screen.getByText("Start coordinates")).toBeInTheDocument(); + expect(screen.getByText("Point 1:")).toBeInTheDocument(); + expect(screen.getByText("Point 2:")).toBeInTheDocument(); + expect(screen.getByText("Point 3:")).toBeInTheDocument(); + expect(screen.getByText("Point 4:")).toBeInTheDocument(); + expect(screen.getByText("Point 5:")).toBeInTheDocument(); + expect(screen.getByText("Point 6:")).toBeInTheDocument(); + }); + + test.each` + pointIndex | coord + ${0} | ${"x"} + ${0} | ${"y"} + ${1} | ${"x"} + ${1} | ${"y"} + ${2} | ${"x"} + ${2} | ${"y"} + `( + "calls onChange when $coord coord is changed (line $pointIndex)", + async ({pointIndex, coord}) => { + // Arrange + const onChangeMock = jest.fn(); + render( + , + ); + + // Act + const input = screen.getAllByRole("spinbutton", { + name: `${coord}`, + })[pointIndex]; + await userEvent.clear(input); + await userEvent.type(input, "101"); + + // Assert + const expectedCoords = [ + [3, -2], + [0, 4], + [-3, -2], + ]; + expectedCoords[pointIndex][coord === "x" ? 0 : 1] = 101; + + expect(onChangeMock).toHaveBeenLastCalledWith(expectedCoords); + }, + ); + }); }); diff --git a/packages/perseus-editor/src/components/__tests__/util.test.ts b/packages/perseus-editor/src/components/__tests__/util.test.ts index d2a4ee565b..b27986297c 100644 --- a/packages/perseus-editor/src/components/__tests__/util.test.ts +++ b/packages/perseus-editor/src/components/__tests__/util.test.ts @@ -326,6 +326,45 @@ describe("getDefaultGraphStartCoords", () => { [5, 0], ]); }); + + test("should get default start coords for a polygon graph, triangle (default)", () => { + // Arrange + const graph: PerseusGraphType = {type: "polygon"}; + const range = [ + [-10, 10], + [-10, 10], + ] satisfies [Range, Range]; + const step = [1, 1] satisfies [number, number]; + + // Act + const defaultCoords = getDefaultGraphStartCoords(graph, range, step); + + expect(defaultCoords).toEqual([ + [3, -2], + [0, 4], + [-3, -2], + ]); + }); + + test("should get default start coords for a polygon graph, square", () => { + // Arrange + const graph: PerseusGraphType = {type: "polygon", numSides: 4}; + const range = [ + [-10, 10], + [-10, 10], + ] satisfies [Range, Range]; + const step = [1, 1] satisfies [number, number]; + + // Act + const defaultCoords = getDefaultGraphStartCoords(graph, range, step); + + expect(defaultCoords).toEqual([ + [3, -3], + [3, 3], + [-3, 3], + [-3, -3], + ]); + }); }); describe("getSinusoidEquation", () => { diff --git a/packages/perseus-editor/src/components/start-coords-settings.tsx b/packages/perseus-editor/src/components/start-coords-settings.tsx index 7031cc4ed8..4bf5bc2947 100644 --- a/packages/perseus-editor/src/components/start-coords-settings.tsx +++ b/packages/perseus-editor/src/components/start-coords-settings.tsx @@ -4,6 +4,7 @@ import { getLineCoords, getLinearSystemCoords, getPointCoords, + getPolygonCoords, getQuadraticCoords, getSegmentCoords, getSinusoidCoords, @@ -91,8 +92,13 @@ const StartCoordsSettingsInner = (props: Props) => { onChange={onChange} /> ); + // Graphs with startCoords of type ReadonlyArray case "point": - const pointCoords = getPointCoords(props, range, step); + case "polygon": + const pointCoords = + type === "point" + ? getPointCoords(props, range, step) + : getPolygonCoords(props, range, step); return ( { const startCoordsPhase1 = flags?.mafs?.["start-coords-ui-phase-1"]; const startCoordsPhase2 = flags?.mafs?.["start-coords-ui-phase-2"]; const startCoordsPoint = flags?.mafs?.["start-coords-ui-point"]; + const startCoordsPolygon = flags?.mafs?.["start-coords-ui-polygon"]; if (startCoordsPhase1 && startCoordsUiPhase1Types.includes(graph.type)) { return true; @@ -292,5 +300,16 @@ export const shouldShowStartCoordsUI = (flags, graph) => { return true; } + if ( + startCoordsPolygon && + graph.type === "polygon" && + graph.numPoints !== "unlimited" && + // Pre-initialized graph with undefined snapTo value + // initializes to snapTo="grid" + (graph.snapTo === "grid" || graph.snapTo === undefined) + ) { + return true; + } + return false; }; diff --git a/packages/perseus-editor/src/widgets/__tests__/interactive-graph-editor.test.tsx b/packages/perseus-editor/src/widgets/__tests__/interactive-graph-editor.test.tsx index 7146dc161c..0936ec9759 100644 --- a/packages/perseus-editor/src/widgets/__tests__/interactive-graph-editor.test.tsx +++ b/packages/perseus-editor/src/widgets/__tests__/interactive-graph-editor.test.tsx @@ -700,9 +700,10 @@ describe("InteractiveGraphEditor", () => { ...flags, mafs: { ...flags.mafs, - "start-coords-ui-phase-1": shouldRender, + "start-coords-ui-phase-1": true, "start-coords-ui-phase-2": false, "start-coords-ui-point": false, + "start-coords-ui-polygon": false, }, }, }} @@ -738,7 +739,7 @@ describe("InteractiveGraphEditor", () => { ${"linear-system"} | ${false} ${"segment"} | ${false} ${"circle"} | ${false} - ${"quadratic"} | ${false} + ${"quadratic"} | ${true} ${"sinusoid"} | ${true} ${"polygon"} | ${false} ${"angle"} | ${false} @@ -759,8 +760,9 @@ describe("InteractiveGraphEditor", () => { mafs: { ...flags.mafs, "start-coords-ui-phase-1": false, - "start-coords-ui-phase-2": shouldRender, + "start-coords-ui-phase-2": true, "start-coords-ui-point": false, + "start-coords-ui-polygon": false, }, }, }} @@ -818,7 +820,67 @@ describe("InteractiveGraphEditor", () => { ...flags.mafs, "start-coords-ui-phase-1": false, "start-coords-ui-phase-2": false, - "start-coords-ui-point": shouldRender, + "start-coords-ui-point": true, + "start-coords-ui-polygon": false, + }, + }, + }} + graph={{type}} + correct={{type}} + />, + { + wrapper: RenderStateRoot, + }, + ); + + // Assert + if (shouldRender) { + expect( + await screen.findByRole("button", { + name: "Use default start coordinates", + }), + ).toBeInTheDocument(); + } else { + expect( + screen.queryByRole("button", { + name: "Use default start coordinates", + }), + ).toBeNull(); + } + }, + ); + + test.each` + type | shouldRender + ${"linear"} | ${false} + ${"ray"} | ${false} + ${"linear-system"} | ${false} + ${"segment"} | ${false} + ${"circle"} | ${false} + ${"quadratic"} | ${false} + ${"sinusoid"} | ${false} + ${"polygon"} | ${true} + ${"angle"} | ${false} + ${"point"} | ${false} + `( + "should render for $type graphs if polygon flag is on: $shouldRender", + async ({type, shouldRender}) => { + // Arrange + + // Act + render( + { coords: null, }; - this.props.onChange({correct: graph}); + this.props.onChange({ + correct: graph, + graph: graph, + }); }} style={styles.singleSelectShort} > diff --git a/packages/perseus/src/index.ts b/packages/perseus/src/index.ts index 697405741b..9829ead8f5 100644 --- a/packages/perseus/src/index.ts +++ b/packages/perseus/src/index.ts @@ -123,6 +123,7 @@ export { getLineCoords, getLinearSystemCoords, getPointCoords, + getPolygonCoords, getSegmentCoords, getSinusoidCoords, getQuadraticCoords, diff --git a/packages/perseus/src/types.ts b/packages/perseus/src/types.ts index ffa6282220..879f2e5d52 100644 --- a/packages/perseus/src/types.ts +++ b/packages/perseus/src/types.ts @@ -172,6 +172,11 @@ export const InteractiveGraphEditorFlags = [ * Includes point graph. */ "start-coords-ui-point", + /** + * Enables the UI for setting the start coordinates of a graph. + * Includes polygon graph. + */ + "start-coords-ui-polygon", ] as const; /** diff --git a/packages/perseus/src/widgets/interactive-graphs/reducer/initialize-graph-state.ts b/packages/perseus/src/widgets/interactive-graphs/reducer/initialize-graph-state.ts index 9dc1998422..2ee250cd91 100644 --- a/packages/perseus/src/widgets/interactive-graphs/reducer/initialize-graph-state.ts +++ b/packages/perseus/src/widgets/interactive-graphs/reducer/initialize-graph-state.ts @@ -277,7 +277,7 @@ export function getLinearSystemCoords( ); } -function getPolygonCoords( +export function getPolygonCoords( graph: PerseusGraphTypePolygon, range: [x: Interval, y: Interval], step: [x: number, y: number],