-
Notifications
You must be signed in to change notification settings - Fork 80
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
feat(scroll-on-touch): use finger to scroll on canvas #104
base: main
Are you sure you want to change the base?
Changes from all commits
494aa59
74ddf26
4c7f381
006dda1
837a5e8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
.DS_Store | ||
node_modules | ||
.cache | ||
.idea | ||
dist | ||
# Cypress | ||
cypress/screenshots/ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
/* eslint-disable react/no-array-index-key */ | ||
import * as React from "react"; | ||
import { useCallback } from "react"; | ||
import { useCallback, useEffect } from "react"; | ||
import Paths, { SvgPath } from "../Paths"; | ||
import { CanvasPath, ExportImageType, Point } from "../types"; | ||
|
||
|
@@ -35,6 +35,7 @@ export interface CanvasProps { | |
paths: CanvasPath[]; | ||
isDrawing: boolean; | ||
onPointerDown: (point: Point, isEraser?: boolean) => void; | ||
scrollOnTouch: boolean; | ||
onPointerMove: (point: Point) => void; | ||
onPointerUp: () => void; | ||
className?: string; | ||
|
@@ -59,6 +60,7 @@ export const Canvas = React.forwardRef<CanvasRef, CanvasProps>((props, ref) => { | |
const { | ||
paths, | ||
isDrawing, | ||
scrollOnTouch, | ||
onPointerDown, | ||
onPointerMove, | ||
onPointerUp, | ||
|
@@ -79,6 +81,7 @@ export const Canvas = React.forwardRef<CanvasRef, CanvasProps>((props, ref) => { | |
} = props; | ||
|
||
const canvasRef = React.useRef<HTMLDivElement>(null); | ||
const currentPointerType = React.useRef<"pen" | "mouse" | "touch" | null>(null); | ||
|
||
// Converts mouse coordinates to relative coordinate based on the absolute position of svg | ||
const getCoordinates = useCallback( | ||
|
@@ -104,10 +107,20 @@ export const Canvas = React.forwardRef<CanvasRef, CanvasProps>((props, ref) => { | |
|
||
const handlePointerDown = useCallback( | ||
(event: React.PointerEvent<HTMLDivElement>): void => { | ||
// Allow only chosen pointer type | ||
currentPointerType.current = event.pointerType; | ||
|
||
if ( | ||
allowOnlyPointerType !== "all" && | ||
if (scrollOnTouch && event.pointerType === "touch") { | ||
return; | ||
} | ||
|
||
if (event.pointerType === 'pen') { | ||
event.preventDefault(); | ||
event.stopPropagation(); | ||
} | ||
|
||
// Allow only chosen pointer type | ||
if ( | ||
allowOnlyPointerType !== "all" && | ||
event.pointerType !== allowOnlyPointerType | ||
) { | ||
return; | ||
|
@@ -129,9 +142,13 @@ export const Canvas = React.forwardRef<CanvasRef, CanvasProps>((props, ref) => { | |
(event: React.PointerEvent<HTMLDivElement>): void => { | ||
if (!isDrawing) return; | ||
|
||
// Allow only chosen pointer type | ||
if ( | ||
allowOnlyPointerType !== "all" && | ||
if (scrollOnTouch && event.pointerType === "touch") { | ||
return; | ||
} | ||
|
||
// Allow only chosen pointer type | ||
if ( | ||
allowOnlyPointerType !== "all" && | ||
event.pointerType !== allowOnlyPointerType | ||
) { | ||
return; | ||
|
@@ -147,6 +164,9 @@ export const Canvas = React.forwardRef<CanvasRef, CanvasProps>((props, ref) => { | |
const handlePointerUp = useCallback( | ||
(event: React.PointerEvent<HTMLDivElement> | PointerEvent): void => { | ||
if (event.pointerType === "mouse" && event.button !== 0) return; | ||
if (scrollOnTouch && event.pointerType === "touch") { | ||
return; | ||
} | ||
|
||
// Allow only chosen pointer type | ||
if ( | ||
|
@@ -291,13 +311,31 @@ release drawing even when point goes out of canvas */ | |
); | ||
}, [paths]); | ||
|
||
// avoid pen from scrolling if scrollOnTouch | ||
useEffect(() => { | ||
const listener = function( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's better to move this listener as a callback using |
||
this: HTMLDivElement, | ||
event: TouchEvent | ||
): void { | ||
if (currentPointerType.current === "pen") { | ||
event.preventDefault(); | ||
event.stopPropagation(); | ||
} | ||
}; | ||
|
||
if (scrollOnTouch) { | ||
canvasRef.current?.addEventListener("touchstart", listener, { passive: false }); | ||
} | ||
return () => canvasRef.current?.removeEventListener("touchstart", listener); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
}, []); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As the dependency array is empty, if the developer changes scrollOnTouch dynamically, this function will fail. |
||
|
||
return ( | ||
<div | ||
role="presentation" | ||
ref={canvasRef} | ||
className={className} | ||
style={{ | ||
touchAction: "none", | ||
touchAction: scrollOnTouch ? 'pan-y' : 'none', | ||
width, | ||
height, | ||
...style, | ||
|
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.
Users must not be able to set
allowOnlyPointerType
as touch or all while they setscrollOnTouch
. ReactSketchCanvas must throw an error with some meaningful error message to avert users from running into this scenario. CanvasProps must also be updated to be conditional here.