From 0b625f56098c4db142891ab4ffed2b2300924711 Mon Sep 17 00:00:00 2001 From: Nisha Yerunkar Date: Mon, 5 Aug 2024 15:06:35 -0700 Subject: [PATCH] [Hint Mode: Start Coords] Add start coords UI for point graphs (limited only) (#1486) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: Add the UI to specify start coords for Point graph type. - Add the point graph type to start-coord-settings.tsx - Create a start-coords-point.tsx file with the main UI - Add the start coords UI point flag - Move the logic for determining whether the UI should be shown based on the feature flags out into the util file. Issue: https://khanacademy.atlassian.net/browse/LEMS-2209 ## Test plan: `yarn jest` Storybook - http://localhost:6006/?path=/story/perseuseditor-widgets-interactive-graph--interactive-graph-point image Author: nishasy Reviewers: nishasy, benchristel, Myranae Required Reviewers: Approved By: benchristel Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ gerald, ⏭️ Publish npm snapshot, ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check builds for changes in size (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), ✅ Check builds for changes in size (ubuntu-latest, 20.x), 🚫 Jest Coverage (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1486 --- .changeset/tasty-eggs-drop.md | 6 ++ .../src/__stories__/flags-for-api-options.ts | 1 + .../__tests__/start-coords-settings.test.tsx | 83 +++++++++++++++++++ .../src/components/__tests__/util.test.ts | 70 ++++++++++++++++ .../src/components/start-coords-circle.tsx | 1 - .../src/components/start-coords-point.tsx | 54 ++++++++++++ .../src/components/start-coords-settings.tsx | 10 +++ .../perseus-editor/src/components/util.ts | 41 +++++++++ .../interactive-graph-editor.test.tsx | 60 ++++++++++++++ .../src/widgets/interactive-graph-editor.tsx | 28 ++----- packages/perseus/src/index.ts | 1 + packages/perseus/src/types.ts | 7 +- .../reducer/initialize-graph-state.ts | 2 +- 13 files changed, 338 insertions(+), 26 deletions(-) create mode 100644 .changeset/tasty-eggs-drop.md create mode 100644 packages/perseus-editor/src/components/start-coords-point.tsx diff --git a/.changeset/tasty-eggs-drop.md b/.changeset/tasty-eggs-drop.md new file mode 100644 index 0000000000..396913e65c --- /dev/null +++ b/.changeset/tasty-eggs-drop.md @@ -0,0 +1,6 @@ +--- +"@khanacademy/perseus": minor +"@khanacademy/perseus-editor": minor +--- + +[Hint Mode: Start Coords] Add start coords UI for point graphs 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 5591904391..de0311ec20 100644 --- a/packages/perseus-editor/src/__stories__/flags-for-api-options.ts +++ b/packages/perseus-editor/src/__stories__/flags-for-api-options.ts @@ -20,6 +20,7 @@ export const flags = { // TODO(LEMS-2228): Remove flags once this is fully released "start-coords-ui-phase-1": true, "start-coords-ui-phase-2": true, + "start-coords-ui-point": 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 c0f3cb7cc9..f92a25c273 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 @@ -523,4 +523,87 @@ describe("StartCoordSettings", () => { }, ); }); + + describe("point graph", () => { + test("shows the start coordinates UI: 1 point (default)", () => { + // Arrange + + // Act + render( + {}} + />, + {wrapper: RenderStateRoot}, + ); + + // Assert + expect(screen.getByText("Start coordinates")).toBeInTheDocument(); + expect(screen.getByText("Point 1:")).toBeInTheDocument(); + }); + + test("shows the start coordinates UI: 6 points", () => { + // 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"} + `( + "calls onChange when $coord coord is changed (line $pointIndex)", + async ({pointIndex, coord}) => { + // Arrange + const onChangeMock = jest.fn(); + + // Act + render( + , + ); + + // Assert + const input = screen.getAllByRole("spinbutton", { + name: `${coord}`, + })[pointIndex]; + await userEvent.clear(input); + await userEvent.type(input, "101"); + + const expectedCoords = [ + [-5, 0], + [5, 0], + ]; + 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 bda3983a98..d2a4ee565b 100644 --- a/packages/perseus-editor/src/components/__tests__/util.test.ts +++ b/packages/perseus-editor/src/components/__tests__/util.test.ts @@ -256,6 +256,76 @@ describe("getDefaultGraphStartCoords", () => { expect(defaultCoords).toEqual({center: [0, 0], radius: 2}); }); + + test("should get default start coords for a sinusoid graph", () => { + // Arrange + const graph: PerseusGraphType = {type: "sinusoid"}; + 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([ + [0, 0], + [3, 2], + ]); + }); + + test("should get default start coords for a quadratic graph", () => { + // Arrange + const graph: PerseusGraphType = {type: "quadratic"}; + 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([ + [-5, 5], + [0, -5], + [5, 5], + ]); + }); + + test("should get default start coords for a point graph", () => { + // Arrange + const graph: PerseusGraphType = {type: "point"}; + 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([[0, 0]]); + }); + + test("should get default start coords for a point graph with multiple points", () => { + // Arrange + const graph: PerseusGraphType = {type: "point", numPoints: 2}; + 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([ + [-5, 0], + [5, 0], + ]); + }); }); describe("getSinusoidEquation", () => { diff --git a/packages/perseus-editor/src/components/start-coords-circle.tsx b/packages/perseus-editor/src/components/start-coords-circle.tsx index 9671b4f62d..d494dd06b9 100644 --- a/packages/perseus-editor/src/components/start-coords-circle.tsx +++ b/packages/perseus-editor/src/components/start-coords-circle.tsx @@ -15,7 +15,6 @@ type Props = { center: Coord; radius: number; }; - // center: number; onChange: (startCoords: PerseusGraphType["startCoords"]) => void; }; diff --git a/packages/perseus-editor/src/components/start-coords-point.tsx b/packages/perseus-editor/src/components/start-coords-point.tsx new file mode 100644 index 0000000000..604c5def8e --- /dev/null +++ b/packages/perseus-editor/src/components/start-coords-point.tsx @@ -0,0 +1,54 @@ +import {View} from "@khanacademy/wonder-blocks-core"; +import {Strut} from "@khanacademy/wonder-blocks-layout"; +import {color, spacing} from "@khanacademy/wonder-blocks-tokens"; +import {LabelLarge} from "@khanacademy/wonder-blocks-typography"; +import {StyleSheet} from "aphrodite"; +import * as React from "react"; + +import CoordinatePairInput from "./coordinate-pair-input"; + +import type {Coord, PerseusGraphType} from "@khanacademy/perseus"; + +type Props = { + startCoords: ReadonlyArray; + onChange: (startCoords: PerseusGraphType["startCoords"]) => void; +}; + +const StartCoordsPoint = (props: Props) => { + const {startCoords, onChange} = props; + + return ( + <> + {startCoords.map((coord, index) => { + return ( + + {`Point ${index + 1}:`} + + { + const newStartCoords = [...startCoords]; + newStartCoords[index] = newCoord; + onChange(newStartCoords); + }} + /> + + ); + })} + + ); +}; + +const styles = StyleSheet.create({ + tile: { + backgroundColor: color.fadedBlue8, + marginTop: spacing.xSmall_8, + padding: spacing.small_12, + borderRadius: spacing.xSmall_8, + flexDirection: "row", + alignItems: "center", + }, +}); + +export default StartCoordsPoint; diff --git a/packages/perseus-editor/src/components/start-coords-settings.tsx b/packages/perseus-editor/src/components/start-coords-settings.tsx index a34170fbc6..7031cc4ed8 100644 --- a/packages/perseus-editor/src/components/start-coords-settings.tsx +++ b/packages/perseus-editor/src/components/start-coords-settings.tsx @@ -3,6 +3,7 @@ import { getCircleCoords, getLineCoords, getLinearSystemCoords, + getPointCoords, getQuadraticCoords, getSegmentCoords, getSinusoidCoords, @@ -18,6 +19,7 @@ import Heading from "./heading"; import StartCoordsCircle from "./start-coords-circle"; import StartCoordsLine from "./start-coords-line"; import StartCoordsMultiline from "./start-coords-multiline"; +import StartCoordsPoint from "./start-coords-point"; import StartCoordsQuadratic from "./start-coords-quadratic"; import StartCoordsSinusoid from "./start-coords-sinusoid"; import {getDefaultGraphStartCoords} from "./util"; @@ -89,6 +91,14 @@ const StartCoordsSettingsInner = (props: Props) => { onChange={onChange} /> ); + case "point": + const pointCoords = getPointCoords(props, range, step); + return ( + + ); default: return null; } diff --git a/packages/perseus-editor/src/components/util.ts b/packages/perseus-editor/src/components/util.ts index bfeeb3c239..c58d84fdb1 100644 --- a/packages/perseus-editor/src/components/util.ts +++ b/packages/perseus-editor/src/components/util.ts @@ -3,6 +3,7 @@ import { getCircleCoords, getLineCoords, getLinearSystemCoords, + getPointCoords, getQuadraticCoords, getSegmentCoords, getSinusoidCoords, @@ -194,6 +195,12 @@ export function getDefaultGraphStartCoords( range, step, ); + case "point": + return getPointCoords( + {...graph, startCoords: undefined}, + range, + step, + ); default: return undefined; } @@ -253,3 +260,37 @@ export const getQuadraticEquation = (startCoords: [Coord, Coord, Coord]) => { "y = " + a.toFixed(3) + "x^2 + " + b.toFixed(3) + "x + " + c.toFixed(3) ); }; + +export const shouldShowStartCoordsUI = (flags, graph) => { + // TODO(LEMS-2228): Remove flags once this is fully released + const startCoordsUiPhase1Types = [ + "linear", + "linear-system", + "ray", + "segment", + "circle", + ]; + const startCoordsUiPhase2Types = ["sinusoid", "quadratic"]; + + 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"]; + + if (startCoordsPhase1 && startCoordsUiPhase1Types.includes(graph.type)) { + return true; + } + + if (startCoordsPhase2 && startCoordsUiPhase2Types.includes(graph.type)) { + return true; + } + + if ( + startCoordsPoint && + graph.type === "point" && + graph.numPoints !== "unlimited" + ) { + 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 b90e75d301..7146dc161c 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 @@ -702,6 +702,7 @@ describe("InteractiveGraphEditor", () => { ...flags.mafs, "start-coords-ui-phase-1": shouldRender, "start-coords-ui-phase-2": false, + "start-coords-ui-point": false, }, }, }} @@ -759,6 +760,65 @@ describe("InteractiveGraphEditor", () => { ...flags.mafs, "start-coords-ui-phase-1": false, "start-coords-ui-phase-2": shouldRender, + "start-coords-ui-point": 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"} | ${false} + ${"angle"} | ${false} + ${"point"} | ${true} + `( + "should render for $type graphs if point flag is on: $shouldRender", + async ({type, shouldRender}) => { + // Arrange + + // Act + render( + { graph =
{this.props.valid}
; } - const startCoordsPhase1 = - this.props.apiOptions?.flags?.mafs?.["start-coords-ui-phase-1"]; - const startCoordsPhase2 = - this.props.apiOptions?.flags?.mafs?.["start-coords-ui-phase-2"]; - - const displayStartCoordsUI = - this.props.graph && - ((startCoordsPhase1 && - startCoordsUiPhase1Types.includes(this.props.graph.type)) || - (startCoordsPhase2 && - startCoordsUiPhase2Types.includes(this.props.graph.type))); - return ( @@ -496,7 +475,10 @@ class InteractiveGraphEditor extends React.Component { )} {this.props.graph?.type && // TODO(LEMS-2228): Remove flags once this is fully released - displayStartCoordsUI && ( + shouldShowStartCoordsUI( + this.props.apiOptions.flags, + this.props.graph, + ) && (