[Interactive Graph] Open and Closing logic for unlimited polygon. (#1852

) ## Summary: Adding opening and closing behavior to unlimited polygon including: - New buttons for graph controls. - Reducing redundancy between polygon and unlimited polygon behavior. - And more robust testing for unlimited graph controls. Issue: LEMS-2570 ## Test plan: - Run `yarn dev` and enable `Unlimited Polygon` option in the Mafs dropdown. Or http://localhost:6006/?path=/story/perseus-widgets-interactive-graph--unlimited-polygon-with-mafs. - Go the the exercise and build polygons with mouse or keyboard. - Use the `Close polygon` button the close the polygon. - The expectation is now the polygon should look and behave like other Mafs polygon graphs. - Use the `Open polygon` button again to add or remove points. Author: catandthemachines Reviewers: catandthemachines, benchristel, nishasy, anakaren-rojas Required Reviewers: Approved By: benchristel Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (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), ✅ Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: #1852
Khan · Nov 20, 2024 · 4b8836b · 4b8836b
1 parent 4e00c12
commit 4b8836b
Show file tree

Hide file tree

Showing 13 changed files with 716 additions and 242 deletions.
diff --git a/.changeset/twenty-baboons-shave.md b/.changeset/twenty-baboons-shave.md
@@ -0,0 +1,5 @@
+---
+"@khanacademy/perseus": patch
+---
+
+Adding open and closing behavior to unlimited polygon graph type.
diff --git a/packages/perseus/src/strings.ts b/packages/perseus/src/strings.ts
@@ -133,6 +133,8 @@ export type PerseusStrings = {
     addPoint: string;
     removePoint: string;
     graphKeyboardPrompt: string;
+    closePolygon: string;
+    openPolygon: string;
     srPointAtCoordinates: ({
         num,
         x,
@@ -319,6 +321,8 @@ export const strings: {
     addPoint: "Add Point",
     removePoint: "Remove Point",
     graphKeyboardPrompt: "Press Shift + Enter to interact with the graph",
+    closePolygon: "Close shape",
+    openPolygon: "Re-open shape",
     srPointAtCoordinates: {
         context: "Screenreader-accessible description of a point on a graph",
         message: "Point %(num)s at %(x)s comma %(y)s",
@@ -484,6 +488,8 @@ export const mockStrings: PerseusStrings = {
     addPoint: "Add Point",
     removePoint: "Remove Point",
     graphKeyboardPrompt: "Press Shift + Enter to interact with the graph",
+    closePolygon: "Close shape",
+    openPolygon: "Re-open shape",
     srPointAtCoordinates: ({num, x, y}) => `Point ${num} at ${x} comma ${y}`,
     srInteractiveElements: ({elements}) => `Interactive elements: ${elements}`,
     srNoInteractiveElements: "No interactive elements",

diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx
@@ -1,4 +1,4 @@
-import {Polygon, vec} from "mafs";
+import {Polygon, Polyline, vec} from "mafs";
 import * as React from "react";
 
 import {snap} from "../math";
@@ -170,32 +170,14 @@ const LimitedPolygonGraph = (props: Props) => {
     );
 };
 
-// TODO(catjohnson): reduce redundancy between LimitedPolygonGraph and UnlimitedPolygonGraph
-// both components are vary similar, however more implementation is needed to be added before
-// it is clear what can and can't be shared between components.
 const UnlimitedPolygonGraph = (props: Props) => {
-    const [hovered, setHovered] = React.useState(false);
-    // This is more so required for the re-rendering that occurs when state
-    // updates; specifically with regard to line weighting and polygon focus.
-    const [focusVisible, setFocusVisible] = React.useState(false);
-
     const {dispatch} = props;
-    const {
-        coords,
-        showAngles,
-        showSides,
-        range,
-        snapStep,
-        snapTo = "grid",
-    } = props.graphState;
+    const {coords, closedPolygon} = props.graphState;
 
     const graphConfig = useGraphConfig();
 
-    // TODO(catjohnson): Explore abstracting this code as it is similar to point.tsx
-    // and hopefully we can cut down ont the unlimited graph redundancy.
     const {
         range: [x, y],
-        disableKeyboardInteraction,
         graphDimensionsInPixels,
     } = graphConfig;
 
@@ -208,161 +190,85 @@ const UnlimitedPolygonGraph = (props: Props) => {
     // TODO(benchristel): can the default set of points be removed here? I don't
     // think coords can be null.
     const points = coords ?? [[0, 0]];
-    const polygonRef = React.useRef<SVGPolygonElement>(null);
-    const dragReferencePoint = points[0];
-    const constrain = ["angles", "sides"].includes(snapTo)
-        ? (p) => p
-        : (p) => snap(snapStep, p);
-    const {dragging} = useDraggable({
-        gestureTarget: polygonRef,
-        point: dragReferencePoint,
-        onMove: (newPoint) => {
-            const delta = vec.sub(newPoint, dragReferencePoint);
-            dispatch(actions.polygon.moveAll(delta));
-        },
-        constrainKeyboardMovement: constrain,
-    });
 
-    const lines = getLines(points);
     React.useEffect(() => {
         const focusedIndex = props.graphState.focusedPointIndex;
         if (focusedIndex != null) {
             pointRef.current[focusedIndex]?.focus();
         }
     }, [props.graphState.focusedPointIndex, pointRef]);
 
-    return (
-        <>
-            {/* This rect is here to grab clicks so that new points can be added */}
-            {/* It's important because it stops mouse events from propogating
+    if (closedPolygon) {
+        const closedPolygonProps = {...props, numSides: coords.length};
+        return <LimitedPolygonGraph {...closedPolygonProps} />;
+    } else {
+        return (
+            <>
+                {/* This rect is here to grab clicks so that new points can be added */}
+                {/* It's important because it stops mouse events from propogating
                 when dragging a points around */}
-            <rect
-                style={{
-                    fill: "rgba(0,0,0,0)",
-                    cursor: "crosshair",
-                }}
-                width={widthPx}
-                height={heightPx}
-                x={left}
-                y={top}
-                onClick={(event) => {
-                    const elementRect =
-                        event.currentTarget.getBoundingClientRect();
+                <rect
+                    style={{
+                        fill: "rgba(0,0,0,0)",
+                        cursor: "crosshair",
+                    }}
+                    width={widthPx}
+                    height={heightPx}
+                    x={left}
+                    y={top}
+                    onClick={(event) => {
+                        const elementRect =
+                            event.currentTarget.getBoundingClientRect();
 
-                    const x = event.clientX - elementRect.x;
-                    const y = event.clientY - elementRect.y;
+                        const x = event.clientX - elementRect.x;
+                        const y = event.clientY - elementRect.y;
 
-                    const graphCoordinates = pixelsToVectors(
-                        [[x, y]],
-                        graphConfig,
-                    );
-                    dispatch(actions.polygon.addPoint(graphCoordinates[0]));
-                }}
-            />
-            {/**
-             * TODO(catjohnson): Will need to conditionally render then once a full polygon is created
-             * And handle when someone wants to remove the polygon connection.
-             */}
-            <Polygon
-                points={[...points]}
-                color="var(--movable-line-stroke-color)"
-                svgPolygonProps={{
-                    strokeWidth: focusVisible
-                        ? "var(--movable-line-stroke-weight-active)"
-                        : "var(--movable-line-stroke-weight)",
-                    style: {fill: "transparent"},
-                }}
-            />
-            {props.graphState.coords.map((point, i) => {
-                const pt1 = points.at(i - 1);
-                const pt2 = points[(i + 1) % points.length];
-                if (!pt1 || !pt2) {
-                    return null;
-                }
-                return (
-                    <PolygonAngle
-                        key={"angle-" + i}
-                        centerPoint={point}
-                        endPoints={[pt1, pt2]}
-                        range={range}
-                        polygonLines={lines}
-                        showAngles={!!showAngles}
-                        snapTo={snapTo}
-                    />
-                );
-            })}
-            {showSides &&
-                lines.map(([start, end], i) => {
-                    const [x, y] = vec.midpoint(start, end);
-                    const length = parseFloat(
-                        vec
-                            .dist(start, end)
-                            .toFixed(snapTo === "sides" ? 0 : 1),
-                    );
-                    return (
-                        <TextLabel key={"side-" + i} x={x} y={y}>
-                            {!Number.isInteger(length) && "≈ "}
-                            {length}
-                        </TextLabel>
-                    );
-                })}
-            {/**
-             * This transparent svg creates a nice big click/touch target,
-             * since the polygon itself can be made smaller than the spec.
-             */}
-            {/**
-             * Will likely want to conditionally render then once a full polygon is created
-             * And handle when someone wants to remove the polygon connection?
-             */}
-            <Polygon
-                points={[...points]}
-                color="transparent"
-                svgPolygonProps={{
-                    ref: polygonRef,
-                    tabIndex: disableKeyboardInteraction ? -1 : 0,
-                    strokeWidth: TARGET_SIZE,
-                    style: {
-                        cursor: dragging ? "grabbing" : "grab",
-                        fill: hovered ? "var(--mafs-blue)" : "transparent",
-                    },
-                    onMouseEnter: () => setHovered(true),
-                    onMouseLeave: () => setHovered(false),
-                    // Required to remove line weighting when user clicks away
-                    // from the focused polygon
-                    onKeyDownCapture: () => {
-                        setFocusVisible(hasFocusVisible(polygonRef.current));
-                    },
-                    // Required for lines to darken on focus
-                    onFocus: () =>
-                        setFocusVisible(hasFocusVisible(polygonRef.current)),
-                    // Required for line weighting to update on blur. Without this,
-                    // the user has to hover over the shape for it to update
-                    onBlur: () =>
-                        setFocusVisible(hasFocusVisible(polygonRef.current)),
-                    className: "movable-polygon",
-                }}
-            />
-            {props.graphState.coords.map((point, i) => (
-                <MovablePoint
-                    key={i}
-                    point={point}
-                    sequenceNumber={i + 1}
-                    onMove={(destination) =>
-                        dispatch(actions.pointGraph.movePoint(i, destination))
-                    }
-                    ref={(ref) => {
-                        pointRef.current[i] = ref;
-                    }}
-                    onFocus={() => {
-                        dispatch(actions.pointGraph.focusPoint(i));
+                        const graphCoordinates = pixelsToVectors(
+                            [[x, y]],
+                            graphConfig,
+                        );
+                        dispatch(actions.polygon.addPoint(graphCoordinates[0]));
                     }}
-                    onClick={() => {
-                        dispatch(actions.pointGraph.clickPoint(i));
+                />
+                <Polyline
+                    points={[...points]}
+                    color="var(--movable-line-stroke-color)"
+                    svgPolylineProps={{
+                        strokeWidth: "var(--movable-line-stroke-weight)",
+                        style: {fill: "transparent"},
                     }}
                 />
-            ))}
-        </>
-    );
+                {props.graphState.coords.map((point, i) => (
+                    <MovablePoint
+                        key={i}
+                        point={point}
+                        sequenceNumber={i + 1}
+                        onMove={(destination) =>
+                            dispatch(actions.polygon.movePoint(i, destination))
+                        }
+                        ref={(ref) => {
+                            pointRef.current[i] = ref;
+                        }}
+                        onFocus={() => {
+                            dispatch(actions.polygon.focusPoint(i));
+                        }}
+                        onClick={() => {
+                            // If the point being clicked is the first point and
+                            // there's enough points to form a polygon (3 or more)
+                            // Close the shape before setting focus.
+                            if (
+                                i === 0 &&
+                                props.graphState.coords.length >= 3
+                            ) {
+                                dispatch(actions.polygon.closePolygon());
+                            }
+                            dispatch(actions.polygon.clickPoint(i));
+                        }}
+                    />
+                ))}
+            </>
+        );
+    }
 };
 
 function getLines(points: readonly vec.Vector2[]): CollinearTuple[] {

diff --git a/packages/perseus/src/widgets/interactive-graphs/interactive-graph.stories.tsx b/packages/perseus/src/widgets/interactive-graphs/interactive-graph.stories.tsx
@@ -24,6 +24,7 @@ import {
     staticGraphQuestion,
     staticGraphQuestionWithAnotherWidget,
     segmentWithLockedLabels,
+    unlimitedPolygonQuestion,
 } from "./interactive-graph.testdata";
 
 import type {APIOptions} from "../../types";
@@ -37,6 +38,7 @@ const enableMafs: APIOptions = {
         mafs: {
             segment: true,
             polygon: true,
+            "unlimited-polygon": true,
             angle: true,
             "interactive-graph-locked-features-labels": true,
         },
@@ -80,6 +82,15 @@ export const PolygonWithMafs = (args: StoryArgs): React.ReactElement => (
     />
 );
 
+export const UnlimitedPolygonWithMafs = (
+    args: StoryArgs,
+): React.ReactElement => (
+    <RendererWithDebugUI
+        apiOptions={{...enableMafs}}
+        question={unlimitedPolygonQuestion}
+    />
+);
+
 export const PolygonWithMafsReadOnly = (
     args: StoryArgs,
 ): React.ReactElement => (

diff --git a/packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts b/packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts
@@ -162,6 +162,25 @@ export const polygonQuestion: PerseusRenderer =
         })
         .build();
 
+export const unlimitedPolygonQuestion: PerseusRenderer =
+    interactiveGraphQuestionBuilder()
+        .withContent(
+            "**Sides shown** Drag the vertices of the triangle below to draw a right triangle with side lengths $3$, $4$, and $5$. \n[[\u2603 interactive-graph 1]] \n",
+        )
+        .withGridStep(1, 1)
+        .withSnapStep(0.25, 0.25)
+        .withTickStep(1, 1)
+        .withXRange(-10, 10)
+        .withYRange(-10, 10)
+        .withPolygon("grid", {
+            match: "congruent",
+            numSides: "unlimited",
+            showSides: true,
+            showAngles: true,
+            coords: [],
+        })
+        .build();
+
 export const polygonWithStartingCoordsQuestion: PerseusRenderer =
     interactiveGraphQuestionBuilder()
         .withPolygon("grid", {
@@ -284,6 +303,8 @@ export const polygonWithUnlimitedSidesQuestion: PerseusRenderer =
             "**Example of unlimited polygon sides** \n[[\u2603 interactive-graph 1]] \n",
         )
         .withPolygon("grid", {
+            showAngles: true,
+            showSides: true,
             numSides: "unlimited",
             coords: [
                 [0, 0],

diff --git a/packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx b/packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx
@@ -436,6 +436,7 @@ describe("MafsGraph", () => {
             showRemovePointButton: false,
             interactionMode: "mouse",
             showKeyboardInteractionInvitation: false,
+            closedPolygon: false,
             coords: [
                 [-1, 1],
                 [0, 0],
@@ -838,6 +839,7 @@ describe("MafsGraph", () => {
                 snapStep: [2, 2],
                 snapTo: "grid",
                 coords: [[4, 5]],
+                closedPolygon: false,
             };
 
             const baseMafsGraphProps: MafsGraphProps = {
@@ -921,6 +923,7 @@ describe("MafsGraph", () => {
                 snapStep: [2, 2],
                 snapTo: "grid",
                 coords: [[9, 9]],
+                closedPolygon: false,
             };
 
             const baseMafsGraphProps: MafsGraphProps = {