diff --git a/.changeset/strange-crews-dress.md b/.changeset/strange-crews-dress.md new file mode 100644 index 0000000000..3eacb99e0b --- /dev/null +++ b/.changeset/strange-crews-dress.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": minor +--- + +Remove unlimited points via keyboard diff --git a/packages/perseus/src/widgets/__tests__/interactive-graph.test.tsx b/packages/perseus/src/widgets/__tests__/interactive-graph.test.tsx index 53a861a01a..0182557004 100644 --- a/packages/perseus/src/widgets/__tests__/interactive-graph.test.tsx +++ b/packages/perseus/src/widgets/__tests__/interactive-graph.test.tsx @@ -350,6 +350,7 @@ describe("tabbing forward on a Mafs segment graph", () => { flags: {mafs: {segment: true}}, }); + await userEvent.tab(); await userEvent.tab(); // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access @@ -359,11 +360,12 @@ describe("tabbing forward on a Mafs segment graph", () => { expect(movablePoints[0]).toHaveFocus(); }); - it("focuses the whole segment second", async () => { + it("focuses the whole segment third", async () => { const {container} = renderQuestion(segmentQuestion, { flags: {mafs: {segment: true}}, }); + await userEvent.tab(); await userEvent.tab(); await userEvent.tab(); @@ -382,6 +384,7 @@ describe("tabbing forward on a Mafs segment graph", () => { await userEvent.tab(); await userEvent.tab(); await userEvent.tab(); + await userEvent.tab(); // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access const movablePoints = container.querySelectorAll( @@ -407,6 +410,7 @@ describe("tabbing backward on a Mafs segment graph", () => { await userEvent.tab(); await userEvent.tab(); await userEvent.tab(); + await userEvent.tab(); await userEvent.tab({shift: true}); // eslint-disable-next-line testing-library/no-container, testing-library/no-node-access @@ -421,6 +425,7 @@ describe("tabbing backward on a Mafs segment graph", () => { flags: {mafs: {segment: true}}, }); + await userEvent.tab(); await userEvent.tab(); await userEvent.tab(); await userEvent.tab({shift: true}); diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/point.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/point.tsx index 1420b45826..193515a4de 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/point.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/point.tsx @@ -79,6 +79,13 @@ export function UnlimitedPointGraph(props: PointGraphProps) { onMove={(destination) => dispatch(actions.pointGraph.movePoint(i, destination)) } + onFocusChange={(isFocused) => { + if (isFocused) { + dispatch(actions.pointGraph.focusPoint(i)); + } else { + dispatch(actions.pointGraph.blurPoint()); + } + }} /> ))} 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 965e164365..d26b50a34f 100644 --- a/packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/mafs-graph.test.tsx @@ -443,6 +443,7 @@ describe("MafsGraph", () => { const state: InteractiveGraphState = { type: "point", numPoints: 2, + focusedPointIndex: null, hasBeenInteractedWith: true, range: [ [-10, 10], @@ -698,6 +699,7 @@ describe("MafsGraph", () => { const state: InteractiveGraphState = { type: "point", numPoints: "unlimited", + focusedPointIndex: null, hasBeenInteractedWith: true, range: [ [-10, 10], @@ -731,6 +733,7 @@ describe("MafsGraph", () => { const state: InteractiveGraphState = { type: "point", numPoints: "unlimited", + focusedPointIndex: null, hasBeenInteractedWith: true, range: [ [-10, 10], diff --git a/packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx b/packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx index 0f9d1e987a..15bbaac3b7 100644 --- a/packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/mafs-graph.tsx @@ -57,6 +57,9 @@ export const MafsGraph = (props: MafsGraphProps) => { const {state, dispatch, labels, readOnly} = props; const [width, height] = props.box; const tickStep = props.step as vec.Vector2; + + const graphRef = React.useRef(null); + return ( { { marginBottom: "20px", pointerEvents: props.static ? "none" : "auto", userSelect: "none", + width, + height, + }} + onKeyUp={(event) => { + if (event.key === "Backspace") { + dispatch(actions.global.deleteIntent()); + graphRef.current?.focus(); + } }} + ref={graphRef} + tabIndex={0} > { ]); }); }); + +describe("delete intent", () => { + it("does nothing when no points are selected", () => { + let state: PointGraphState = { + ...baseUnlimitedPointGraphState, + }; + + state = interactiveGraphReducer( + state, + actions.pointGraph.addPoint([1, 1]), + ) as PointGraphState; + + state = interactiveGraphReducer( + state, + actions.pointGraph.addPoint([2, 2]), + ) as PointGraphState; + + state = interactiveGraphReducer( + state, + actions.global.deleteIntent(), + ) as PointGraphState; + + expect(state.coords).toMatchObject([ + [1, 1], + [2, 2], + ]); + }); + + it("removes points when a point is focused and a delete intent is received", () => { + let state: PointGraphState = { + ...baseUnlimitedPointGraphState, + }; + + // Add some points + state = interactiveGraphReducer( + state, + actions.pointGraph.addPoint([1, 1]), + ) as PointGraphState; + + state = interactiveGraphReducer( + state, + actions.pointGraph.addPoint([2, 2]), + ) as PointGraphState; + + state = interactiveGraphReducer( + state, + actions.pointGraph.addPoint([3, 3]), + ) as PointGraphState; + + // Focus a point + state = interactiveGraphReducer( + state, + actions.pointGraph.focusPoint(0), + ) as PointGraphState; + + // Fire a delete intent + state = interactiveGraphReducer( + state, + actions.global.deleteIntent(), + ) as PointGraphState; + + expect(state.coords).toMatchObject([ + [2, 2], + [3, 3], + ]); + }); +}); + +describe("unlimited points", () => { + it("adds points", () => { + const state: PointGraphState = { + ...baseUnlimitedPointGraphState, + }; + + const stateAfterAddingPoint = interactiveGraphReducer( + state, + actions.pointGraph.addPoint([8, 10]), + ) as PointGraphState; + + expect(stateAfterAddingPoint.coords).toMatchObject([[8, 10]]); + }); + + it("removes points", () => { + let state: PointGraphState = { + ...baseUnlimitedPointGraphState, + }; + + state = interactiveGraphReducer( + state, + actions.pointGraph.addPoint([1, 1]), + ) as PointGraphState; + + state = interactiveGraphReducer( + state, + actions.pointGraph.addPoint([2, 2]), + ) as PointGraphState; + + state = interactiveGraphReducer( + state, + actions.pointGraph.addPoint([3, 3]), + ) as PointGraphState; + + state = interactiveGraphReducer( + state, + actions.pointGraph.removePoint(1), + ) as PointGraphState; + + expect(state.coords).toMatchObject([ + [1, 1], + [3, 3], + ]); + }); +}); 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 6d3e69ced1..b5069e619c 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 @@ -20,6 +20,7 @@ import {bound} from "../utils"; import {initializeGraphState} from "./initialize-graph-state"; import { + actions, CHANGE_RANGE, CHANGE_SNAP_STEP, type ChangeRange, @@ -42,6 +43,12 @@ import { type AddPoint, REMOVE_POINT, type RemovePoint, + DELETE_INTENT, + type DeleteIntent, + FOCUS_POINT, + type FocusPoint, + BLUR_POINT, + type BlurPoint, } from "./interactive-graph-action"; import type {Coord} from "../../../interactive2/types"; @@ -82,11 +89,65 @@ export function interactiveGraphReducer( return doAddPoint(state, action); case REMOVE_POINT: return doRemovePoint(state, action); + case FOCUS_POINT: + return doFocusPoint(state, action); + case BLUR_POINT: + return doBlurPoint(state, action); + case DELETE_INTENT: + return doDeleteIntent(state, action); default: throw new UnreachableCaseError(action); } } +function doDeleteIntent( + state: InteractiveGraphState, + action: DeleteIntent, +): InteractiveGraphState { + // For unlimited point graphs + if (state.type === "point" && state.numPoints === "unlimited") { + // if there's a focused point + if (state.focusedPointIndex !== null) { + // Remove the focused focus + return doRemovePoint( + state, + actions.pointGraph.removePoint(state.focusedPointIndex), + ); + } + } + return state; +} + +function doFocusPoint( + state: InteractiveGraphState, + action: FocusPoint, +): InteractiveGraphState { + switch (state.type) { + case "point": + return { + ...state, + focusedPointIndex: action.index, + }; + default: + return state; + } +} + +function doBlurPoint( + state: InteractiveGraphState, + action: BlurPoint, +): InteractiveGraphState { + switch (state.type) { + case "point": + return { + ...state, + focusedPointIndex: null, + }; + default: + return state; + } +} + function doMovePointInFigure( state: InteractiveGraphState, action: MovePointInFigure, @@ -543,6 +604,7 @@ function doRemovePoint( return { ...state, coords: state.coords.filter((_, i) => i !== action.index), + focusedPointIndex: null, }; } diff --git a/packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.test.tsx b/packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.test.tsx index 4e7a334b1e..83fbd81f7b 100644 --- a/packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.test.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/stateful-mafs-graph.test.tsx @@ -67,6 +67,7 @@ describe("StatefulMafsGraph", () => { />, ); + await userEvent.tab(); await userEvent.tab(); await userEvent.keyboard("{arrowup}"); diff --git a/packages/perseus/src/widgets/interactive-graphs/types.ts b/packages/perseus/src/widgets/interactive-graphs/types.ts index 169f469cc1..5b503e7e0c 100644 --- a/packages/perseus/src/widgets/interactive-graphs/types.ts +++ b/packages/perseus/src/widgets/interactive-graphs/types.ts @@ -53,6 +53,7 @@ export interface PointGraphState extends InteractiveGraphStateCommon { type: "point"; coords: Coord[]; numPoints?: number | "unlimited"; + focusedPointIndex: number | null; } export interface RayGraphState extends InteractiveGraphStateCommon {