Skip to content
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

Pinch to zoom not working anymore #6508

Closed
clementcontet opened this issue Oct 9, 2022 · 3 comments · Fixed by #6544
Closed

Pinch to zoom not working anymore #6508

clementcontet opened this issue Oct 9, 2022 · 3 comments · Fixed by #6544
Assignees
Labels
component: touch issue: bug Describes why the code or behaviour is wrong

Comments

@clementcontet
Copy link
Contributor

Describe the bug

Since the migration from Blockly 6.2 to Blockly 9, the pinch to zoom doesn't work for me on touch devices.

To Reproduce

Visit https://www.supercodingball.com/code/offline/do-nothing on a touch device (or clone and run https://github.com/Orange-OpenSource/super-coding-ball on a touch device).

Expected behavior

I may do something wrong but I can't figure out what.

The pinch setting is still there: https://github.com/Orange-OpenSource/super-coding-ball/blob/d86a9d3dd2bdf4830518bac607a0e347ff380e86/src/app/components/blockly/blockly.component.ts#L83

Smartphone (please complete the following information):

  • Chrome Android
@clementcontet clementcontet added the issue: triage Issues awaiting triage by a Blockly team member label Oct 9, 2022
@clementcontet
Copy link
Contributor Author

In console, I get the following warning: 'Tried to start the same gesture twice.'
I've tried https://blockly-demo.appspot.com/static/demos/code/index.html on a mobile device and I get the same error.

@BeksOmega BeksOmega added issue: bug Describes why the code or behaviour is wrong component: touch and removed issue: triage Issues awaiting triage by a Blockly team member labels Oct 12, 2022
@BeksOmega BeksOmega assigned BeksOmega and unassigned BeksOmega Oct 12, 2022
@BeksOmega BeksOmega added this to the Bug Bash Backlog milestone Oct 12, 2022
@maribethb
Copy link
Contributor

maribethb commented Oct 13, 2022

I've done a git bisect and #6099 is the culprit, still working on root causing. Using BrowserStack Local for testing and it's slow going. Using the debugger doesn't work all that well because the gesture changes when the debugger is in play (e.g. it thinks I'm long pressing instead of pinching)

What I've found so far is that in blockly-v8.0.5, in browserEvents.conditionalBind the wrapped function (in this case, workspaceSvg.onMouseDown_) is called twice, each time with one event in the array returned by Touch.splitEventByTouches. But the second time the wrapped function is called, Touch.shouldHandleEvent returns false, so we continue out of the loop and the event handler is not actually called a second time.

On develop, the wrapped function is called twice, each time with one event in the array. But this time, Touch.shouldHandleEvent returns true both times, causing the event handler to get called twice, causing the "tried to start the same gesture twice" message.

I don't yet know why Touch.shouldHandleEvent behaves differently, since that function nor the two it calls seem changed by #6099 at first glance, but this will be investigation for tomorrow.

@maribethb
Copy link
Contributor

maribethb commented Oct 13, 2022

root cause found:

blockly/core/touch.ts

Lines 167 to 173 in 1162a66

if (e instanceof MouseEvent) {
return 'mouse';
}
if (e instanceof PointerEvent) {
return String(e.pointerId);
}

PointerEvent is a subclass of MouseEvent, so the touch identifier is always set to 'mouse' if pointer events are supported. This causes shouldHandleEvent to always think we are talking about the same touch point, and says we should always handle the touch event.

PR incoming (after more testing is done)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: touch issue: bug Describes why the code or behaviour is wrong
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants