Skip to content

Bump appium java-client to 5.0.3 - #368

Open
mach6 wants to merge 1 commit into
paypal:developfrom
mach6:appium-java-client-update
Open

Bump appium java-client to 5.0.3#368
mach6 wants to merge 1 commit into
paypal:developfrom
mach6:appium-java-client-update

Conversation

@mach6

@mach6 mach6 commented Sep 28, 2017

Copy link
Copy Markdown
Contributor

Bump appium java-client to 5.0.3

  • Also updated the swipe and tap methods of SeLionAppiumAndroidDriver and
    SeLionAppiumIOSDriver to use TouchActions -- both methods were removed
    from appium java-client
  • Removed no-op rotation methods which now have implmentations in appium
    java-cient

- Also updated the swipe and tap methods of SeLionAppiumAndroidDriver and
  SeLionAppiumIOSDriver to use TouchActions  -- both methods were removed
  from appium java-client
- Removed no-op rotation methods which now have implmentations in appium
  java-cient
@mach6

mach6 commented Sep 28, 2017

Copy link
Copy Markdown
Contributor Author

@vikram1711 @elhuang can I also solicit your review of this change set?

@mach6
mach6 requested a review from sebady September 28, 2017 21:28
multiTouch.add(tap.press(x, y).waitAction(Duration.ofMillis(duration)).release());
}
multiTouch.perform();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can you please confirm if it is multi finger tap or multi tap action or combination of both?
Based on my observation it is behaving like multi finger multi tap action.

Above implementation completely depends on the duration parameter for example if I want to perform a double tap with duration 200ms, this will become two long press actions, but if I perform a double tap action with 30ms then that will be treated as double tap.

My suggestion would be not use raw actions rather use tap action exposed by TouchAction api-

Ex-

MultiTouchAction multiTouch = new MultiTouchAction(driver);
        for (int i = 0; i < fingers; i++) {
            TouchAction action = new TouchAction(driver);
            multiTouch.add(action.tap(element));
        }
        multiTouch.perform();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Observation : Most of the methods are expecting WebElement as an argument, so in test first you have to grab the WebElement instance and then only you can invoke these methods. Why can't we follow the same approach we have taken for HTML elements where we are invoking the findElement internally to grab the element reference. I am not sure if that would violate the design of the framework.

this.tap(1, starty, endx, OPERATION_DURATION_MILLI_SECONDS);
public void swipe(int startx, int starty, int endx, int endy, int duration) {
new TouchAction(this).press(startx, starty).waitAction(Duration.ofMillis(duration)).moveTo(endx, endy).release().perform();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

endx and endy - these are not absolute x,y value rather they represent the shift in x and y axis. So renaming them to shiftX and shiftY would make more sense

Also, consider a situation where you are swiping a carousel, most of the time you would be interested to go to a certain carousel content. To solve this problem if we can add a swipeCount argument then the author of the test can easily decide how many swipe is needed to go to the desired content, by default it will swipe only once.

.tap(webElement).release()
.perform();
new TouchAction(this).tap(webElement).release().waitAction(Duration.ofMillis(DOUBLE_TAP_WAIT_TIME))
.tap(webElement).release().perform();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I would recommend to use appium provided doubleTap for iOS -

 new IOSTouchAction(this).doubleTap(webElement).perform();


multiTouch.perform();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I still need to test this action before I can comment on it since I am not sure if it is multi finger or multi tap action.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I believe these are all multi finger actions.

@sebady sebady left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

@mach6 mach6 added this to the 2.0.0 milestone Nov 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants