feat(RenderWindowInteractor): add Tap and LongTap gestures - #3596
feat(RenderWindowInteractor): add Tap and LongTap gestures#3596UlysseDurand wants to merge 1 commit into
Conversation
20dfa20 to
af35a27
Compare
UlysseDurand
changed the title
af35a27 to
1cd09c4
Compare
UlysseDurand
changed the title
1cd09c4 to
485f201
Compare
| 'EndInteraction', | ||
| 'AnimationFrameRateUpdate', | ||
| 'Tap', | ||
| 'LongTap', |
There was a problem hiding this comment.
please add them after "Pan" events
| 'EndInteraction', | ||
| 'AnimationFrameRateUpdate', | ||
| 'Tap', | ||
| 'LongTap', |
| } | ||
|
|
||
| function distanceBetweenPositions(a, b) { | ||
| return Math.sqrt((a.x - b.x) * (a.x - b.x) + (a.y - b.y) * (a.y - b.y)); |
There was a problem hiding this comment.
don't compute sqrt, it is expensive: distanceBetweenPositions --> distance2BetweenPositions.
It can be a separate commit (because you would need to consider the pinchDistance, rotateDistance and panDistance as well)
| let tapGestureActive = false; | ||
|
|
||
| function cancelTapGesture() { | ||
| if (longTapTimer !== null) { |
There was a problem hiding this comment.
longTapTimer != null to support longTapTimer=undefined
|
|
||
| function startLongTapTimer() { | ||
| if (longTapTimer !== null) { | ||
| clearTimeout(longTapTimer); |
| } | ||
| tapGestureActive = false; | ||
| tapPointerId = null; | ||
| if (longTapFired) { |
There was a problem hiding this comment.
if you delete tapObject when you fire a long tap, you wouldn't have to keep the information of "fired" or not.
| * Enable/Disable recognition of tap and long-press gestures. | ||
| * @param recognizeTapGestures | ||
| */ | ||
| setRecognizeTapGestures(recognizeTapGestures: boolean): boolean; |
There was a problem hiding this comment.
why do you want to add Tap gesture recognition granularity ?
Or said differently, by recognizing tap gestures, do you change the behavior of regular gestures ? do you decrease performance ?
| longTapTimer = null; | ||
| } | ||
| tapPointerId = null; | ||
| tapStartPosition = null; |
There was a problem hiding this comment.
can't you reuse startingEventPositions ?
| return positions; | ||
| } | ||
|
|
||
| function distanceBetweenPositions(a, b) { |
There was a problem hiding this comment.
you can take the opportunity in the file to call this function where the math is done manually (e.g. pinchDistance, rotateDistance and panDistance) . It can be a separate commit.
| publicAPI.handleTouchMove = (event) => { | ||
| const pointers = [...pointerCache.values()]; | ||
| if (model.recognizeGestures && pointers.length > 1) { | ||
| cancelTapGesture(); |
There was a problem hiding this comment.
should probably have been already cancelled when a second pointer was made down.
|
Can you consider writing a test ? |
2 participants
Context
There is a need for mobile web applications made with vtk-js to have more control interactions, especially because right click isn't available. The LongTap event can replace the right click, Tap comes along the way.
Results
TapEventandLongTapevents through the standardhandledEventsmechanism, alongside the existing press/move/release events.Changes
Sources/Rendering/Core/RenderWindowInteractor/index.jsTap, andLongTaptohandledEvents.Sources/Rendering/Core/RenderWindowInteractor/index.d.tshandledEventsenum,invokeTap,invokeLongTapand the correspondingon*/*Eventtypes, plus the new initial values/getters.Examples/Applications/TapGestureDemo/index.jsrecognizeTapGestures(true)longTapDuration(500 ms)longTapDistance(30 px)PR and Code Checklist
npm run reformatto have correctly formatted codeTesting
TapGestureDemoexample and tap and long-tap with a touch device or pen.