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 support for tapAtPoint action #189

Merged
merged 4 commits into from
Jul 13, 2017
Merged

Add support for tapAtPoint action #189

merged 4 commits into from
Jul 13, 2017

Conversation

blankg
Copy link

@blankg blankg commented Jul 8, 2017

Motivation for adding this api - we are migrating our mobile app from a hybrid Cordova app to react-native, our first milestone is moving to react-native, getting rid of Cordova but still staying in the WebView and then gradually move our UI into react-native.
I would like to use detox for a very basic e2e sanity test and without the tapAtPoint api we cannot do that while most of our app is in a WebView.
Also this API can be useful for other scenarios I guess (tapping on maps and such) otherwise it won't be in EarlGrey :)

@rotemmiz
Copy link
Member

Hey @blankg,
Thanks for the contribution!

We were actually having a debate regarding this PR. Although EarlGrey offers this API, we were not sure if it makes any sense when used on full screen elements, since the tap point will be change on different devices (it will even change if you decide to change the font size in your system).
This means we can't trust the same test will pass on all devices.

How are you planning to use it in your tests ? have you already tried it ?

@blankg
Copy link
Author

blankg commented Jul 11, 2017

Hi @rotemmiz, I figured this was the reason you didn't expose this api at the first place.
I would not build an extensive test suite based on such an API but what we are looking for is running a very basic e2e sanity test (just a couple of clicks to see that the app is not broken).
It seems to work fine with the tap at point api.
I think this api could be useful for others migrating from cordova to react-native, which will probably mean staying with the WebView in some parts for a while.

I could added some disclaimers in the api documentation mentioning what you have written above, what do you say?

@rotemmiz
Copy link
Member

Sounds good, let's do it.

@blankg
Copy link
Author

blankg commented Jul 13, 2017

@rotemmiz, updated the documentation. Let me know if you have some different phrasing in mind.

@rotemmiz rotemmiz merged commit 3d4009b into wix:master Jul 13, 2017
@rotemmiz
Copy link
Member

PR is now merged, thanks @blankg !

@wix wix locked and limited conversation to collaborators Jul 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants