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

Allow touchmode-interface for Microsoft Surface tablets #613

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
3 participants
@samhed
Copy link
Member

commented May 13, 2016

We should use navigator.maxTouchPoints since 'ontouchstart' isn't enough to detect touch on MS Surface devices. This had the effect that the touch controls in the UI didn't show on these devices.

Allow touchmode for Microsoft Surface tablets
'ontouchstart' wasn't enough to detect touch on MS Surface devices

@samhed samhed added the bug label May 13, 2016

@zarmhast

This comment has been minimized.

Copy link
Contributor

commented Jun 5, 2016

@samhed,
This solution though will display the touch related buttons in the toolbar it will not enable proper touch interactivity in MS Edge.
I've tested master branch on a touch capable laptop with Windows 10 and MS Edge. It all worked once I changed the default setting to allow touch events, then the buttons appeared on their own.

Navigate to about:flags in MS Edge in order to change the default "Always off" to anything else. Once that is done master works with touch events without a problem.

The only issue on master right now is that once that is done, then mouse events are disabled. One of the fixes for that can be #619 (which, this time, was properly tested on touch devices, including an android tablet for clicking and dragging).

P.S.: the screenshot:
msedge

Thank you.

@samhed

This comment has been minimized.

Copy link
Member Author

commented Jun 6, 2016

@samhed,
This solution though will display the touch related buttons in the toolbar it will not enable proper touch interactivity in MS Edge.

Yeah, I'm quite aware that this would only solve part of the problem. The fact still stands that the UI-detection for touch is incomplete. I believe that we should display the touch controls in the UI for any touch device, even if the device has access to an ordinary mouse and keyboard.

I've tested master branch on a touch capable laptop with Windows 10 and MS Edge. It all worked once I changed the default setting to allow touch events, then the buttons appeared on their own.

Navigate to about:flags in MS Edge in order to change the default "Always off" to anything else. Once that is done master works with touch events without a problem.

Expecting the users to change settings is not a good "solution" in my mind.

The only issue on master right now is that once that is done, then mouse events are disabled. One of the fixes for that can be #619 (which, this time, was properly tested on touch devices, including an android tablet for clicking and dragging).

In order to properly support noVNC on MS Surface tablets for instance, we would need both the changes from this PR and the changes from your #619

@zarmhast

This comment has been minimized.

Copy link
Contributor

commented Jun 6, 2016

Expecting the users to change settings is not a good "solution" in my mind.

Agree, but without that option change MS Edge pretends to not have a touchscreen. Meaning that even though the touch interface will be displayed, it will have no effect since only "mouse" messages will be coming through.

I have taken out the whole condition for touch events locally and put in a console.log statement in the mousebuttonhandle. With the option at its default setting, even though touchstart was registered, only MouseEvents where received.

So in the end it is not a problem with the code as much as with the browser. Btw, there is the same thing with Firefox on desktops (both mobile ie and firefox work fine with touch).

Maybe include a warning: we've detected that your device has a touchscreen but its support is disabled for this browser, click here for instructions how to enable touch support.

Or something like that. What do you think?

@zarmhast

This comment has been minimized.

Copy link
Contributor

commented Jun 6, 2016

Actually it is even worse than I thought: with touchscreen support disabled, attempting to double tap engages some weird zoom mode that does not display as zoom in the menu, but still makes elements bigger and messes with the coordinate translation in the Mouse object.

In case you do not have a windows touch device we could set up a call and I'll share my screen with you to show you what is going on, if you'd like. Having more experience with noVNC, you are more likely to understand what is going on.

P.S.: a few double taps later the canvas area disappeared, this is what I saw in the dom explorer:
<canvas width="-676" height="462" id="noVNC_canvas" style="width: 144px; height: 462px; cursor: none;">Canvas not supported.</canvas>

@samhed

This comment has been minimized.

Copy link
Member Author

commented Jun 7, 2016

You are confusing me here.. Didn't you say that #619 is a fix for sending touch events correctly, even on MS Edge?

#619 only concerns mouse events sent to the server. This PR only concerns user interface. The goal should be to fix both these parts.

The functions that are specific to touch devices in the UI are:

  • Forced disabled local cursor setting
  • Forced clipping mode setting
  • Mouse selector button
  • Panning button
  • Virtual keyboard button
  • Extra key buttons (Ctrl, Alt, Tab, Esc)
@zarmhast

This comment has been minimized.

Copy link
Contributor

commented Jun 8, 2016

You are confusing me here.. Didn't you say that #619 is a fix for sending touch events correctly, even on MS Edge?

I might've claimed something like that in one of my edited messages, but that was before i read up more about touch events, browser support and have actually tried on an edge browser. My tests were more concerned with testing on a variety of devices than a variety of browsers and hence did not notice that edge does not process touches. (I don't have Firefox installed on my touch device, btw.)

#619 only concerns mouse events sent to the server.

PR #619 is concerned with mouse events being captured, proceeded on client side without interfering with touch event handling. So that dual input devices can use touch and mouse if touch is supported by both the browser and the device. In other words: fix mouse control without screwing touch.

@samhed, sorry for confusing you, hope that this reply will introduce more clarity. Also, please let me know if my awkward attempts to be helpful are not helping.

@samhed

This comment has been minimized.

Copy link
Member Author

commented Jun 8, 2016

Seeing as this PR will still be useful alongside #619 and that this discussion mostly regards #619 we should move the discussion to that PR.

@DirectXMan12 DirectXMan12 added this to the v0.7.0 milestone Sep 17, 2016

@samhed samhed referenced this pull request Oct 14, 2016

Merged

New way of detecting touch #650

@samhed

This comment has been minimized.

Copy link
Member Author

commented Oct 14, 2016

Closing in favor for #650

@samhed samhed closed this Oct 14, 2016

@samhed samhed deleted the surfacetouch branch Oct 14, 2016

samhed added a commit that referenced this pull request Oct 19, 2016

Disable forced touch gestures on IE and Edge
Fixes issue discussed in #613
@samhed

This comment has been minimized.

Copy link
Member Author

commented Oct 19, 2016

Actually it is even worse than I thought: with touchscreen support disabled, attempting to double tap engages some weird zoom mode that does not display as zoom in the menu, but still makes elements bigger and messes with the coordinate translation in the Mouse object.

In case you do not have a windows touch device we could set up a call and I'll share my screen with you to show you what is going on, if you'd like. Having more experience with noVNC, you are more likely to understand what is going on.

P.S.: a few double taps later the canvas area disappeared, this is what I saw in the dom explorer:
Canvas not supported.

Fixed in d6cb04a

@samhed samhed removed this from the v0.7.0 milestone Oct 21, 2016

SnifferNandez pushed a commit to SnifferNandez/noVNC that referenced this pull request Dec 16, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.