-
Notifications
You must be signed in to change notification settings - Fork 351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Copy the implementation of useMovable
into useDraggable
#1370
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
96662b8
Copy useMovable implementation into useDraggable
benchristel e94bd29
Add tests
benchristel de05f4e
Add changeset
benchristel 28a0e4e
Use a button element for TestDraggable
benchristel a123a0e
Add tests for dragging and constraints
benchristel 0dc02f4
Add test for userTransform
benchristel 652c5b8
Add test for keyboard movement
benchristel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
"@khanacademy/perseus": patch | ||
--- | ||
|
||
Internal: copy Mafs' implementation of useMovable into our own useDraggable hook. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
249 changes: 249 additions & 0 deletions
249
packages/perseus/src/widgets/interactive-graphs/graphs/use-draggable.test.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,249 @@ | ||
import {render, screen, fireEvent} from "@testing-library/react"; | ||
import {userEvent as userEventLib} from "@testing-library/user-event"; | ||
import {Mafs, Transform} from "mafs"; | ||
import * as React from "react"; | ||
import {useRef} from "react"; | ||
|
||
import {useDraggable} from "./use-draggable"; | ||
|
||
import type {vec, Interval} from "mafs"; | ||
|
||
function TestDraggable(props: { | ||
point: vec.Vector2; | ||
constrain?: (point: vec.Vector2) => vec.Vector2; | ||
onMove?: (point: vec.Vector2) => unknown; | ||
}) { | ||
const {onMove = () => {}, constrain = (p) => p} = props; | ||
const gestureTarget = useRef<HTMLButtonElement>(null); | ||
const {dragging} = useDraggable({ | ||
...props, | ||
onMove, | ||
constrain, | ||
gestureTarget, | ||
}); | ||
return ( | ||
<button ref={gestureTarget} tabIndex={0}> | ||
dragging: {String(dragging)} | ||
</button> | ||
); | ||
} | ||
|
||
describe("useDraggable", () => { | ||
let userEvent; | ||
beforeEach(() => { | ||
userEvent = userEventLib.setup({ | ||
advanceTimers: jest.advanceTimersByTime, | ||
}); | ||
}); | ||
|
||
it("initially returns {dragging: false}", () => { | ||
render( | ||
<Mafs width={200} height={200}> | ||
<TestDraggable point={[0, 0]} /> | ||
</Mafs>, | ||
); | ||
|
||
expect(screen.getByText("dragging: false")).toBeInTheDocument(); | ||
}); | ||
|
||
it("returns {dragging: true} when the mouse button is held down", async () => { | ||
render( | ||
<Mafs width={200} height={200}> | ||
<TestDraggable point={[0, 0]} /> | ||
</Mafs>, | ||
); | ||
const dragHandle = screen.getByRole("button"); | ||
|
||
// Act | ||
await userEvent.pointer({keys: "[MouseLeft>]", target: dragHandle}); | ||
|
||
// Assert | ||
expect(screen.getByText("dragging: true")).toBeInTheDocument(); | ||
}); | ||
|
||
it("returns {dragging: false} when the mouse button is released", async () => { | ||
render( | ||
<Mafs width={200} height={200}> | ||
<TestDraggable point={[0, 0]} /> | ||
</Mafs>, | ||
); | ||
const dragHandle = screen.getByRole("button"); | ||
|
||
// Act | ||
await userEvent.pointer([ | ||
{keys: "[MouseLeft>]", target: dragHandle}, | ||
{keys: "[/MouseLeft]"}, | ||
]); | ||
|
||
// Assert | ||
expect(screen.getByText("dragging: false")).toBeInTheDocument(); | ||
}); | ||
|
||
it("calls onMove with the destination point when the user drags", 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( | ||
<Mafs {...mafsProps}> | ||
<TestDraggable point={[0, 0]} onMove={onMoveSpy} /> | ||
</Mafs>, | ||
); | ||
const dragHandle = screen.getByRole("button"); | ||
|
||
// Act: click and hold the drag handle... | ||
mouseDownAt(dragHandle, 0, 0); | ||
// ...and then drag 10px right and 10px down | ||
moveMouseTo(dragHandle, 10, 10); | ||
|
||
// Assert: the draggable element was moved to (1, -1) | ||
expect(onMoveSpy).toHaveBeenCalledWith([1, -1]); | ||
}); | ||
|
||
it("constrains the destination point using the given constrain function", 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( | ||
<Mafs {...mafsProps}> | ||
<TestDraggable | ||
point={[0, 0]} | ||
onMove={onMoveSpy} | ||
constrain={(p) => [Math.round(p[0]), Math.round(p[1])]} | ||
/> | ||
</Mafs>, | ||
); | ||
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); | ||
|
||
// 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]); | ||
}); | ||
|
||
it("accounts for the user transform when measuring drag distance", async () => { | ||
// See: https://mafs.dev/guides/custom-components/contexts | ||
|
||
// 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( | ||
<Mafs {...mafsProps}> | ||
<Transform scale={2}> | ||
<TestDraggable point={[10, 10]} onMove={onMoveSpy} /> | ||
</Transform> | ||
</Mafs>, | ||
); | ||
const dragHandle = screen.getByRole("button"); | ||
|
||
// Act: click and hold the drag handle... | ||
mouseDownAt(dragHandle, 0, 0); | ||
// ...and then drag 10px right and 10px down. Because of the | ||
// <Transform scale={2}>, this movement actually represents the vector | ||
// [0.5, -0.5] in graph coordinates. | ||
moveMouseTo(dragHandle, 10, 10); | ||
|
||
// Assert: the draggable element moved to (10.5, 9.5). | ||
// If you see... | ||
// - (5.5, 4.5), the userTransform was not applied to the pickupPoint. | ||
// - (21, 19), the inverse user transform was not applied to the move. | ||
// - (11, 9), neither userTransform nor the inverse was applied. | ||
expect(onMoveSpy).toHaveBeenCalledWith([10.5, 9.5]); | ||
}); | ||
|
||
it("moves a draggable element with the keyboard", 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( | ||
<Mafs {...mafsProps}> | ||
<TestDraggable | ||
point={[0, 0]} | ||
onMove={onMoveSpy} | ||
constrain={(point) => [ | ||
Math.round(point[0]), | ||
Math.round(point[1]), | ||
]} | ||
/> | ||
</Mafs>, | ||
); | ||
// focus the draggable element | ||
await userEvent.tab(); | ||
await userEvent.tab(); | ||
|
||
// Pre-assert: the draggable element is actually focused | ||
expect(screen.getByRole("button")).toHaveFocus(); | ||
|
||
// Act: | ||
await userEvent.keyboard("{arrowright>1}{arrowup>1}"); | ||
|
||
// Assert: the element moved one step to the right and then one step up | ||
expect(onMoveSpy.mock.calls).toEqual([[[1, 0]], [[0, 1]]]); | ||
}); | ||
}); | ||
|
||
function mouseDownAt(element: Element, clientX: number, clientY: number) { | ||
// NOTE(benchristel): I could not figure out how to write these tests in | ||
// terms of userEvent. The tests for @use-gesture/react use fireEvent, so | ||
// I went with that approach. | ||
// eslint-disable-next-line testing-library/prefer-user-event | ||
fireEvent.mouseDown(element, { | ||
pointerId: 1, | ||
buttons: 1, | ||
clientX, | ||
clientY, | ||
}); | ||
} | ||
|
||
function moveMouseTo(element: Element, clientX: number, clientY: number) { | ||
// NOTE(benchristel): I could not figure out how to write these tests in | ||
// terms of userEvent. The tests for @use-gesture/react use fireEvent, so | ||
// I went with that approach. | ||
// eslint-disable-next-line testing-library/prefer-user-event | ||
fireEvent.mouseMove(element, { | ||
pointerId: 1, | ||
buttons: 1, | ||
clientX, | ||
clientY, | ||
}); | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests are unfortunately incomplete. I couldn't figure out how to get
@use-gesture/react
to register a drag gesture, either withuserEvent
orfireEvent
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: Is this Moving a Pointer section of the Testing Library docs useful? It seems like it is showing how to user userEvent.pointer to drag, though I could be reading it wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I tried what those docs suggest.
@use-gesture/react
doesn't see it as a drag, though.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update: I looked at the tests for
@use-gesture/react
itself and found that they're usingfireEvent
, so I came up with a testing solution based on that.