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, {