diff --git a/.changeset/twenty-baboons-shave.md b/.changeset/twenty-baboons-shave.md new file mode 100644 index 0000000000..8cd20506e8 --- /dev/null +++ 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 index 41c8d30a6f..7d94512110 100644 --- 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 index 6347651ff1..ba7635dbfc 100644 --- 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,22 +190,7 @@ 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(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) { @@ -231,138 +198,77 @@ const UnlimitedPolygonGraph = (props: Props) => { } }, [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 ; + } 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 */} - { - const elementRect = - event.currentTarget.getBoundingClientRect(); + { + 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. - */} - - {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 ( - - ); - })} - {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 ( - - {!Number.isInteger(length) && "≈ "} - {length} - - ); - })} - {/** - * 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? - */} - 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) => ( - - 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)); + /> + - ))} - - ); + {props.graphState.coords.map((point, i) => ( + + 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 index 34fef89c92..3093830a83 100644 --- 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 => ( + +); + 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 index 07596372e7..a2f5e97e07 100644 --- 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 index 5b598ae250..7a5d5a970d 100644 --- 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 = { diff --git a/packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx b/packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx index 44c423b380..fd1bebbcc8 100644 --- a/packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx @@ -396,73 +396,120 @@ const renderPointGraphControls = (props: { ); }; -/** - * TODO(catjohnson): The polygon controls will be vastly different from the point controls. - * Will be keeping this function as is for the time being, even if it's a copy of the point graph - * controls. - */ const renderPolygonGraphControls = (props: { state: PolygonGraphState; dispatch: (action: InteractiveGraphAction) => unknown; width: number; perseusStrings: PerseusStrings; }) => { - const {interactionMode, showRemovePointButton, focusedPointIndex} = - props.state; + const { + interactionMode, + showRemovePointButton, + focusedPointIndex, + closedPolygon, + coords, + } = props.state; const {perseusStrings} = props; const shouldShowRemoveButton = showRemovePointButton && focusedPointIndex !== null; - return ( - { + props.dispatch(actions.polygon.openPolygon()); }} > - {interactionMode === "keyboard" && ( - - )} - {interactionMode === "mouse" && ( - - )} - + {perseusStrings.openPolygon} + + ) : ( + + ); + + return ( + <> + + {/** + * Only show this in keyboard mode. + */} + {interactionMode === "keyboard" && ( + + )} + {/* + Make sure remove button is always present, just disabled/enabled depending + on when a point is selected or if the polygon is closed. + */} + {interactionMode === "mouse" && ( + + )} + {polygonButton} + + ); }; @@ -531,7 +578,7 @@ function handleKeyboardEvent( dispatch: (action: InteractiveGraphAction) => unknown, ) { if (isUnlimitedGraphState(state)) { - if (event.key === "Backspace") { + if (event.key === "Backspace" || event.key === "Delete") { // TODO(benchristel): Checking classList here is a hack to prevent // points from being deleted if the user presses the backspace key // while the whole graph is focused. Instead of doing this, we @@ -542,7 +589,13 @@ function handleKeyboardEvent( "movable-point__focusable-handle", ) ) { - dispatch(actions.global.deleteIntent()); + // Only allow delete if type is point or a polygon that is open. + if ( + state.type === "point" || + (state.type === "polygon" && !state.closedPolygon) + ) { + dispatch(actions.global.deleteIntent()); + } } // After removing a point blur diff --git a/packages/perseus/src/widgets/interactive-graphs/mafs-state-to-interactive-graph.test.ts b/packages/perseus/src/widgets/interactive-graphs/mafs-state-to-interactive-graph.test.ts index 59e0dc1f5e..26bdaec575 100644 --- a/packages/perseus/src/widgets/interactive-graphs/mafs-state-to-interactive-graph.test.ts +++ b/packages/perseus/src/widgets/interactive-graphs/mafs-state-to-interactive-graph.test.ts @@ -271,6 +271,7 @@ describe("mafsStateToInteractiveGraph", () => { [3, 4], [5, 6], ], + closedPolygon: false, }; const result: PerseusGraphType = mafsStateToInteractiveGraph( 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 8ae0c0dc5a..a20ef4bef8 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 @@ -75,6 +75,7 @@ export function initializeGraphState( showRemovePointButton: false, interactionMode: "mouse", showKeyboardInteractionInvitation: false, + closedPolygon: false, }; case "point": return { diff --git a/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-action.ts b/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-action.ts index c94a10f686..a4f1908563 100644 --- a/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-action.ts +++ b/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-action.ts @@ -18,6 +18,8 @@ export type InteractiveGraphAction = | BlurPoint | DeleteIntent | ClickPoint + | ClosePolygon + | OpenPolygon | ChangeInteractionMode | ChangeKeyboardInvitationVisibility; @@ -59,6 +61,8 @@ export const actions = { focusPoint, blurPoint, clickPoint, + closePolygon, + openPolygon, }, quadratic: { movePoint, @@ -189,6 +193,26 @@ function changeKeyboardInvitationVisibility( }; } +export const CLOSE_POLYGON = "close-polygon"; +export interface ClosePolygon { + type: typeof CLOSE_POLYGON; +} +function closePolygon(): ClosePolygon { + return { + type: CLOSE_POLYGON, + }; +} + +export const OPEN_POLYGON = "open-polygon"; +export interface OpenPolygon { + type: typeof OPEN_POLYGON; +} +function openPolygon(): OpenPolygon { + return { + type: OPEN_POLYGON, + }; +} + export const MOVE_ALL = "move-all"; export interface MoveAll { type: typeof MOVE_ALL; diff --git a/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-reducer.test.ts b/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-reducer.test.ts index c674602652..41667dacb3 100644 --- a/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-reducer.test.ts +++ b/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-reducer.test.ts @@ -131,6 +131,7 @@ const basePolygonGraphState: InteractiveGraphState = { [0, 1], [1, 0], ], + closedPolygon: false, }; const baseUnlimitedPolygonGraphState: PolygonGraphState = { @@ -150,6 +151,7 @@ const baseUnlimitedPolygonGraphState: PolygonGraphState = { snapStep: [1, 1], coords: [], numSides: "unlimited", + closedPolygon: false, }; describe("movePointInFigure", () => { @@ -1150,32 +1152,452 @@ describe("doDeleteIntent", () => { }); }); -xdescribe("doChangeInteractionMode", () => { - // TODO(catjohnson): Add tests for doChangeInteractionMode function. +describe("doChangeInteractionMode", () => { + it("does nothing when type is not an unlimited graph", () => { + const state: InteractiveGraphState = baseCircleGraphState; + + const updated = interactiveGraphReducer( + state, + actions.global.changeInteractionMode("keyboard"), + ); + + expect(updated).toMatchObject(state); + }); + + it("updates interactionMode to `keyboard` and all other required state values", () => { + const state: InteractiveGraphState = baseUnlimitedPointGraphState; + + const updated = interactiveGraphReducer( + state, + actions.global.changeInteractionMode("keyboard"), + ); + + invariant(updated.type === "point"); + expect(updated.interactionMode).toBe("keyboard"); + expect(updated.showKeyboardInteractionInvitation).toBeFalsy(); + }); + + it("updates interactionMode to `mouse` and all other required state values", () => { + const state: PointGraphState = { + ...baseUnlimitedPointGraphState, + interactionMode: "keyboard", + }; + + const updated = interactiveGraphReducer( + state, + actions.global.changeInteractionMode("mouse"), + ); + + invariant(updated.type === "point"); + expect(updated.interactionMode).toBe("mouse"); + expect(updated.showKeyboardInteractionInvitation).toBe( + state.showKeyboardInteractionInvitation, + ); + }); +}); + +describe("doChangeKeyboardInvitationVisibility", () => { + it("does nothing when type is not an unlimited graph", () => { + const state: InteractiveGraphState = baseCircleGraphState; + + const updated = interactiveGraphReducer( + state, + actions.global.changeKeyboardInvitationVisibility(true), + ); + + expect(updated).toMatchObject(state); + }); + + it("updates keyboard invitation visibility to `true`", () => { + const state: InteractiveGraphState = baseUnlimitedPointGraphState; + + const updated = interactiveGraphReducer( + state, + actions.global.changeKeyboardInvitationVisibility(true), + ); + + invariant(updated.type === "point"); + expect(updated.showKeyboardInteractionInvitation).toBeTruthy(); + expect(updated.hasBeenInteractedWith).toBeTruthy(); + }); + + it("updates keyboard invitation visibility to `false`", () => { + const state: InteractiveGraphState = baseUnlimitedPointGraphState; + + const updated = interactiveGraphReducer( + state, + actions.global.changeKeyboardInvitationVisibility(false), + ); + + invariant(updated.type === "point"); + expect(updated.showKeyboardInteractionInvitation).toBeFalsy(); + expect(updated.hasBeenInteractedWith).toBeTruthy(); + }); + + it("hasBeenInteractedWith property is always set to 'true'", () => { + const state: InteractiveGraphState = baseUnlimitedPointGraphState; + + const updated1 = interactiveGraphReducer( + state, + actions.global.changeKeyboardInvitationVisibility(false), + ); + + invariant(updated1.type === "point"); + expect(updated1.hasBeenInteractedWith).toBeTruthy(); + + const updated2 = interactiveGraphReducer( + state, + actions.global.changeKeyboardInvitationVisibility(true), + ); + + invariant(updated2.type === "point"); + expect(updated2.hasBeenInteractedWith).toBeTruthy(); + }); +}); + +describe("doClickPoint", () => { + it("does nothing when type is not an unlimited graph", () => { + const state: InteractiveGraphState = baseCircleGraphState; + + const updated = interactiveGraphReducer( + state, + actions.pointGraph.clickPoint(0), + ); + + expect(updated).toMatchObject(state); + }); + + it("updates focusedPointIndex with new index value", () => { + const state: InteractiveGraphState = baseUnlimitedPointGraphState; + + const updated = interactiveGraphReducer( + state, + actions.pointGraph.clickPoint(0), + ); + + invariant(updated.type === "point"); + expect(updated.focusedPointIndex).toBe(0); + expect(updated.showRemovePointButton).toBeTruthy(); + }); + + it("showRemovePointButton property is always set to 'true'", () => { + const state: InteractiveGraphState = baseUnlimitedPointGraphState; + + const updated = interactiveGraphReducer( + state, + actions.pointGraph.clickPoint(10), + ); + + invariant(updated.type === "point"); + expect(updated.showRemovePointButton).toBeTruthy(); + }); }); -xdescribe("doChangeKeyboardInvitationVisibility", () => { - // TODO(catjohnson): Add tests for doChangeKeyboardInvitationVisibility function. +describe("doBlurPoint", () => { + it("does nothing when type is not an unlimited graph", () => { + const state: InteractiveGraphState = baseCircleGraphState; + + const updated = interactiveGraphReducer( + state, + actions.pointGraph.blurPoint(), + ); + + expect(updated).toMatchObject(state); + }); + + it("showRemovePointButton property is always set to 'false'", () => { + const state: InteractiveGraphState = { + ...baseUnlimitedPointGraphState, + focusedPointIndex: 0, + interactionMode: "keyboard", + showRemovePointButton: true, + }; + + const updated = interactiveGraphReducer( + state, + actions.pointGraph.blurPoint(), + ); + + invariant(updated.type === "point"); + expect(updated.focusedPointIndex).toBe(state.focusedPointIndex); + expect(updated.showRemovePointButton).toBeFalsy(); + }); + + it("if interactionMode is `mouse`, updates focusedPointIndex to null", () => { + const state: InteractiveGraphState = { + ...baseUnlimitedPointGraphState, + interactionMode: "mouse", + }; + const updated = interactiveGraphReducer( + state, + actions.pointGraph.blurPoint(), + ); + + invariant(updated.type === "point"); + expect(updated.focusedPointIndex).toBe(null); + expect(updated.showRemovePointButton).toBeFalsy(); + }); }); -xdescribe("doClickPoint", () => { - // TODO(catjohnson): Add tests for doClickPoint function. +describe("doFocusPoint", () => { + it("does nothing when type is not an unlimited graph", () => { + const state: InteractiveGraphState = baseCircleGraphState; + + const updated = interactiveGraphReducer( + state, + actions.pointGraph.focusPoint(0), + ); + + expect(updated).toMatchObject(state); + }); + + it("updates focusedPointIndex with new index value", () => { + const state: InteractiveGraphState = baseUnlimitedPointGraphState; + + const updated = interactiveGraphReducer( + state, + actions.pointGraph.focusPoint(0), + ); + + invariant(updated.type === "point"); + expect(updated.focusedPointIndex).toBe(0); + expect(updated.showRemovePointButton).toBe(state.showRemovePointButton); + }); + + it("showRemovePointButton property is not changed", () => { + const state: InteractiveGraphState = baseUnlimitedPointGraphState; + + const updated = interactiveGraphReducer( + state, + actions.pointGraph.focusPoint(10), + ); + + invariant(updated.type === "point"); + expect(updated.showRemovePointButton).toBe(state.showRemovePointButton); + }); }); -xdescribe("doBlurPoint", () => { - // TODO(catjohnson): Add tests for doBlurPoint function. +describe("doAddPoint", () => { + it("does nothing when type is not an unlimited graph", () => { + const state: InteractiveGraphState = baseCircleGraphState; + + const updated = interactiveGraphReducer( + state, + actions.pointGraph.addPoint([0, 0]), + ); + + expect(updated).toMatchObject(state); + }); + + it("adds a new point at a specified graph location", () => { + const state: InteractiveGraphState = { + ...baseUnlimitedPointGraphState, + coords: [[2, -2]], + }; + + const updated = interactiveGraphReducer( + state, + actions.pointGraph.addPoint([0, 0]), + ); + + invariant(updated.type === "point"); + expect(updated.coords).toMatchObject([ + [2, -2], + [0, 0], + ]); + expect(updated.hasBeenInteractedWith).toBeTruthy(); + expect(updated.showRemovePointButton).toBeFalsy(); + expect(updated.focusedPointIndex).toBe(1); + }); + + it("does not adds a new point if there is already a point at that location", () => { + const state: InteractiveGraphState = { + ...baseUnlimitedPointGraphState, + coords: [[0, 0]], + }; + + const updated = interactiveGraphReducer( + state, + actions.pointGraph.addPoint([0, 0]), + ); + + expect(updated).toMatchObject(state); + }); }); -xdescribe("doFocusPoint", () => { - // TODO(catjohnson): Add tests for doFocusPoint function. +describe("doRemovePoint", () => { + it("does nothing when type is not an unlimited graph", () => { + const state: InteractiveGraphState = baseCircleGraphState; + + const updated = interactiveGraphReducer( + state, + actions.pointGraph.removePoint(0), + ); + + expect(updated).toMatchObject(state); + }); + + it("removes a point at a specific index in our coordinates array", () => { + const state: InteractiveGraphState = { + ...baseUnlimitedPointGraphState, + interactionMode: "keyboard", + coords: [ + [0, 0], + [2, -2], + ], + }; + + const updated = interactiveGraphReducer( + state, + actions.pointGraph.removePoint(0), + ); + + invariant(updated.type === "point"); + expect(updated.coords).toMatchObject([[2, -2]]); + expect(updated.showRemovePointButton).toBeFalsy(); + expect(updated.focusedPointIndex).toBe(0); + expect(updated.showRemovePointButton).toBeFalsy(); + }); + + it("focusedPointIndex is set to null if interaction mode is in mouse", () => { + const state: InteractiveGraphState = { + ...baseUnlimitedPointGraphState, + interactionMode: "mouse", + focusedPointIndex: 0, + coords: [ + [0, 0], + [2, -2], + ], + }; + + const updated = interactiveGraphReducer( + state, + actions.pointGraph.removePoint(0), + ); + + invariant(updated.type === "point"); + expect(updated.coords).toMatchObject([[2, -2]]); + expect(updated.showRemovePointButton).toBeFalsy(); + expect(updated.focusedPointIndex).toBe(null); + expect(updated.showRemovePointButton).toBeFalsy(); + }); }); -xdescribe("doAddPoint", () => { - // TODO(catjohnson): Add tests for doAddPoint function. +describe("doClosePolygon", () => { + it("does nothing when type is not `polygon`", () => { + const state: InteractiveGraphState = baseUnlimitedPointGraphState; + + const updated = interactiveGraphReducer( + state, + actions.polygon.closePolygon(), + ); + invariant(updated.type !== "polygon"); + expect(updated).toMatchObject(state); + }); + + it("does nothing when type is not an unlimited graph", () => { + const state: InteractiveGraphState = { + ...basePolygonGraphState, + closedPolygon: false, + }; + + const updated = interactiveGraphReducer( + state, + actions.polygon.closePolygon(), + ); + + invariant(updated.type === "polygon"); + expect(updated.closedPolygon).toBeFalsy(); + }); + + it("changes `closedPolygon` property to true", () => { + const state: InteractiveGraphState = { + ...baseUnlimitedPolygonGraphState, + closedPolygon: false, + }; + + const updated = interactiveGraphReducer( + state, + actions.polygon.closePolygon(), + ); + + invariant(updated.type === "polygon"); + expect(updated.closedPolygon).toBeTruthy(); + }); + + it("does not change `closedPolygon` property when it's already false", () => { + const state: InteractiveGraphState = { + ...baseUnlimitedPolygonGraphState, + closedPolygon: true, + }; + + const updated = interactiveGraphReducer( + state, + actions.polygon.closePolygon(), + ); + + invariant(updated.type === "polygon"); + expect(updated.closedPolygon).toBeTruthy(); + }); }); -xdescribe("doRemovePoint", () => { - // TODO(catjohnson): Add tests for doRemovePoint function. +describe("doOpenPolygon", () => { + it("does nothing when type is not `polygon`", () => { + const state: InteractiveGraphState = baseUnlimitedPointGraphState; + + const updated = interactiveGraphReducer( + state, + actions.polygon.openPolygon(), + ); + + invariant(updated.type !== "polygon"); + expect(updated).toMatchObject(state); + }); + + it("does nothing when type is not an unlimited graph", () => { + const state: InteractiveGraphState = { + ...basePolygonGraphState, + closedPolygon: true, + }; + + const updated = interactiveGraphReducer( + state, + actions.polygon.openPolygon(), + ); + + invariant(updated.type === "polygon"); + expect(updated.closedPolygon).toBeTruthy(); + }); + + it("changes `closedPolygon` property to false", () => { + const state: InteractiveGraphState = { + ...baseUnlimitedPolygonGraphState, + closedPolygon: true, + }; + + const updated = interactiveGraphReducer( + state, + actions.polygon.openPolygon(), + ); + + invariant(updated.type === "polygon"); + expect(updated.closedPolygon).toBeFalsy(); + }); + + it("does not change `closedPolygon` property when it's already false", () => { + const state: InteractiveGraphState = { + ...baseUnlimitedPolygonGraphState, + closedPolygon: false, + }; + + const updated = interactiveGraphReducer( + state, + actions.polygon.openPolygon(), + ); + + invariant(updated.type === "polygon"); + expect(updated.closedPolygon).toBeFalsy(); + }); }); describe("unlimited points", () => { @@ -1238,11 +1660,6 @@ describe("unlimited polygon", () => { expect(stateAfterAddingPoint.coords).toMatchObject([[8, 10]]); }); - xit("does not adds point to polygon when polygon is closed", () => { - // TODO(catjohnson): ensure unlimited polygon does not add points - // when the polygon is closed. - }); - it("removes point to polygon", () => { let state: PolygonGraphState = { ...baseUnlimitedPolygonGraphState, @@ -1273,9 +1690,4 @@ describe("unlimited polygon", () => { [3, 3], ]); }); - - xit("does not remove point to polygon when closed", () => { - // TODO(catjohnson): ensure unlimited polygon does not add points - // when the polygon is closed. - }); }); diff --git a/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-reducer.ts b/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-reducer.ts index 6acee8591f..60cecb3991 100644 --- a/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-reducer.ts +++ b/packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-reducer.ts @@ -65,6 +65,8 @@ import { type ChangeInteractionMode, CHANGE_KEYBOARD_INVITATION_VISIBILITY, type ChangeKeyboardInvitationVisibility, + CLOSE_POLYGON, + OPEN_POLYGON, } from "./interactive-graph-action"; import type {Coord} from "../../../interactive2/types"; @@ -113,6 +115,10 @@ export function interactiveGraphReducer( return doDeleteIntent(state, action); case CLICK_POINT: return doClickPoint(state, action); + case CLOSE_POLYGON: + return doClosePolygon(state); + case OPEN_POLYGON: + return doOpenPolygon(state); case CHANGE_INTERACTION_MODE: return doChangeInteractionMode(state, action); case CHANGE_KEYBOARD_INVITATION_VISIBILITY: @@ -192,6 +198,28 @@ function doClickPoint( return state; } +function doClosePolygon(state: InteractiveGraphState): InteractiveGraphState { + if (isUnlimitedGraphState(state) && state.type === "polygon") { + return { + ...state, + closedPolygon: true, + }; + } + + return state; +} + +function doOpenPolygon(state: InteractiveGraphState): InteractiveGraphState { + if (isUnlimitedGraphState(state) && state.type === "polygon") { + return { + ...state, + closedPolygon: false, + }; + } + + return state; +} + function doChangeInteractionMode( state: InteractiveGraphState, action: ChangeInteractionMode, @@ -664,13 +692,15 @@ function doAddPoint( } } + const newCoords = [...state.coords, snappedPoint]; + // If there's no point in spot where we want the new point to go we add it there return { ...state, hasBeenInteractedWith: true, - coords: [...state.coords, snappedPoint], + coords: newCoords, showRemovePointButton: false, - focusedPointIndex: state.coords.length, + focusedPointIndex: newCoords.length - 1, }; } diff --git a/packages/perseus/src/widgets/interactive-graphs/types.ts b/packages/perseus/src/widgets/interactive-graphs/types.ts index 866caea495..565643ca81 100644 --- a/packages/perseus/src/widgets/interactive-graphs/types.ts +++ b/packages/perseus/src/widgets/interactive-graphs/types.ts @@ -95,6 +95,7 @@ export interface PolygonGraphState extends InteractiveGraphStateCommon { showRemovePointButton: boolean; interactionMode: InteractionMode; showKeyboardInteractionInvitation: boolean; + closedPolygon: boolean; } export interface CircleGraphState extends InteractiveGraphStateCommon {