From 4c2ace57a922a527579293c065f8ed8120193344 Mon Sep 17 00:00:00 2001 From: Ben Christel Date: Thu, 8 Aug 2024 17:00:13 -0700 Subject: [PATCH] Add keyboard controls to angle graphs (#1472) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR makes it so angle graph questions can be solved with only a keyboard - no mouse required! There are several tricky constraints to this problem: - Every distinct state of the graph needs to be reachable using only the keyboard. - In order for the movement to feel natural, pressing an arrow key should always move the point in the corresponding direction: up, down, left, or right. - However, the points need to snap, not to a grid, but to angle measures (in degrees). - The side points of the angle should not be allowed to get too close to the vertex - if they overlap the vertex there's no way to drag them away again. - The points shouldn't be able to move outside the graph bounds - It should never be possible for an edge point to get "stuck" or blocked in its movement by the edge of the graph, or by the angle vertex. I believe this PR solves for all these constraints. To make it work, I had to do a few slightly weird things: - I constrained the movement of the vertex so no part of the angle-measure arc can go outside the graph bounds. This ensures that all the control points of the angle can always be placed onscreen, and prevents the side points from getting stuck on the edges of the graph. - I reworked the interface of `useDraggable`, renaming `constrain` to `constrainKeyboardMovement`. Movement constraints for dragging are now implemented entirely in the reducer. (They were almost all in the reducer before; circles were the only graph type that still depended on the constrain function for dragging.) - I also introduced a new type of `useDraggable` constraint, where the destination points for the up, left, right, and down arrows are listed explicitly. I think this ultimately makes the code clearer. It's also more efficient (since useDraggable does not have to do a bunch of repeated trig calculations to search for valid destination points). Issue: https://khanacademy.atlassian.net/browse/LEMS-2134 ## Test plan: View `/?path=/story/perseus-widgets-interactive-graph--angle-with-mafs` in Storybook. Try to break the graph. Author: benchristel Reviewers: benchristel, SonicScrewdriver, mark-fitzgerald, Myranae, nicolecomputer, nishasy Required Reviewers: Approved By: SonicScrewdriver Checks: ✅ codecov/project, ✅ codecov/patch, ✅ Upload Coverage (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Jest Coverage (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1472 --- .changeset/neat-days-remain.md | 5 + .../__stories__/interactive-graph.stories.tsx | 5 + .../interactive-graphs/graphs/angle.test.ts | 69 ++++++++++ .../interactive-graphs/graphs/angle.tsx | 124 +++++++++++++++--- .../interactive-graphs/graphs/circle.tsx | 6 +- .../graphs/components/angle-indicators.tsx | 1 - .../graphs/components/movable-line.tsx | 6 +- .../graphs/components/movable-point.test.tsx | 20 +-- .../graphs/components/movable-point.tsx | 17 ++- .../interactive-graphs/graphs/point.tsx | 4 +- .../interactive-graphs/graphs/polygon.tsx | 25 ++-- .../interactive-graphs/graphs/quadratic.tsx | 4 +- .../interactive-graphs/graphs/sinusoid.tsx | 4 +- .../graphs/use-draggable.test.tsx | 67 +++++++--- .../graphs/use-draggable.ts | 54 ++++++-- .../widgets/interactive-graphs/math/angles.ts | 1 + .../interactive-graphs/math/geometry.test.ts | 64 ++++++++- .../interactive-graphs/math/geometry.ts | 27 +++- .../widgets/interactive-graphs/protractor.tsx | 2 +- .../reducer/interactive-graph-reducer.test.ts | 17 +++ .../reducer/interactive-graph-reducer.ts | 112 +++++++++------- 21 files changed, 505 insertions(+), 129 deletions(-) create mode 100644 .changeset/neat-days-remain.md create mode 100644 packages/perseus/src/widgets/interactive-graphs/graphs/angle.test.ts diff --git a/.changeset/neat-days-remain.md b/.changeset/neat-days-remain.md new file mode 100644 index 0000000000..885a70935d --- /dev/null +++ b/.changeset/neat-days-remain.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": minor +--- + +Add keyboard controls to Mafs angle graphs diff --git a/packages/perseus/src/widgets/__stories__/interactive-graph.stories.tsx b/packages/perseus/src/widgets/__stories__/interactive-graph.stories.tsx index 77d9f2119d..3df8714a45 100644 --- a/packages/perseus/src/widgets/__stories__/interactive-graph.stories.tsx +++ b/packages/perseus/src/widgets/__stories__/interactive-graph.stories.tsx @@ -33,6 +33,7 @@ const enableMafs: APIOptions = { mafs: { segment: true, polygon: true, + angle: true, }, }, }; @@ -155,6 +156,10 @@ export const Sinusoid = (args: StoryArgs): React.ReactElement => ( ); +export const AngleWithMafs = (args: StoryArgs): React.ReactElement => ( + +); + // TODO(jeremy): As of Jan 2022 there are no peresus items in production that // use the "quadratic" graph type. // "quadratic" diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/angle.test.ts b/packages/perseus/src/widgets/interactive-graphs/graphs/angle.test.ts new file mode 100644 index 0000000000..fefc07d519 --- /dev/null +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/angle.test.ts @@ -0,0 +1,69 @@ +import {getAngleSideConstraint} from "./angle"; + +import type {vec} from "mafs"; + +function closeTo(x: number) { + return expect.closeTo(x, 6); +} + +describe("getAngleSideConstraint", () => { + it("prevents vertical movement given a vertical side of an angle", () => { + const side: vec.Vector2 = [0, 5]; + const vertex: vec.Vector2 = [0, 0]; + const snapDegrees = 45; + + const constraint = getAngleSideConstraint(side, vertex, snapDegrees); + + expect(constraint).toEqual({ + up: side, + down: side, + left: [closeTo(-5), 5], + right: [closeTo(5), 5], + }); + }); + + it("prevents horizontal movement given a horizontal side of an angle", () => { + const side: vec.Vector2 = [5, 0]; + const vertex: vec.Vector2 = [0, 0]; + const snapDegrees = 45; + + const constraint = getAngleSideConstraint(side, vertex, snapDegrees); + + expect(constraint).toEqual({ + up: [5, closeTo(5)], + down: [5, closeTo(-5)], + left: side, + right: side, + }); + }); + + it("assigns the correct points to 'left' and 'right' when the side is pointing down", () => { + const side: vec.Vector2 = [0, -5]; + const vertex: vec.Vector2 = [0, 0]; + const snapDegrees = 45; + + const constraint = getAngleSideConstraint(side, vertex, snapDegrees); + + expect(constraint).toEqual({ + up: side, + down: side, + left: [closeTo(-5), -5], + right: [closeTo(5), -5], + }); + }); + + it("assigns the correct points to 'up' and 'down' when the side is pointing left", () => { + const side: vec.Vector2 = [-5, 0]; + const vertex: vec.Vector2 = [0, 0]; + const snapDegrees = 45; + + const constraint = getAngleSideConstraint(side, vertex, snapDegrees); + + expect(constraint).toEqual({ + up: [-5, closeTo(5)], + down: [-5, closeTo(-5)], + left: side, + right: side, + }); + }); +}); diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/angle.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/angle.tsx index cd636c2d5e..cadd5a57fe 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/angle.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/angle.tsx @@ -1,19 +1,22 @@ +import {vec} from "mafs"; import * as React from "react"; +import {calculateAngleInDegrees, polar} from "../math"; +import {findIntersectionOfRays} from "../math/geometry"; import {actions} from "../reducer/interactive-graph-action"; import useGraphConfig from "../reducer/use-graph-config"; import {Angle} from "./components/angle-indicators"; import {trimRange} from "./components/movable-line"; -import {StyledMovablePoint} from "./components/movable-point"; +import {MovablePoint} from "./components/movable-point"; import {SVGLine} from "./components/svg-line"; import {Vector} from "./components/vector"; import {useTransformVectorsToPixels} from "./use-transform"; import {getIntersectionOfRayWithBox} from "./utils"; import type {CollinearTuple} from "../../../perseus-types"; +import type {Segment} from "../math/geometry"; import type {AngleGraphState, MafsGraphProps} from "../types"; -import type {vec} from "mafs"; type AngleGraphProps = MafsGraphProps; @@ -21,14 +24,8 @@ export function AngleGraph(props: AngleGraphProps) { const {dispatch, graphState} = props; const {graphDimensionsInPixels} = useGraphConfig(); - const { - coords, - showAngles, - range, - allowReflexAngles, - angleOffsetDeg, - snapDegrees, - } = graphState; + const {coords, showAngles, range, allowReflexAngles, snapDegrees} = + graphState; // Break the coords into the two end points and the center point const endPoints: [vec.Vector2, vec.Vector2] = [coords[0], coords[2]]; @@ -78,7 +75,6 @@ export function AngleGraph(props: AngleGraphProps) { vertex: centerPoint, coords: endPoints, allowReflexAngles: allowReflexAngles || false, // Whether to allow reflex angles or not - angleOffsetDeg: angleOffsetDeg || 0, // The angle offset from the x-axis snapDegrees: snapDegrees || 1, // The multiple of degrees to snap to range: range, showAngles: showAngles || false, // Whether to show the angle or not @@ -89,16 +85,102 @@ export function AngleGraph(props: AngleGraphProps) { <> {svgLines} - {coords.map((point, i) => ( - - dispatch(actions.angle.movePoint(i, destination)) - } - /> - ))} + {/* vertex */} + p} + onMove={(destination: vec.Vector2) => + dispatch(actions.angle.movePoint(1, destination)) + } + /> + {/* side 1 */} + + dispatch(actions.angle.movePoint(0, destination)) + } + /> + {/* side 2 */} + + dispatch(actions.angle.movePoint(2, destination)) + } + /> ); } + +const positiveX: vec.Vector2 = [1, 0]; +const negativeX: vec.Vector2 = [-1, 0]; +const positiveY: vec.Vector2 = [0, 1]; +const negativeY: vec.Vector2 = [0, -1]; +export function getAngleSideConstraint( + sidePoint: [number, number], + vertex: [number, number], + snapDegrees: number, +): { + up: vec.Vector2; + down: vec.Vector2; + left: vec.Vector2; + right: vec.Vector2; +} { + const currentAngle = calculateAngleInDegrees(vec.sub(sidePoint, vertex)); + + // Find the rays that start at the current point and point up, down, left + // and right. + const leftRay: Segment = [sidePoint, vec.add(sidePoint, negativeX)]; + const rightRay: Segment = [sidePoint, vec.add(sidePoint, positiveX)]; + const upRay: Segment = [sidePoint, vec.add(sidePoint, positiveY)]; + const downRay: Segment = [sidePoint, vec.add(sidePoint, negativeY)]; + + // find the angles that lie one snap step clockwise and counter-clockwise + // from the current angle. These are the angles to which the side can be + // moved. + const oneStepCounterClockwise = currentAngle + snapDegrees; + const oneStepClockwise = currentAngle - snapDegrees; + + // find the rays that start from the vertex and point in the direction of + // the angles we just computed. + const counterClockwiseRay: Segment = [ + vertex, + vec.add(vertex, polar(1, oneStepCounterClockwise)), + ]; + const clockwiseRay: Segment = [ + vertex, + vec.add(vertex, polar(1, oneStepClockwise)), + ]; + + // find the intersections of those rays with the horizontal and vertical + // rays extending from the sidePoint. These intersections are the points to + // which sidePoint can move that will result in a rotation of `snapDegrees`. + const left = + findIntersectionOfRays(leftRay, counterClockwiseRay) ?? + findIntersectionOfRays(leftRay, clockwiseRay); + const right = + findIntersectionOfRays(rightRay, counterClockwiseRay) ?? + findIntersectionOfRays(rightRay, clockwiseRay); + const up = + findIntersectionOfRays(upRay, counterClockwiseRay) ?? + findIntersectionOfRays(upRay, clockwiseRay); + const down = + findIntersectionOfRays(downRay, counterClockwiseRay) ?? + findIntersectionOfRays(downRay, clockwiseRay); + + return { + up: up ?? sidePoint, + down: down ?? sidePoint, + left: left ?? sidePoint, + right: right ?? sidePoint, + }; +} diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/circle.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/circle.tsx index 8f4e220ba2..97c985f4dd 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/circle.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/circle.tsx @@ -7,7 +7,7 @@ import {actions} from "../reducer/interactive-graph-action"; import {getRadius} from "../reducer/interactive-graph-state"; import useGraphConfig from "../reducer/use-graph-config"; -import {StyledMovablePoint} from "./components/movable-point"; +import {MovablePoint} from "./components/movable-point"; import {useDraggable} from "./use-draggable"; import { useTransformDimensionsToPixels, @@ -29,7 +29,7 @@ export function CircleGraph(props: CircleGraphProps) { radius={getRadius(graphState)} onMove={(c) => dispatch(actions.circle.moveCenter(c))} /> - { @@ -54,7 +54,7 @@ function MovableCircle(props: { gestureTarget: draggableRef, point: center, onMove, - constrain: (p) => snap(snapStep, p), + constrainKeyboardMovement: (p) => snap(snapStep, p), }); const [centerPx] = useTransformVectorsToPixels(center); diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/components/angle-indicators.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/components/angle-indicators.tsx index 6be5d4078f..4494e40715 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/components/angle-indicators.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/components/angle-indicators.tsx @@ -140,7 +140,6 @@ interface AngleProps { coords: [vec.Vector2, vec.Vector2]; showAngles: boolean; allowReflexAngles: boolean; - angleOffsetDeg: number; snapDegrees: number; range: [Interval, Interval]; } diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-line.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-line.tsx index d10d8d6ad1..c7c5afec12 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-line.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-line.tsx @@ -85,7 +85,7 @@ function useControlPoint( gestureTarget: keyboardHandleRef, point, onMove: onMovePoint, - constrain: (p) => snap(snapStep, p), + constrainKeyboardMovement: (p) => snap(snapStep, p), }); const visiblePointRef = useRef(null); @@ -93,7 +93,7 @@ function useControlPoint( gestureTarget: visiblePointRef, point, onMove: onMovePoint, - constrain: (p) => snap(snapStep, p), + constrainKeyboardMovement: (p) => snap(snapStep, p), }); const focusableHandle = ( @@ -169,7 +169,7 @@ export const Line = (props: LineProps) => { onMove: (newPoint) => { onMove(vec.sub(newPoint, start)); }, - constrain: (p) => snap(snapStep, p), + constrainKeyboardMovement: (p) => snap(snapStep, p), }); return ( diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.test.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.test.tsx index 40e5d253f0..963a88b7a0 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.test.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.test.tsx @@ -6,7 +6,7 @@ import React from "react"; import * as ReducerGraphConfig from "../../reducer/use-graph-config"; import * as UseDraggableModule from "../use-draggable"; -import {StyledMovablePoint} from "./movable-point"; +import {MovablePoint} from "./movable-point"; import type {GraphConfig} from "../../reducer/use-graph-config"; @@ -25,7 +25,7 @@ const TooltipMock = ({children}) => { return children; }; -describe("StyledMovablePoint", () => { +describe("MovablePoint", () => { let useGraphConfigMock: jest.SpyInstance; let useDraggableMock: jest.SpyInstance; const baseGraphConfigContext: GraphConfig = { @@ -72,7 +72,7 @@ describe("StyledMovablePoint", () => { useGraphConfigMock.mockReturnValue(graphConfigContextWithTooltips); render( - {}} />, + {}} />, , ); expect(Tooltip).toHaveBeenCalled(); @@ -82,7 +82,7 @@ describe("StyledMovablePoint", () => { useGraphConfigMock.mockReturnValue(baseGraphConfigContext); render( - {}} />, + {}} />, , ); expect(Tooltip).not.toHaveBeenCalled(); @@ -92,7 +92,7 @@ describe("StyledMovablePoint", () => { useGraphConfigMock.mockReturnValue(graphConfigContextWithTooltips); render( - {}} />, + {}} />, , ); // @ts-expect-error // TS2339: Property mock does not exist on type typeof Tooltip @@ -105,7 +105,7 @@ describe("StyledMovablePoint", () => { useGraphConfigMock.mockReturnValue(graphConfigContextWithTooltips); render( - {}} @@ -123,7 +123,7 @@ describe("StyledMovablePoint", () => { useGraphConfigMock.mockReturnValue(graphConfigContextWithTooltips); render( - {}} @@ -144,7 +144,7 @@ describe("StyledMovablePoint", () => { useDraggableMock.mockReturnValue({dragging: true}); const {container} = render( - {}} />, + {}} />, , ); @@ -160,7 +160,7 @@ describe("StyledMovablePoint", () => { useGraphConfigMock.mockReturnValue(baseGraphConfigContext); const {container} = render( - {}} />, + {}} />, , ); @@ -176,7 +176,7 @@ describe("StyledMovablePoint", () => { useDraggableMock.mockReturnValue({dragging: true}); const {container} = render( - {}} />, + {}} />, , ); diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.tsx index aa25bb7eb5..14ea97b0b9 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/components/movable-point.tsx @@ -9,6 +9,7 @@ import {useDraggable} from "../use-draggable"; import {MovablePointView} from "./movable-point-view"; import type {CSSCursor} from "./css-cursor"; +import type {KeyboardMovementConstraint} from "../use-draggable"; import type {vec} from "mafs"; type Props = { @@ -16,20 +17,24 @@ type Props = { onMove: (newPoint: vec.Vector2) => unknown; color?: string; cursor?: CSSCursor | undefined; - snapTo?: "grid" | "angles" | "sides"; + constrain?: KeyboardMovementConstraint; }; -export const StyledMovablePoint = (props: Props) => { +export const MovablePoint = (props: Props) => { const {snapStep} = useGraphConfig(); const elementRef = useRef(null); - const {point, onMove, cursor, color = WBColor.blue, snapTo} = props; - const snapToValue = snapTo ?? "grid"; + const { + point, + onMove, + cursor, + color = WBColor.blue, + constrain = (p) => snap(snapStep, p), + } = props; const {dragging} = useDraggable({ gestureTarget: elementRef, point, onMove, - constrain: (p) => - ["angles", "sides"].includes(snapToValue) ? p : snap(snapStep, p), + constrainKeyboardMovement: constrain, }); return ( diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/point.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/point.tsx index 0877650d21..92a12cb5dd 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/point.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/point.tsx @@ -2,7 +2,7 @@ import * as React from "react"; import {actions} from "../reducer/interactive-graph-action"; -import {StyledMovablePoint} from "./components/movable-point"; +import {MovablePoint} from "./components/movable-point"; import type {PointGraphState, MafsGraphProps} from "../types"; @@ -13,7 +13,7 @@ export function PointGraph(props: PointGraphProps) { return ( <> {props.graphState.coords.map((point, i) => ( - diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx index 040e62a1df..f3c0c4234f 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/polygon.tsx @@ -7,7 +7,7 @@ import useGraphConfig from "../reducer/use-graph-config"; import {TARGET_SIZE} from "../utils"; import {PolygonAngle} from "./components/angle-indicators"; -import {StyledMovablePoint} from "./components/movable-point"; +import {MovablePoint} from "./components/movable-point"; import {TextLabel} from "./components/text-label"; import {useDraggable} from "./use-draggable"; @@ -23,8 +23,14 @@ export const PolygonGraph = (props: Props) => { const [focusVisible, setFocusVisible] = React.useState(false); const {dispatch} = props; - const {coords, showAngles, showSides, range, snapStep, snapTo} = - props.graphState; + const { + coords, + showAngles, + showSides, + range, + snapStep, + snapTo = "grid", + } = props.graphState; const {disableKeyboardInteraction} = useGraphConfig(); // TODO(benchristel): can the default set of points be removed here? I don't @@ -33,7 +39,9 @@ export const PolygonGraph = (props: Props) => { const ref = React.useRef(null); const dragReferencePoint = points[0]; - const snapToValue = snapTo ?? "grid"; + const constrain = ["angles", "sides"].includes(snapTo) + ? (p) => p + : (p) => snap(snapStep, p); const {dragging} = useDraggable({ gestureTarget: ref, point: dragReferencePoint, @@ -41,8 +49,7 @@ export const PolygonGraph = (props: Props) => { const delta = vec.sub(newPoint, dragReferencePoint); dispatch(actions.polygon.moveAll(delta)); }, - constrain: (p) => - ["angles", "sides"].includes(snapToValue) ? p : snap(snapStep, p), + constrainKeyboardMovement: constrain, }); const lastMoveTime = React.useRef(0); @@ -75,7 +82,7 @@ export const PolygonGraph = (props: Props) => { range={range} polygonLines={lines} showAngles={!!showAngles} - snapTo={snapToValue} + snapTo={snapTo} /> ); })} @@ -126,9 +133,9 @@ export const PolygonGraph = (props: Props) => { }} /> {points.map((point, i) => ( - { const now = Date.now(); diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/quadratic.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/quadratic.tsx index a741db1c67..746425e164 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/quadratic.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/quadratic.tsx @@ -4,7 +4,7 @@ import * as React from "react"; import {actions} from "../reducer/interactive-graph-action"; -import {StyledMovablePoint} from "./components/movable-point"; +import {MovablePoint} from "./components/movable-point"; import type {QuadraticGraphState, MafsGraphProps} from "../types"; @@ -36,7 +36,7 @@ export function QuadraticGraph(props: QuadraticGraphProps) { <> {coords.map((coord, i) => ( - diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/sinusoid.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/sinusoid.tsx index 73b95c57bb..46cf219d2c 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/sinusoid.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/sinusoid.tsx @@ -5,7 +5,7 @@ import * as React from "react"; import {X, Y} from "../math"; import {actions} from "../reducer/interactive-graph-action"; -import {StyledMovablePoint} from "./components/movable-point"; +import {MovablePoint} from "./components/movable-point"; import type {Coord} from "../../../interactive2/types"; import type {SinusoidGraphState, MafsGraphProps} from "../types"; @@ -51,7 +51,7 @@ export function SinusoidGraph(props: SinusoidGraphProps) { color={color.blue} /> {coords.map((coord, i) => ( - diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/use-draggable.test.tsx b/packages/perseus/src/widgets/interactive-graphs/graphs/use-draggable.test.tsx index 15a917dceb..ade51c9d75 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/use-draggable.test.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/use-draggable.test.tsx @@ -4,22 +4,25 @@ import {Mafs, Transform} from "mafs"; import * as React from "react"; import {useRef} from "react"; +import {snap} from "../math"; + import {useDraggable} from "./use-draggable"; +import type {KeyboardMovementConstraint} from "./use-draggable"; import type {UserEvent} from "@testing-library/user-event"; import type {vec, Interval} from "mafs"; function TestDraggable(props: { point: vec.Vector2; - constrain?: (point: vec.Vector2) => vec.Vector2; + constrainKeyboardMovement?: KeyboardMovementConstraint; onMove?: (point: vec.Vector2) => unknown; }) { - const {onMove = () => {}, constrain = (p) => p} = props; + const {onMove = () => {}, constrainKeyboardMovement = (p) => p} = props; const gestureTarget = useRef(null); const {dragging} = useDraggable({ - ...props, + point: props.point, onMove, - constrain, + constrainKeyboardMovement, gestureTarget, }); return ( @@ -109,7 +112,7 @@ describe("useDraggable", () => { expect(onMoveSpy).toHaveBeenCalledWith([1, -1]); }); - it("constrains the destination point using the given constrain function", async () => { + it("constrains the destination point using a constrainKeyboardMovement function", async () => { // Arrange: a 200x200px graph with a 20-unit range in each dimension. // One graph unit = 10px. const mafsProps = { @@ -127,21 +130,55 @@ describe("useDraggable", () => { [Math.round(p[0]), Math.round(p[1])]} + constrainKeyboardMovement={(p) => snap([1, 1], p)} /> , ); const dragHandle = screen.getByRole("button"); - // Act: click and hold the drag handle... - mouseDownAt(dragHandle, 0, 0); - // ...and then drag 12px right and 13px down - moveMouseTo(dragHandle, 12, 13); + // Act + await userEvent.tab(); + await userEvent.type(dragHandle, "{arrowright}"); - // Assert: the draggable element was moved to (1, -1) due to the - // constrain function. If you see (1.2, -1.3) instead, that means the - // constraint is not being applied. - expect(onMoveSpy).toHaveBeenCalledWith([1, -1]); + // Assert: the draggable element was moved one step to the right + expect(onMoveSpy.mock.calls).toEqual([[[1, 0]]]); + }); + + it("constrains the destination point using a constrainKeyboardMovement object", async () => { + // Arrange: a 200x200px graph with a 20-unit range in each dimension. + // One graph unit = 10px. + const mafsProps = { + width: 200, + height: 200, + viewBox: { + x: [-10, 10] as Interval, + y: [-10, 10] as Interval, + padding: 0, + }, + }; + const onMoveSpy = jest.fn(); + render( + + + , + ); + const dragHandle = screen.getByRole("button"); + + // Act + await userEvent.tab(); + await userEvent.type(dragHandle, "{arrowright}"); + + // Assert: + expect(onMoveSpy.mock.calls).toEqual([[[4, 4]]]); }); it("accounts for the user transform when measuring drag distance", async () => { @@ -201,7 +238,7 @@ describe("useDraggable", () => { [ + constrainKeyboardMovement={(point) => [ Math.round(point[0]), Math.round(point[1]), ]} diff --git a/packages/perseus/src/widgets/interactive-graphs/graphs/use-draggable.ts b/packages/perseus/src/widgets/interactive-graphs/graphs/use-draggable.ts index 5717e0c337..f3e29d8b62 100644 --- a/packages/perseus/src/widgets/interactive-graphs/graphs/use-draggable.ts +++ b/packages/perseus/src/widgets/interactive-graphs/graphs/use-draggable.ts @@ -30,15 +30,36 @@ export type Params = { gestureTarget: RefObject; onMove: (point: vec.Vector2) => unknown; point: vec.Vector2; - constrain: (point: vec.Vector2) => vec.Vector2; + constrainKeyboardMovement: KeyboardMovementConstraint; }; +export type KeyboardMovementConstraint = + // The function form of KeyboardMovementConstraint should be a "snap" + // function that takes a target point to move near, and returns the closest + // valid point. If the function form is used, useDraggable will search for + // the valid destination that is closest to the current point in the + // direction of the movement. + | ((point: vec.Vector2) => vec.Vector2) + // Alternatively, the movement can be constrained to specific + // pre-determined points based on which key is pressed. + | { + left: vec.Vector2; + right: vec.Vector2; + up: vec.Vector2; + down: vec.Vector2; + }; + type DragState = { dragging: boolean; }; export function useDraggable(args: Params): DragState { - const {gestureTarget: target, onMove, point, constrain} = args; + const { + gestureTarget: target, + onMove, + point, + constrainKeyboardMovement, + } = args; const [dragging, setDragging] = React.useState(false); const {xSpan, ySpan} = useSpanContext(); const {viewTransform, userTransform} = useTransformContext(); @@ -60,6 +81,7 @@ export function useDraggable(args: Params): DragState { const isKeyboard = type.includes("key"); if (isKeyboard) { + invariant(event instanceof KeyboardEvent); event?.preventDefault(); // When a key is held down, we see multiple "keydown" events, @@ -72,6 +94,13 @@ export function useDraggable(args: Params): DragState { return; } + if (typeof constrainKeyboardMovement === "object") { + const destination = + constrainKeyboardMovement[directionForKey[event.key]]; + onMove(destination ?? point); + return; + } + const { direction: yDownDirection, altKey, @@ -83,6 +112,7 @@ export function useDraggable(args: Params): DragState { yDownDirection[X], -yDownDirection[Y], ] as vec.Vector2; + const span = Math.abs(direction[X]) ? xSpan : ySpan; let divisions = 50; @@ -103,7 +133,7 @@ export function useDraggable(args: Params): DragState { for (const dx of tests) { // Transform the test back into the point's coordinate system const testMovement = vec.scale(direction, dx); - const testPoint = constrain( + const testPoint = constrainKeyboardMovement( vec.transform( vec.add( vec.transform(point, userTransform), @@ -135,11 +165,9 @@ export function useDraggable(args: Params): DragState { inverseViewTransform, ); onMove( - constrain( - vec.transform( - vec.add(pickup.current, movement), - inverseTransform, - ), + vec.transform( + vec.add(pickup.current, movement), + inverseTransform, ), ); } @@ -149,6 +177,16 @@ export function useDraggable(args: Params): DragState { return {dragging}; } +const directionForKey: Record< + "ArrowLeft" | "ArrowRight" | "ArrowUp" | "ArrowDown", + "left" | "right" | "up" | "down" | undefined +> = { + ArrowLeft: "left", + ArrowRight: "right", + ArrowUp: "up", + ArrowDown: "down", +}; + function getInverseTransform(transform: vec.Matrix) { const invert = vec.matrixInvert(transform); invariant( diff --git a/packages/perseus/src/widgets/interactive-graphs/math/angles.ts b/packages/perseus/src/widgets/interactive-graphs/math/angles.ts index a9ffdbfb2c..c45c1b43b9 100644 --- a/packages/perseus/src/widgets/interactive-graphs/math/angles.ts +++ b/packages/perseus/src/widgets/interactive-graphs/math/angles.ts @@ -12,6 +12,7 @@ export function calculateAngleInDegrees([x, y]: vec.Vector2): number { return (Math.atan2(y, x) * 180) / Math.PI; } +// Converts polar coordinates to cartesian. The th(eta) parameter is in degrees. export function polar(r: number | vec.Vector2, th: number): vec.Vector2 { if (typeof r === "number") { r = [r, r]; diff --git a/packages/perseus/src/widgets/interactive-graphs/math/geometry.test.ts b/packages/perseus/src/widgets/interactive-graphs/math/geometry.test.ts index 8189f04750..3aa95fdfab 100644 --- a/packages/perseus/src/widgets/interactive-graphs/math/geometry.test.ts +++ b/packages/perseus/src/widgets/interactive-graphs/math/geometry.test.ts @@ -1,4 +1,4 @@ -import {segmentsIntersect} from "./geometry"; +import {findIntersectionOfRays, segmentsIntersect} from "./geometry"; import type {Segment} from "./geometry"; @@ -176,3 +176,65 @@ describe("segmentsIntersect", () => { expect(segmentsIntersect(segment1, segment2)).toBe(false); }); }); + +describe("findIntersectionOfRays", () => { + it("returns undefined when the direction of the first ray is undefined", () => { + const ray1: Segment = [ + [0, 0], + [0, 0], + ]; + const ray2: Segment = [ + [-1, -1], + [1, 1], + ]; + expect(findIntersectionOfRays(ray1, ray2)).toBe(undefined); + }); + + it("returns undefined when the direction of the second ray is undefined", () => { + const ray1: Segment = [ + [-1, -1], + [1, 1], + ]; + const ray2: Segment = [ + [0, 0], + [0, 0], + ]; + expect(findIntersectionOfRays(ray1, ray2)).toBe(undefined); + }); + + it("returns undefined when the rays are parallel", () => { + const ray1: Segment = [ + [0, 0], + [1, 1], + ]; + const ray2: Segment = [ + [0, 1], + [1, 2], + ]; + expect(findIntersectionOfRays(ray1, ray2)).toBe(undefined); + }); + + it("returns the intersection point", () => { + const ray1: Segment = [ + [0, 0], + [1, 1], + ]; + const ray2: Segment = [ + [1, 0], + [1, 0.5], + ]; + expect(findIntersectionOfRays(ray1, ray2)).toEqual([1, 1]); + }); + + it("returns undefined when one ray points away from the other", () => { + const ray1: Segment = [ + [0, 0], + [1, 1], + ]; + const ray2: Segment = [ + [1, 0], + [1, -0.5], + ]; + expect(findIntersectionOfRays(ray1, ray2)).toBe(undefined); + }); +}); diff --git a/packages/perseus/src/widgets/interactive-graphs/math/geometry.ts b/packages/perseus/src/widgets/interactive-graphs/math/geometry.ts index bda3e75074..95128b632f 100644 --- a/packages/perseus/src/widgets/interactive-graphs/math/geometry.ts +++ b/packages/perseus/src/widgets/interactive-graphs/math/geometry.ts @@ -1,4 +1,4 @@ -import type {vec} from "mafs"; +import {vec} from "mafs"; export type Segment = [vec.Vector2, vec.Vector2]; @@ -19,3 +19,28 @@ export const segmentsIntersect = ( return 0 < lambda && lambda < 1 && 0 < gamma && gamma < 1; } }; + +// Finds the unique intersection point between two rays. Returns undefined if +// there is not exactly one intersection point. The first point in each of the +// given ray "segments" should be the initial point of the ray. The second point +// can be any arbitrary point on the ray different from the initial point. +export function findIntersectionOfRays( + [[a, b], [c, d]]: Segment, + [[p, q], [r, s]]: Segment, +): vec.Vector2 | undefined { + // See https://stackoverflow.com/a/24392281/7347484 for an explanation of + // the determinant and lambda values. + const determinant = (c - a) * (s - q) - (r - p) * (d - b); + if (determinant === 0) { + return undefined; + } else { + const lambda = ((s - q) * (r - a) + (p - r) * (s - b)) / determinant; + const gamma = ((b - d) * (r - a) + (c - a) * (s - b)) / determinant; + if (lambda <= 0 || gamma >= 1) { + return undefined; + } + const initialPoint: vec.Vector2 = [a, b]; + const direction = vec.sub([c, d], initialPoint); + return vec.add(initialPoint, vec.scale(direction, lambda)); + } +} diff --git a/packages/perseus/src/widgets/interactive-graphs/protractor.tsx b/packages/perseus/src/widgets/interactive-graphs/protractor.tsx index 8894f7c980..13823c8170 100644 --- a/packages/perseus/src/widgets/interactive-graphs/protractor.tsx +++ b/packages/perseus/src/widgets/interactive-graphs/protractor.tsx @@ -50,7 +50,7 @@ export function Protractor() { gestureTarget: draggableRef, onMove: setCenter, point: center, - constrain: (point) => bound({snapStep, range, point}), + constrainKeyboardMovement: (point) => bound({snapStep, range, point}), }); const rotationHandleRef = useRef(null); 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 aa7f7dda92..a4f1bb9abe 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 @@ -893,6 +893,23 @@ describe("doMoveRadiusPoint", () => { expect(updated.hasBeenInteractedWith).toBe(true); }); + it("snaps", () => { + const state: InteractiveGraphState = { + ...baseCircleGraphState, + snapStep: [2, 7], + }; + + const updated = interactiveGraphReducer( + state, + actions.circle.moveRadiusPoint([-3.1, 0]), + ); + + // make sure the state object is different + expect(state).not.toBe(updated); + // Assert: the x-coordinate snaps to the nearest multiple of 2. + expect((updated as CircleGraphState).radiusPoint).toEqual([-4, 0]); + }); + it("constrains to range", () => { const state: InteractiveGraphState = { ...baseCircleGraphState, 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 2fa038b9f5..aa18e2a00f 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 @@ -15,7 +15,7 @@ import { vector, } from "../../../util/geometry"; import {getQuadraticCoefficients} from "../graphs/quadratic"; -import {clamp, findAngle, polar, snap, X, Y} from "../math"; +import {clamp, clampToBox, findAngle, inset, polar, snap, X, Y} from "../math"; import {bound} from "../utils"; import {initializeGraphState} from "./initialize-graph-state"; @@ -41,10 +41,16 @@ import { } from "./interactive-graph-action"; import type {QuadraticCoords} from "../graphs/quadratic"; -import type {InteractiveGraphState, PairOfPoints} from "../types"; +import type { + AngleGraphState, + InteractiveGraphState, + PairOfPoints, +} from "../types"; import type {Coord} from "@khanacademy/perseus"; import type {Interval} from "mafs"; +const minDistanceBetweenAngleVertexAndSidePoint = 2; + export function interactiveGraphReducer( state: InteractiveGraphState, action: InteractiveGraphAction, @@ -244,28 +250,39 @@ function doMovePoint( case "angle": // If the index is 1, we are moving the vertex of the angle, // which will move the other two points as well - if (action.index === 1) { - const updatedCoords = boundAndSnapAngleVertex(state, action); - + const newState = (() => { + if (action.index === 1) { + const updatedCoords = boundAndSnapAngleVertex( + state, + action, + ); + + return { + ...state, + hasBeenInteractedWith: true, + coords: updatedCoords, + }; + } return { ...state, hasBeenInteractedWith: true, - coords: updatedCoords, + coords: setAtIndex({ + array: state.coords, + index: action.index, + newValue: boundAndSnapAngleEndPoints( + action.destination, + state, + action.index, + ), + }), }; + })(); + if (angleSidePointsTooCloseToVertex(newState)) { + // cancel the move + return state; } - return { - ...state, - hasBeenInteractedWith: true, - coords: setAtIndex({ - array: state.coords, - index: action.index, - newValue: boundAndSnapAngleEndPoints( - action.destination, - state, - action.index, - ), - }), - }; + return newState; + case "polygon": let newValue: vec.Vector2; if (state.snapTo === "sides") { @@ -375,11 +392,10 @@ function doMoveCenter( switch (state.type) { case "circle": { // Constrain the center of the circle to the chart range - const constrainedCenter: vec.Vector2 = bound({ - snapStep: state.snapStep, - range: state.range, - point: action.destination, - }); + const constrainedCenter = boundAndSnapToGrid( + action.destination, + state, + ); // Reposition the radius point based on the new center // (spread to make sure we're not going to mutate anything) @@ -425,12 +441,12 @@ function doMoveRadiusPoint( switch (state.type) { case "circle": { const [xMin, xMax] = state.range[X]; - const nextRadiusPoint: vec.Vector2 = [ + const nextRadiusPoint = snap(state.snapStep, [ // Constrain to graph range // The +0 is to convert -0 to +0 clamp(action.destination[X] + 0, xMin, xMax), state.center[1], - ]; + ]); if (_.isEqual(nextRadiusPoint, state.center)) { return state; @@ -551,17 +567,22 @@ function boundAndSnapAngleVertex( // Get the current and upcoming positions of the vertex const startingVertex = coordsCopy[1]; - const newVertex = bound({ - snapStep, - range, - point: snap(snapStep, destination), - }); + + // Prevent the vertex from getting too close to the edge, so that all + // points of the angle can fit within the graph bounds. + const insetAmount = vec.add(snapStep, [ + minDistanceBetweenAngleVertexAndSidePoint, + minDistanceBetweenAngleVertexAndSidePoint, + ]); + const newVertex = clampToBox( + inset(insetAmount, range), + snap(snapStep, destination), + ); // Get the vector from the starting vertex to the new vertex const delta = vec.add(newVertex, reverseVector(startingVertex)); // Apply the delta to each of the other two points so that the angle is maintained - let valid = true; const newPoints: Record = {}; for (const i of [0, 2]) { const oldPoint = coordsCopy[i]; @@ -572,21 +593,13 @@ function boundAndSnapAngleVertex( newPoint = constrainToBoundsOnAngle(newPoint, angle, range, snapStep); newPoints[i] = newPoint; - - // Check if the new point is too close to the vertex - if (tooClose(newVertex, newPoint, range)) { - valid = false; - } } // Update the vertex after snapping to the snapStep newPoints[1] = newVertex; - // Only move points if all new positions are valid - if (valid) { - Object.entries(newPoints).forEach(([i, newPoint]) => { - coordsCopy[i] = newPoint; - }); - } + Object.entries(newPoints).forEach(([i, newPoint]) => { + coordsCopy[i] = newPoint; + }); return coordsCopy; } @@ -697,12 +710,23 @@ function boundAndSnapAngleEndPoints( // Snap the angle to the nearest multiple of snapDegrees (if provided) angle = Math.round((angle - offsetDegrees) / snap) * snap + offsetDegrees; - const distance = vec.dist(coordsCopy[index], vertex); + + // add 0.01 to prevent rounding errors from causing the point to snap to + // a location that is too close to the vertex. + const minDistance = minDistanceBetweenAngleVertexAndSidePoint + 0.01; + const distance = Math.max(vec.dist(coordsCopy[index], vertex), minDistance); const snappedValue = vec.add(vertex, polar(distance, angle)); return snappedValue; } +function angleSidePointsTooCloseToVertex(state: AngleGraphState): boolean { + return ( + tooClose(state.coords[0], state.coords[1], state.range) || + tooClose(state.coords[2], state.coords[1], state.range) + ); +} + function boundAndSnapToPolygonAngle( destinationPoint: vec.Vector2, {