[Polygon] Remove duplicate points when determining if a polygon can be closed

.changeset/small-mugs-bow.md

@@ -0,0 +1,6 @@
+---
+"@khanacademy/perseus": patch
+"@khanacademy/perseus-editor": patch
+---
+
+[Polygon] Remove duplicate points when determining if a polygon can be closed

packages/perseus-editor/src/__stories__/interactive-graph-editor.stories.tsx

@@ -22,7 +22,7 @@ import {
     segmentWithStartingCoordsQuestion,
     segmentsWithStartingCoordsQuestion,
     sinusoidWithStartingCoordsQuestion,
-    unlimitedPolygonWithStartingCoordsQuestion,
+    unlimitedPolygonWithCorrectAnswerQuestion,
 } from "../../../perseus/src/widgets/interactive-graphs/interactive-graph.testdata";
 import {registerAllWidgetsAndEditorsForTesting} from "../util/register-all-widgets-and-editors-for-testing";
 
@@ -129,7 +129,7 @@ export const InteractiveGraphPolygon = (): React.ReactElement => {
 export const InteractiveGraphUnlimitedPolygon = (): React.ReactElement => {
     return (
         <EditorPageWithStorybookPreview
-            question={unlimitedPolygonWithStartingCoordsQuestion}
+            question={unlimitedPolygonWithCorrectAnswerQuestion}
         />
     );
 };

packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx

@@ -11,6 +11,7 @@ import {MovablePoint} from "./components/movable-point";
 import {TextLabel} from "./components/text-label";
 import {useDraggable} from "./use-draggable";
 import {pixelsToVectors, useTransformVectorsToPixels} from "./use-transform";
+import {getArrayWithoutDuplicates} from "./utils";
 
 import type {Coord} from "../../../interactive2/types";
 import type {CollinearTuple} from "../../../perseus-types";
@@ -322,9 +323,13 @@ const UnlimitedPolygonGraph = (statefulProps: StatefulProps) => {
                     }}
                     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 && coords.length >= 3) {
+                        // there's enough non-duplicated points to form
+                        // a polygon (3 or more), close the shape before
+                        // setting focus.
+                        if (
+                            i === 0 &&
+                            getArrayWithoutDuplicates(coords).length >= 3
+                        ) {
                             dispatch(actions.polygon.closePolygon());
                         }
                         dispatch(actions.polygon.clickPoint(i));

packages/perseus/src/widgets/interactive-graphs/graphs/utils.test.ts

@@ -1,5 +1,6 @@
-import {getIntersectionOfRayWithBox} from "./utils";
+import {getIntersectionOfRayWithBox, getArrayWithoutDuplicates} from "./utils";
 
+import type {Coord} from "@khanacademy/perseus";
 import type {Interval, vec} from "mafs";
 
 describe("getIntersectionOfRayWithBox", () => {
@@ -139,3 +140,63 @@ describe("getIntersectionOfRayWithBox", () => {
         expect(intersection).toEqual([-1.11, -1.11]);
     });
 });
+
+describe("removeDuplicateCoordsFromArray", () => {
+    test("removes duplicate coordinates", () => {
+        // Arrange
+        const arr: Coord[] = [
+            [0, 0],
+            [0, 0],
+            [1, 1],
+        ];
+
+        // Act
+        const result = getArrayWithoutDuplicates(arr);
+
+        // Assert
+        expect(result).toEqual([
+            [0, 0],
+            [1, 1],
+        ]);
+    });
+
+    test("removes many duplicate coordinates", () => {
+        // Arrange
+        const arr: Coord[] = [
+            [0, 0],
+            [1, 1],
+            [0, 0],
+            [1, 1],
+            [0, 0],
+            [1, 1],
+        ];
+
+        // Act
+        const result = getArrayWithoutDuplicates(arr);
+
+        // Assert
+        expect(result).toEqual([
+            [0, 0],
+            [1, 1],
+        ]);
+    });
+
+    test("does not remove unique coordinates", () => {
+        // Arrange
+        const arr: Coord[] = [
+            [0, 1],
+            [1, 2],
+            [2, 3],
+        ];
+
+        // Act
+        const result = getArrayWithoutDuplicates(arr);
+
+        // Assert
+        expect(result).toEqual([
+            [0, 1],
+            [1, 2],
+            [2, 3],
+        ]);
+    });
+});

packages/perseus/src/widgets/interactive-graphs/graphs/utils.ts

@@ -1,3 +1,4 @@
+import type {Coord} from "@khanacademy/perseus";
 import type {Interval, vec} from "mafs";
 
 /**
@@ -44,3 +45,20 @@ export const getIntersectionOfRayWithBox = (
 function isBetween(x: number, low: number, high: number) {
     return x >= low && x <= high;
 }
+
+export function getArrayWithoutDuplicates(array: Array<Coord>): Array<Coord> {
+    const returnArray: Array<Coord> = [];
+
+    for (const coordPair of array) {
+        if (
+            // Check if the coordPair is not already in the returnArray
+            !returnArray.some(
+                ([x, y]) => x === coordPair[0] && y === coordPair[1],
+            )
+        ) {
+            returnArray.push(coordPair);
+        }
+    }
+
+    return returnArray;
+}

packages/perseus/src/widgets/interactive-graphs/interactive-graph.testdata.ts

@@ -193,7 +193,7 @@ export const polygonWithStartingCoordsQuestion: PerseusRenderer =
         })
         .build();
 
-export const unlimitedPolygonWithStartingCoordsQuestion: PerseusRenderer =
+export const unlimitedPolygonWithCorrectAnswerQuestion: PerseusRenderer =
     interactiveGraphQuestionBuilder()
         .withPolygon("grid", {
             numSides: "unlimited",

packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx

@@ -922,6 +922,107 @@ describe("MafsGraph", () => {
                 {type: REMOVE_POINT, index: 0},
             ]);
         });
+
+        it("polygon - enables the 'Close shape' button when the polygon has 3 or more unique points", () => {
+            // Arrange
+            // Render the question
+            const mockDispatch = jest.fn();
+            const state: InteractiveGraphState = {
+                type: "polygon",
+                numSides: "unlimited",
+                focusedPointIndex: null,
+                hasBeenInteractedWith: true,
+                showRemovePointButton: false,
+                interactionMode: "mouse",
+                showKeyboardInteractionInvitation: false,
+                showAngles: false,
+                showSides: false,
+                range: [
+                    [-10, 10],
+                    [-10, 10],
+                ],
+                snapStep: [2, 2],
+                snapTo: "grid",
+                coords: [
+                    [4, 5],
+                    [5, 6],
+                    [6, 7],
+                ],
+                closedPolygon: false,
+            };
+
+            const baseMafsGraphProps: MafsGraphProps = {
+                ...getBaseMafsGraphPropsForTests(),
+                markings: "none",
+            };
+
+            render(
+                <MafsGraph
+                    {...baseMafsGraphProps}
+                    state={state}
+                    dispatch={mockDispatch}
+                />,
+            );
+
+            // Assert
+            // Find the button
+            const closeShapeButton = screen.getByRole("button", {
+                name: "Close shape",
+            });
+            // Make sure the button is enabled
+            expect(closeShapeButton).toHaveAttribute("aria-disabled", "false");
+        });
+
+        it("polygon - disables the 'Close shape' button when the polygon has fewer than 3 unique points", () => {
+            // Arrange
+            // Render the question
+            const mockDispatch = jest.fn();
+            const state: InteractiveGraphState = {
+                type: "polygon",
+                numSides: "unlimited",
+                focusedPointIndex: null,
+                hasBeenInteractedWith: true,
+                showRemovePointButton: false,
+                interactionMode: "mouse",
+                showKeyboardInteractionInvitation: false,
+                showAngles: false,
+                showSides: false,
+                range: [
+                    [-10, 10],
+                    [-10, 10],
+                ],
+                snapStep: [2, 2],
+                snapTo: "grid",
+                coords: [
+                    [4, 5],
+                    [5, 6],
+                    // not unique
+                    [5, 6],
+                ],
+                closedPolygon: false,
+            };
+
+            const baseMafsGraphProps: MafsGraphProps = {
+                ...getBaseMafsGraphPropsForTests(),
+                markings: "none",
+            };
+
+            render(
+                <MafsGraph
+                    {...baseMafsGraphProps}
+                    state={state}
+                    dispatch={mockDispatch}
+                />,
+            );
+
+            // Assert
+            // Find the button
+            const closeShapeButton = screen.getByRole("button", {
+                name: "Close shape",
+            });
+            // Make sure the button is disabled
+            expect(closeShapeButton).toHaveAttribute("aria-disabled", "true");
+        });
     });
 });
 

packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx

@@ -37,6 +37,7 @@ import {renderQuadraticGraph} from "./graphs/quadratic";
 import {renderRayGraph} from "./graphs/ray";
 import {renderSegmentGraph} from "./graphs/segment";
 import {renderSinusoidGraph} from "./graphs/sinusoid";
+import {getArrayWithoutDuplicates} from "./graphs/utils";
 import {MIN, X, Y} from "./math";
 import {Protractor} from "./protractor";
 import {actions} from "./reducer/interactive-graph-action";
@@ -422,7 +423,7 @@ const renderPolygonGraphControls = (props: {
 
     // We want to disable the closePolygon button when
     // there are not enough coords to make a polygon
-    const disableCloseButton = coords.length < 3;
+    const disableCloseButton = getArrayWithoutDuplicates(coords).length < 3;
 
     // If polygon is closed, show open button.
     // If polygon is open, show close button.

packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-reducer.test.ts

@@ -1548,7 +1548,7 @@ describe("doClosePolygon", () => {
         expect(updated.closedPolygon).toBeTruthy();
     });
 
-    it("does not change `closedPolygon` property when it's already false", () => {
+    it("does not change `closedPolygon` property when it's already true", () => {
         const state: InteractiveGraphState = {
             ...baseUnlimitedPolygonGraphState,
             closedPolygon: true,
@@ -1562,6 +1562,32 @@ describe("doClosePolygon", () => {
         invariant(updated.type === "polygon");
         expect(updated.closedPolygon).toBeTruthy();
     });
+
+    it("removes duplicated points from the new state when closed", () => {
+        const state: InteractiveGraphState = {
+            ...baseUnlimitedPolygonGraphState,
+            coords: [
+                [0, 0],
+                [0, 1],
+                [1, 1],
+                [0, 0], // last point same as first point
+            ],
+            closedPolygon: false,
+        };
+
+        const updated = interactiveGraphReducer(
+            state,
+            actions.polygon.closePolygon(),
+        );
+
+        invariant(updated.type === "polygon");
+        expect(updated.closedPolygon).toBeTruthy();
+        expect(updated.coords).toEqual([
+            [0, 0],
+            [0, 1],
+            [1, 1],
+        ]);
+    });
 });
 
 describe("doOpenPolygon", () => {

packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-reducer.ts

@@ -15,6 +15,7 @@ import {
     vector,
 } from "../../../util/geometry";
 import {getQuadraticCoefficients} from "../graphs/quadratic";
+import {getArrayWithoutDuplicates} from "../graphs/utils";
 import {
     clamp,
     clampToBox,
@@ -200,8 +201,15 @@ function doClickPoint(
 
 function doClosePolygon(state: InteractiveGraphState): InteractiveGraphState {
     if (isUnlimitedGraphState(state) && state.type === "polygon") {
+        // We want to remove any duplicate points when closing the polygon to
+        // (1) prevent the polygon from sides with length zero, and
+        // (2) make sure the question is can be marked correct if the polygon
+        //     LOOKS correct, even if two of the points are at the same coords.
+        const noDupedPoints = getArrayWithoutDuplicates(state.coords);
+
         return {
             ...state,
+            coords: noDupedPoints,
             closedPolygon: true,
         };
     }