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

Touch API #1289

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Touch API #1289

wants to merge 6 commits into from

Conversation

twalpole
Copy link
Member

@twalpole twalpole commented Apr 8, 2014

Touch API to get this moving forward

@carhartl
Copy link

carhartl commented Apr 8, 2014

I'd like to suggest additional swipe methods: swipe_left, swipe_right, swipe_up, swipe_down or maybe as swipe(:left), swipe(:down) etc.

end

def long_press
#driver.browser.touch.long_press(native).perform
Copy link
Collaborator

Choose a reason for hiding this comment

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

It should be uncommented. I think you commented it because Android driver doesn't support long_press currenly (https://github.com/appium/appium/blob/463e692c7bb7c38dc1a95319aaa6c95ba986bed7/lib/devices/android/android-controller.js#L1042)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it's commented because it didn't work, so I manually did the long press with a delay

@abotalov
Copy link
Collaborator

abotalov commented Apr 8, 2014

  1. It seems Sauce Labs is able to provide a free account (https://saucelabs.com/opensauce, choose Open Sauce at sign up form) so we can use it until Travis CI will provide support for Android (Android as first class citizen travis-ci/travis-ci#1395)
  2. I'm OK with providing only methods that you implemented in the scope of this PR as it will be always possible to add optional method arguments/additional methods (e.g. x, y params for single_tap). Eventually Capybara can provide wrappers for all mobile parts of Webdriver W3C spec but I'm fine with merging only this part for now.

@twalpole
Copy link
Member Author

twalpole commented Apr 8, 2014

@carhartl yeah, the question there is just how far (in px) is a 'swipe' -- the flick currently is just set to a fast 200px swipe right, not really sure what makes sense for defaults on something like that. Should swipe replace flick, or is a flick a fast swipe and should remain (so we would have swipe(:left) and flick(:left) ), etc?

@abotalov
Copy link
Collaborator

abotalov commented Apr 8, 2014

What is the difference between flick and swipe?

@abotalov
Copy link
Collaborator

abotalov commented Apr 8, 2014

I've just filed https://www.w3.org/Bugs/Public/show_bug.cgi?id=25293 as Webdriver spec doesn't define a way to set speed/duration of swipe.

However, I don't understand the difference between swipe and flick so I don't know if Capybara should provide both methods.

@twalpole
Copy link
Member Author

twalpole commented Apr 8, 2014

@abotalov - speed -- webdriver does support a speed for the flick (the ruby selenium-webdriver has :fast and :normal) -- a slower flick would be what most people would call a swipe I believe -- I don't know if it makes sense to have the two as separate methods, or pass a parameter, or just not worry about speed

@abotalov
Copy link
Collaborator

abotalov commented Apr 8, 2014

The part about speed is currently missing in Webdriver spec so when it will appear there may be API breakage in Selenium 3.

So I would prefer to provide a single method without saying in documentation if it's fast or not. Maybe it would be swipe with normal speed for now.

Appium provides a much more robust API than Webdriver language bindings currently provide. I think swipe vs flick situation and other disputable API decisions should be handled upstream.

@abotalov
Copy link
Collaborator

abotalov commented Apr 8, 2014

I don't like single_tap as a method name. Appium provides a count parameter to tap (https://github.com/appium/appium/blob/9f798f52e112477eb35abcb04c6c46778d77a100/docs/gestures.md) so maybe this API will be added to Selenium in future.

I think it would be better to name this method tap. This allow us to add :count option in future if Webdriver Ruby binding will support it.

I'm fine with adding double_tap as a short-hand method.

directions = if args.last.is_a?(Hash) then args.pop else {} end
directions.default = 0
args.each { |arg| directions[arg] = 200 }
x = directions[:right] - directions[:left]
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do you mean by :left, :right, :down, :up?

Copy link
Member Author

Choose a reason for hiding this comment

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

its the direction of swipe

@twalpole
Copy link
Member Author

twalpole commented Apr 8, 2014

@abotalov - the issue with tap is that it would be overriding Object#tap with a function that is completely different - that might be a surprise to some people


##
#
# Sinlge tap on the Element
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo

@twalpole
Copy link
Member Author

twalpole commented Apr 9, 2014

pulling in @jonleighton @mhoran @jferris @jcoglan , if any of you are planning on implementing touch methods do you have any feedback on this?

@jferris
Copy link

jferris commented Apr 9, 2014

QtWebKit supports touch events (and seems to have them on by default, even on non-mobile clients), so I think this should be fine in capybara-webkit. I know some users are using them now by just sending the events manually. I'm not sure when I'll be able to go through and implement the new API, though.

@Ricardonacif
Copy link

Hey, there's this gem I have developed that adds touch gestures support to Capybara. Take a look at https://github.com/Ricardonacif/touch_action .

Thanks,

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

Successfully merging this pull request may close these issues.

None yet

5 participants