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.

Sign up for GitHub

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

Jump to bottom

DoubleTap Support #444

Closed
wants to merge 1 commit into from
Closed

DoubleTap Support #444

wants to merge 1 commit into from

Conversation

rafe-g
Copy link
Contributor

@rafe-g rafe-g commented Jul 27, 2016

Change list

DoubleTap support to TouchAction

Types of changes

What types of changes are you proposing/introducing to Java client?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

@rafe-g
Copy link
Contributor Author

rafe-g commented Jul 27, 2016

@TikhomirovSergey @SrinivasanTarget

@SrinivasanTarget
Copy link
Member

Awesome. Can you add some tests please? Will review soon.

@TikhomirovSergey
Copy link
Contributor

TikhomirovSergey commented Jul 27, 2016 •
edited
Loading

tests
+1

@Rafael-Chavez
Also
could you provide tests for both Android and iOS?

@rafe-g rafe-g force-pushed the doubleTap branch 2 times, most recently from 344b6ed to b4d9b6b Compare July 27, 2016 23:13
rafe-g
rafe-g reviewed Jul 27, 2016
View reviewed changes
src/test/java/io/appium/java_client/ios/IOSGesturesTest.java
firstField.sendKeys("2");
driver.doubleTap(firstField);
IOSElement editingMenu = (IOSElement) driver.findElementByClassName("UIAEditingMenu");
assertNotNull(editingMenu);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

there was little to double tap on in ios TestApp. But found the editing menu shows up when you double tap on one of the fields.

@rafe-g
Copy link
Contributor Author

rafe-g commented Jul 28, 2016 •
edited
Loading

added android test as well @TikhomirovSergey @SrinivasanTarget

rafe-g
rafe-g reviewed Jul 28, 2016
View reviewed changes
src/test/java/io/appium/java_client/android/AndroidGestureTest.java
@Test public void doubleTapTest() throws Exception {
driver.startActivity("io.appium.android.apis", ".view.TextSwitcher1");
String textBeforeDoubleTap = (driver.findElementByClassName("android.widget.TextView")).getText();
AndroidElement next = driver.findElementById("io.appium.android.apis:id/next");
Copy link
Contributor Author

@rafe-g rafe-g Jul 28, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

next is used

@SrinivasanTarget
Copy link
Member

@Rafael-Chavez Looks good but some remarks,

1)Correct java doc here: https://github.com/appium/java-client/pull/444/files#diff-970f723bf2aa8111d5817a3de004cc13R208 & https://github.com/appium/java-client/pull/444/files#diff-6f4ca47e6c9b3f6056a3d5f88a6d7821R65
2)please change the java doc here: https://github.com/appium/java-client/pull/444/files#diff-c60f84c0cc90f37a9a4513eeebd4896dR400 like this @see TouchShortcuts#doubleTap(WebElement, int, int). Here too https://github.com/appium/java-client/pull/444/files#diff-c60f84c0cc90f37a9a4513eeebd4896dR408.
3)Few Checkstyle errors: https://gist.github.com/SrinivasanTarget/d44880040b29b67ff8861fca1409be74

@rafe-g
Copy link
Contributor Author

rafe-g commented Jul 28, 2016

i will update with the javadoc and style fixes.

@rafe-g
Copy link
Contributor Author

rafe-g commented Jul 28, 2016

Fixed java doc and codestyle @SrinivasanTarget !

@SrinivasanTarget SrinivasanTarget added this to the 4.1.0 milestone Jul 29, 2016
@SrinivasanTarget
Copy link
Member

@Rafael-Chavez Does the tests passed? I see server errors: https://gist.github.com/SrinivasanTarget/1a5683fc5523bd50b74a98884cf484b3

@rafe-g
Copy link
Contributor Author

rafe-g commented Jul 29, 2016 •
edited
Loading

that error makes sense @SrinivasanTarget as i only added doubleTap support for iOS driver in appium. I dont see support for double tap (or double click) in uiautomator.

@SrinivasanTarget
Copy link
Member

SrinivasanTarget commented Jul 29, 2016 •
edited
Loading

@Rafael-Chavez Yes i see that there is no support for doubleTap in appium-android-driver. I would suggest to move this to IOSDriver.java instead of AppiumDriver.java as there is no support. What do you say @TikhomirovSergey ? Or we should throw appropriate NotYetImplemented error.

@rafe-g
Copy link
Contributor Author

rafe-g commented Jul 29, 2016

Im ok with either of these two suggestions @SrinivasanTarget @TikhomirovSergey

@SrinivasanTarget SrinivasanTarget removed this from the 4.1.0 milestone Jul 31, 2016
@rafe-g
Copy link
Contributor Author

rafe-g commented Aug 2, 2016

Im moving this to IOSDriver if thats ok with you @SrinivasanTarget

@SrinivasanTarget SrinivasanTarget added this to the 5.0.0 milestone Aug 26, 2016
@rafe-g rafe-g force-pushed the doubleTap branch 4 times, most recently from 63677a2 to 59cfb95 Compare August 30, 2016 23:39
@rafe-g
Copy link
Contributor Author

rafe-g commented Aug 30, 2016

@SrinivasanTarget @TikhomirovSergey moved to IOSDriver and removed Android test added.

@rafe-g
Copy link
Contributor Author

rafe-g commented Sep 2, 2016

ping @SrinivasanTarget @TikhomirovSergey

@SrinivasanTarget
Copy link
Member

@Rafael-Chavez Apologies for the delay. I'm on vacation till 12th Sep. We were working on splitting TouchActions as you can see here, #458. We will get this in soon. @TikhomirovSergey FYI

@SrinivasanTarget SrinivasanTarget mentioned this pull request Nov 24, 2016
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rafe-g @SrinivasanTarget @TikhomirovSergey