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

Make scroll tracking configurable with a cap #284

Merged
merged 3 commits into from
Jun 11, 2019
Merged

Make scroll tracking configurable with a cap #284

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
43ed200
Make scroll tracking configurable with a cap
imurchie Jun 10, 2019
573e2e5
Address comments
imurchie Jun 10, 2019
f99ee67
Shift to a setting
imurchie Jun 10, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 82 additions & 33 deletions app/src/main/java/io/appium/uiautomator2/core/InteractionController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@

import io.appium.uiautomator2.common.exceptions.UiAutomator2Exception;

import io.appium.uiautomator2.model.AppiumUIA2Driver;
import io.appium.uiautomator2.model.Session;
import io.appium.uiautomator2.utils.Logger;

import static io.appium.uiautomator2.utils.ReflectionUtils.invoke;
import static io.appium.uiautomator2.utils.ReflectionUtils.method;

Expand All @@ -32,6 +36,7 @@ public class InteractionController {
private static final String METHOD_TOUCH_DOWN = "touchDown";
private static final String METHOD_TOUCH_UP = "touchUp";
private static final String METHOD_TOUCH_MOVE = "touchMove";
private static final String TRACK_SCROLL_EVENT_CAP = "trackScrollEvents";
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather make it an appium setting

private final Object interactionController;

public InteractionController(Object interactionController) {
Expand Down Expand Up @@ -62,48 +67,92 @@ public boolean injectEventSync(final InputEvent event) throws UiAutomator2Except
return injectEventSync(event, true);
}

public boolean shouldTrackScrollEvents() {
Session session = AppiumUIA2Driver.getInstance().getSessionOrThrow();
Boolean trackScrollEvents = true;
if (session.hasCapability(TRACK_SCROLL_EVENT_CAP)) {
try {
trackScrollEvents = (Boolean) session.getCapability(TRACK_SCROLL_EVENT_CAP);
Copy link
Contributor

Choose a reason for hiding this comment

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

how about using String.format?

Copy link
Contributor

Choose a reason for hiding this comment

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

same below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I personally think the else ought to be there when it is, logically, an else, even if the return makes it technically "redundant".

Logger.error("Capability '" + TRACK_SCROLL_EVENT_CAP + "': " + trackScrollEvents);
Copy link
Member

Choose a reason for hiding this comment

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

is this meant to be an error log?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, no. That was just all that would show up in my logs without fiddling. I'll fix.

} catch (Exception e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is getCapability supposed to throw an exception?

Logger.error("Could not set '" + TRACK_SCROLL_EVENT_CAP + "' from capability: ", e);
trackScrollEvents = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

this is redundant

}
}

return trackScrollEvents;
}

private boolean doTouchDown(final int x, final int y) {
return (Boolean) invoke(method(CLASS_INTERACTION_CONTROLLER,
METHOD_TOUCH_DOWN, int.class, int.class), interactionController, x, y);
}

public boolean touchDown(final int x, final int y) throws UiAutomator2Exception {
return EventRegister.runAndRegisterScrollEvents(new ReturningRunnable<Boolean>() {
@Override
public void run() {
Boolean result = (Boolean) invoke(method(CLASS_INTERACTION_CONTROLLER,
METHOD_TOUCH_DOWN, int.class, int.class), interactionController, x, y);
setResult(result);
}
});
if (shouldTrackScrollEvents()) {
return EventRegister.runAndRegisterScrollEvents(new ReturningRunnable<Boolean>() {
@Override
public void run() {
setResult(doTouchDown(x, y));
}
});
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

else is redundant

Copy link
Contributor

Choose a reason for hiding this comment

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

same below

return doTouchDown(x, y);
}
}

private boolean doTouchUp(final int x, final int y) {
return (Boolean) invoke(method(CLASS_INTERACTION_CONTROLLER, METHOD_TOUCH_UP,
int.class, int.class), interactionController, x, y);
}

public boolean touchUp(final int x, final int y) throws UiAutomator2Exception {
return EventRegister.runAndRegisterScrollEvents(new ReturningRunnable<Boolean>() {
@Override
public void run() {
Boolean result = (Boolean) invoke(method(CLASS_INTERACTION_CONTROLLER, METHOD_TOUCH_UP,
int.class, int.class), interactionController, x, y);
setResult(result);
}
});
if (shouldTrackScrollEvents()) {
return EventRegister.runAndRegisterScrollEvents(new ReturningRunnable<Boolean>() {
@Override
public void run() {
setResult(doTouchUp(x, y));
}
});
} else {
Copy link
Member

Choose a reason for hiding this comment

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

spacing seems off in this block

return doTouchUp(x, y);
}
}

private boolean doTouchMove(final int x, final int y) {
return (Boolean) invoke(method(CLASS_INTERACTION_CONTROLLER,
METHOD_TOUCH_MOVE, int.class, int.class), interactionController, x, y);
}

public boolean touchMove(final int x, final int y) throws UiAutomator2Exception {
return EventRegister.runAndRegisterScrollEvents(new ReturningRunnable<Boolean>() {
@Override
public void run() {
Boolean result = (Boolean) invoke(method(CLASS_INTERACTION_CONTROLLER,
METHOD_TOUCH_MOVE, int.class, int.class), interactionController, x, y);
setResult(result);
}
});
if (shouldTrackScrollEvents()) {
return EventRegister.runAndRegisterScrollEvents(new ReturningRunnable<Boolean>() {
@Override
public void run() {;
setResult(doTouchMove(x, y));
}
});
} else {
return doTouchMove(x, y);
}
}

private boolean doPerformMultiPointerGesture(final PointerCoords[][] pcs) {
return (Boolean) invoke(method(CLASS_INTERACTION_CONTROLLER,
METHOD_PERFORM_MULTI_POINTER_GESTURE, PointerCoords[][].class),
interactionController, (Object) pcs);
}

public Boolean performMultiPointerGesture(final PointerCoords[][] pcs) throws UiAutomator2Exception {
return EventRegister.runAndRegisterScrollEvents(new ReturningRunnable<Boolean>() {
@Override
public void run() {
Boolean result = (Boolean) invoke(method(CLASS_INTERACTION_CONTROLLER,
METHOD_PERFORM_MULTI_POINTER_GESTURE, PointerCoords[][].class),
interactionController, (Object) pcs);
setResult(result);
}
});
if (shouldTrackScrollEvents()) {
return EventRegister.runAndRegisterScrollEvents(new ReturningRunnable<Boolean>() {
@Override
public void run() {
setResult(doPerformMultiPointerGesture(pcs));
}
});
} else {
return doPerformMultiPointerGesture(pcs);
}
}
}