Skip to content

Commit

Permalink
Create our own wrapper around Mafs' useMovable (#1367)
Browse files Browse the repository at this point in the history
## Summary:
We need the `useMovable` hook to do a couple things that Mafs doesn't
yet enable:

- let us handle keyboard events explicitly (needed for angle graphs, and
  maybe polygons if we decide to continue supporting side-length
  snapping). Currently, Mafs uses heuristics and search to convert
  keyboard input into movement vectors based on the provided `constrain`
  function, but this only works for grid-based snapping.

- get the movement vector in *pixel coordinates*. This is needed to
  implement the protractor rotation handle. The protractor is a
  fixed-size image that doesn't scale with the graph, so we really want
  to be working in pixels - otherwise, we have to do a bunch of
  confusing coordinate transformations.

We'll likely want to iterate on these features a bit before contributing
them back to Mafs. Therefore, this commit creates a wrapper around
`useMovable` where we can add our new functionality.

I have named the wrapper `useDraggable` instead of `useMovable`
so people don't mistake it for the Mafs hook.

Issue: none

Test plan:

- Run `yarn dev` and view the graphs at `http://localhost:5173/`.
- Turn on the Mafs flags for all graph types.
- Verify that you can move all the interactive graph elements with
  mouse and keyboard.

Author: benchristel

Reviewers: SonicScrewdriver, handeyeco, mark-fitzgerald, Myranae, nishasy

Required Reviewers:

Approved By: SonicScrewdriver

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

Pull Request URL: #1367
  • Loading branch information
benchristel committed Jun 21, 2024
1 parent 86f94e1 commit e5a54d8
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 41 deletions.
5 changes: 5 additions & 0 deletions .changeset/shaggy-moles-promise.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Internal: wrap the Mafs `useMovable` hook, creating a seam where we can add new functionality.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {useMovable, vec} from "mafs";
import {vec} from "mafs";
import * as React from "react";
import {useRef} from "react";

Expand All @@ -8,6 +8,7 @@ import useGraphConfig from "../reducer/use-graph-config";
import {snap} from "../utils";

import {StyledMovablePoint} from "./components/movable-point";
import {useDraggable} from "./use-draggable";
import {
useTransformDimensionsToPixels,
useTransformVectorsToPixels,
Expand Down Expand Up @@ -49,7 +50,7 @@ function MovableCircle(props: {

const draggableRef = useRef<SVGGElement>(null);

const {dragging} = useMovable({
const {dragging} = useDraggable({
gestureTarget: draggableRef,
point: center,
onMove,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,13 @@
import {render} from "@testing-library/react";
import * as MafsLibrary from "mafs";
import {Mafs} from "mafs";
import React from "react";

import * as UseDraggableModule from "../use-draggable";

import {MovableLine, trimRange} from "./movable-line";

import type {Interval, vec} from "mafs";

jest.mock("mafs", () => {
const originalModule = jest.requireActual("mafs");
return {
__esModule: true,
...originalModule,
useMovable: jest.fn(),
};
});

describe("trimRange", () => {
it("does not trim smaller than [[0, 0], [0, 0]]", () => {
const graphDimensionsInPixels: vec.Vector2 = [1, 1];
Expand Down Expand Up @@ -78,12 +71,11 @@ describe("trimRange", () => {
});

describe("Rendering", () => {
let useMovableMock: jest.SpyInstance;
const Mafs = MafsLibrary.Mafs;
let useDraggable: jest.SpyInstance;

beforeEach(() => {
useMovableMock = jest
.spyOn(MafsLibrary, "useMovable")
useDraggable = jest
.spyOn(UseDraggableModule, "useDraggable")
.mockReturnValue({dragging: false});
});

Expand Down Expand Up @@ -159,7 +151,7 @@ describe("Rendering", () => {
expect(line?.classList).not.toContain("movable-dragging");

// Verify dragging state
useMovableMock.mockReturnValue({dragging: true});
useDraggable.mockReturnValue({dragging: true});
container = render(
<Mafs width={200} height={200}>
<MovableLine
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import {useMovable, vec} from "mafs";
import {vec} from "mafs";
import {useRef, useState} from "react";
import * as React from "react";

import useGraphConfig from "../../reducer/use-graph-config";
import {snap, TARGET_SIZE} from "../../utils";
import {useDraggable} from "../use-draggable";
import {useTransformVectorsToPixels} from "../use-transform";
import {getIntersectionOfRayWithBox} from "../utils";

Expand Down Expand Up @@ -79,15 +80,15 @@ function useControlPoint(
const {snapStep} = useGraphConfig();
const [focused, setFocused] = useState(false);
const keyboardHandleRef = useRef<SVGGElement>(null);
useMovable({
useDraggable({
gestureTarget: keyboardHandleRef,
point,
onMove: onMovePoint,
constrain: (p) => snap(snapStep, p),
});

const visiblePointRef = useRef<SVGGElement>(null);
const {dragging} = useMovable({
const {dragging} = useDraggable({
gestureTarget: visiblePointRef,
point,
onMove: onMovePoint,
Expand Down Expand Up @@ -156,7 +157,7 @@ const Line = (props: LineProps) => {
}

const line = useRef<SVGGElement>(null);
const {dragging} = useMovable({
const {dragging} = useDraggable({
gestureTarget: line,
point: start,
onMove: (newPoint) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,13 @@
import Tooltip from "@khanacademy/wonder-blocks-tooltip";
import {render} from "@testing-library/react";
import * as MafsLibrary from "mafs";
import {Mafs} from "mafs";
import React from "react";

import * as ReducerGraphConfig from "../../reducer/use-graph-config";
import * as UseDraggableModule from "../use-draggable";

import {StyledMovablePoint} from "./movable-point";

jest.mock("mafs", () => {
const originalModule = jest.requireActual("mafs");
return {
__esModule: true,
...originalModule,
useMovable: jest.fn(),
};
});

jest.mock("@khanacademy/wonder-blocks-tooltip", () => {
const originalModule = jest.requireActual(
"@khanacademy/wonder-blocks-tooltip",
Expand All @@ -33,8 +25,7 @@ const TooltipMock = ({children}) => {

describe("StyledMovablePoint", () => {
let useGraphConfigMock: jest.SpyInstance;
let useMovableMock: jest.SpyInstance;
const Mafs = MafsLibrary.Mafs;
let useDraggableMock: jest.SpyInstance;
const baseGraphConfigContext = {
snapStep: 1,
range: [
Expand All @@ -47,8 +38,8 @@ describe("StyledMovablePoint", () => {

beforeEach(() => {
useGraphConfigMock = jest.spyOn(ReducerGraphConfig, "default");
useMovableMock = jest
.spyOn(MafsLibrary, "useMovable")
useDraggableMock = jest
.spyOn(UseDraggableModule, "useDraggable")
.mockReturnValue({dragging: false});
});

Expand Down Expand Up @@ -142,7 +133,7 @@ describe("StyledMovablePoint", () => {
describe("Hairlines", () => {
it("Shows hairlines when dragging and 'markings' are NOT set to 'none'", () => {
useGraphConfigMock.mockReturnValue(baseGraphConfigContext);
useMovableMock.mockReturnValue({dragging: true});
useDraggableMock.mockReturnValue({dragging: true});
const {container} = render(
<Mafs width={200} height={200}>
<StyledMovablePoint point={[0, 0]} onMove={() => {}} />,
Expand Down Expand Up @@ -174,7 +165,7 @@ describe("StyledMovablePoint", () => {
const graphStateContext = {...baseGraphConfigContext};
graphStateContext.markings = "none";
useGraphConfigMock.mockReturnValue(graphStateContext);
useMovableMock.mockReturnValue({dragging: true});
useDraggableMock.mockReturnValue({dragging: true});
const {container} = render(
<Mafs width={200} height={200}>
<StyledMovablePoint point={[0, 0]} onMove={() => {}} />,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import {color as WBColor} from "@khanacademy/wonder-blocks-tokens";
import {useMovable} from "mafs";
import * as React from "react";
import {useRef} from "react";

import useGraphConfig from "../../reducer/use-graph-config";
import {snap} from "../../utils";
import {useDraggable} from "../use-draggable";

import {MovablePointView} from "./movable-point-view";

Expand All @@ -24,7 +24,7 @@ export const StyledMovablePoint = (props: Props) => {
const elementRef = useRef<SVGGElement>(null);
const {point, onMove, cursor, color = WBColor.blue, snapTo} = props;
const snapToValue = snapTo ?? "grid";
const {dragging} = useMovable({
const {dragging} = useDraggable({
gestureTarget: elementRef,
point,
onMove,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Polygon, useMovable, vec} from "mafs";
import {Polygon, vec} from "mafs";
import * as React from "react";

import {moveAll, movePoint} from "../reducer/interactive-graph-action";
Expand All @@ -7,6 +7,7 @@ import {TARGET_SIZE, snap} from "../utils";
import {Angle} from "./components/angle";
import {StyledMovablePoint} from "./components/movable-point";
import {TextLabel} from "./components/text-label";
import {useDraggable} from "./use-draggable";
import {getLines} from "./utils";

import type {MafsGraphProps, PolygonGraphState} from "../types";
Expand All @@ -28,7 +29,7 @@ export const PolygonGraph = (props: Props) => {
const ref = React.useRef<SVGPolygonElement>(null);
const dragReferencePoint = points[0];
const snapToValue = snapTo ?? "grid";
const {dragging} = useMovable({
const {dragging} = useDraggable({
gestureTarget: ref,
point: dragReferencePoint,
onMove: (newPoint) => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import {useMovable} from "mafs";

import type {vec} from "mafs";
import type {RefObject} from "react";

type Params = {
gestureTarget: RefObject<Element>;
onMove: (point: vec.Vector2) => unknown;
point: vec.Vector2;
constrain: (point: vec.Vector2) => vec.Vector2;
};

type DragState = {
dragging: boolean;
};

export function useDraggable(params: Params): DragState {
return useMovable(params);
}

0 comments on commit e5a54d8

Please sign in to comment.