[Polygon] Remove duplicate points when determining if a polygon can b…

…e closed (#1978)

## Summary:
Right now, if a user were trying to close the polygon by clicking the last point, missed, created a new point instead as a result, and then tried to drag the new point to the first point to close out the polygon, the question would be marked wrong even if the polygon looks right.

Relatedly, if a user were to drag a middle point over another point in the polygon, even if the polygon looks right, it would be marked wrong.

To fix both of these issues, I'm making it so
- the close button is only enabled if there are three or more _unique_ points in the polygon
- closing the polygon removes any duplicate points from the coords array. This allows the dragging to work more intuitively and the question to be graded fairly (based on how the polygon looks regardless of duplicate points)

Issue: https://khanacademy.atlassian.net/browse/LEMS-2711
(and also https://khanacademy.atlassian.net/browse/LEMS-2722)

## Test plan:

`yarn jest packages/perseus/src/widgets/interactive-graphs/graphs/utils.test.ts`
`yarn jest packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx`
`yarn jest packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-reducer.test.ts`

Storybook
- Go to http://localhost:6006/?path=/story/perseus-widgets-interactive-graph--unlimited-polygon-with-mafs
- Put down four points
- Click to create a fifth point
- drag the fifth point over to the first one
- close the shape with the button
- re-open the shape
- confirm that the previous fifth point is now gone
- move a middle point over another middle point
- close and reopen the shape
- confirm that the duplicate point is gone
- get to a state of three points
- move one point over another so it looks like there are two points
- confirm that the close button is disabled

### Before:

https://github.com/user-attachments/assets/2e26ca48-5925-4738-96a2-91adf71b2826

### After:

https://github.com/user-attachments/assets/66dc76f1-4207-47e1-9ab8-2ef5a8f781a6

Author: nishasy

Reviewers: catandthemachines, nishasy, anakaren-rojas

Required Reviewers:

Approved By: catandthemachines

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x)

Pull Request URL: #1978

.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,
         };
     }