Conversation
|
|
||
| let touchX: number; | ||
| let touchY: number; | ||
| let touchTimestamp: number; |
There was a problem hiding this comment.
These should be class properties rather than global as they are now.
| private handleTouchEnd(event: TouchEvent) { | ||
| if (Date.now() - touchTimestamp < touchThresholdMilliseconds) { | ||
| /* a short touch, i.e., a click */ | ||
| if ((event.detail as any).element) { |
There was a problem hiding this comment.
This check is redundant here and also in the source it was copied from (handleClick). It can be removed from there too. It was originally a catch for when this class was an element itself, which is no longer the case.
| } | ||
| let x = 0; | ||
| let y = 0; | ||
| if (!document.pointerLockElement) { |
There was a problem hiding this comment.
If this is a touch handler then it seems to follow that we should always be using the position of the touch rather than check if there was a pointerLockElement.
It's plausible that a device with both mouse and touchscreen could be using pointer lock controls and then have the user touch the screen to "click" something, but in that instance I'd expect the touch location to still be used.
| x = (offsetX / width) * 2 - 1; | ||
| y = -((offsetY / height) * 2 - 1); | ||
| } | ||
| this.raycaster.setFromCamera(new THREE.Vector2(x, y), this.scene.getCamera()); |
There was a problem hiding this comment.
From this point in this method this is duplicated from handleClick. It's likely best to extract this into a private method to avoid them diverging unintentionally.
| private handleTouchStart(event: TouchEvent) { | ||
| /* remember the x and y position of the touch, so that it can be used in touchEnd */ | ||
| touchX = event.touches[0].clientX; | ||
| touchY = event.touches[0].clientY; |
There was a problem hiding this comment.
Using the first-available touch to track position here (rather than using the position of the touch that has ended in touchEnd) has an unintentional side-effect.
If you hold a finger (A) on the screen, and then tap with a second finger (B), you will be "clicking" at point A rather than B which is where the actual tapping is happening.
| if (Date.now() - this.clickTimestamp < this.clickThresholdMilliseconds) { | ||
| /* this is a click */ | ||
| // Create and dispatch a new mouse event with specific x and y coordinates | ||
| const clickEvent = new MouseEvent("click", { |
There was a problem hiding this comment.
This seems extraneous now as the MMLClickTrigger handles clicks without this logic.
* Added support for zoom (pinch in and out) * Added support for pan (drag left and right)
…not get mistaken as a click
…ioning from zoom to pan does not flick the view
…he whole document, moved preventdefault inside handleTouchStart so that touch events outside document still behave as normal
MarcusLongmuir
force-pushed
the
Standalone-client-mobile-support
branch
from
ce40b9f to
7329c6f
Compare
2 participants
Resolves #53
What kind of changes does your PR introduce? (check at least one)
Does your PR introduce a breaking change? (check one)
If yes, please describe its impact and migration path for existing applications:
Does your PR fulfill the following requirements?