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

Add two fingers tap support #170

Conversation

sergey-plevako-badoo
Copy link

Add two fingers tap support from WDA

@@ -27,6 +27,8 @@ commands.performTouch = async function (gestures) {
return await this.handleTap(gestures[0]);
} else if (gestures.length === 1 && gestures[0].action === 'doubleTap') {
return await this.handleDoubleTap(gestures[0]);
} else if (gestures.length === 1 && gestures[0].action === 'twoFingerTap') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not exist as a thing in any of the clients.

Two finger tap would be a particular version of multi touch, so needs to be decoded within performMultiAction.

WebDriverAgent Outdated
@@ -1 +1 @@
Subproject commit 6813e2549b684379e1b6a6ee96deb2b42f4e1a85
Subproject commit a4c4f7ed24bb54a67d29719a57d397a5c90571af
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't update WDA like this. We update it in separate PRs so it is not buried.

@sergey-plevako-badoo sergey-plevako-badoo force-pushed the add_two_fingers_tap_support branch from bc5e1b1 to c80e61d Compare October 13, 2016 10:50
@jsf-clabot
Copy link

jsf-clabot commented Nov 2, 2016

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

@imurchie
Copy link
Contributor

imurchie commented Nov 3, 2016

This looks good to go! Thank you!

Two things:

  1. Have you signed the CLA?
  2. Can you squash all this into a single commit? Then we can get it in.

@jlipps
Copy link
Member

jlipps commented Nov 24, 2016

@sergey-plevako-badoo mind rebasing? looks like there's a conflict now

@sergey-plevako-badoo sergey-plevako-badoo force-pushed the add_two_fingers_tap_support branch from c3fa1f9 to 8924801 Compare December 14, 2016 15:30
@sergey-plevako-badoo sergey-plevako-badoo force-pushed the add_two_fingers_tap_support branch from 16641c3 to fc87872 Compare December 14, 2016 16:48
@@ -79,6 +82,13 @@ function isDoubleTap (gestures) {
return false;
}

function isTwoFingerTap (gestures) {
if (gestures.length === 1 && gestures[0].action.toLowerCase() === 'twofingertap') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sergey-plevako-badoo! Is twofingertap in the W3C gesture spec? I don't remember it being there.

If not, we can't just add new actions. I like this change in general but I think this function probably needs to be rewritten to actually detect a 2-digit tap based on the number of digits in the action chain, rather than creating a new action type.

I'm not up on the spec these days though so maybe I can be convinced otherwise. @imurchie should probably also comment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just saw some more changes come through here. @imurchie have you taken a look?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically nothing of this is in the W3C spec, so we could add it. Otherwise it could be mapped from a two tap gestures in the multi-action api.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, however given that when we do migrate to the new W3C spec, and it doesn't seem to have a "double-tap" primitive, probably better to do it the way that will be more easy to map to the spec later.

@jlipps
Copy link
Member

jlipps commented Jan 27, 2017

@sergey-plevako-badoo still waiting to hear back from you on the "twofingertap" + spec issue.

@mykola-mokhnach
Copy link
Contributor

Closing this PR as already implemented

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants