Skip to content

Refactor setting of touch actions options #756

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
306 changes: 220 additions & 86 deletions src/main/java/io/appium/java_client/TouchAction.java

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* See the NOTICE file distributed with this work for additional
* information regarding copyright ownership.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.appium.java_client.android;

import io.appium.java_client.PerformsTouchActions;
import io.appium.java_client.TouchAction;


public class AndroidTouchAction extends TouchAction<AndroidTouchAction> {
Copy link
Contributor

Choose a reason for hiding this comment

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

What the purpose of this class?

Copy link
Contributor Author

@mykola-mokhnach mykola-mokhnach Oct 28, 2017

Choose a reason for hiding this comment

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

This class will be needed in the future to add Android-specific touch actions (as well as the IOS one).

Also, there is second very Java-specific reason. This class is a specific implementation of Builder pattern with inheritance. If you check the ancestor classes they all have quite specific generic signatures and this all is done to make builder methods return appropriate class without casting, for example:

new IOSTouchAction(driver).press(...).doubleTap(...)

won't work in the previous implementation, because press returns the instance of TouchAction class instead of IOSTouchAction, where this method is implemented, so it is necessary to perform class cast:

((IOSTouchAction)new IOSTouchAction(driver).press(...)).doubleTap(...)

This approach fixes it, but now Java prevents one from passing TouchAction as a generic parameter, so Supplier<TouchAction> won't work, but Supplier<IOSTouchAction> works as expected. https://stackoverflow.com/questions/21086417/builder-pattern-and-inheritance has more details.


public AndroidTouchAction(PerformsTouchActions performsTouchActions) {
super(performsTouchActions);
}

}
39 changes: 31 additions & 8 deletions src/main/java/io/appium/java_client/ios/IOSTouchAction.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,28 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* See the NOTICE file distributed with this work for additional
* information regarding copyright ownership.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.appium.java_client.ios;

import io.appium.java_client.PerformsTouchActions;
import io.appium.java_client.TouchAction;
import io.appium.java_client.ios.touch.DoubleTapOptions;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.internal.HasIdentity;


public class IOSTouchAction extends TouchAction {
public class IOSTouchAction extends TouchAction<IOSTouchAction> {
Copy link
Contributor

Choose a reason for hiding this comment

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

@mykola-mokhnach Why this class is not fully deprecated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This class will be needed to add further TouchAction extensions


public IOSTouchAction(PerformsTouchActions performsTouchActions) {
super(performsTouchActions);
Expand All @@ -18,12 +34,15 @@ public IOSTouchAction(PerformsTouchActions performsTouchActions) {
* @param el element to tap.
* @param x x offset.
* @param y y offset.
* @return this TouchAction, for chaining.
* @return this IOSTouchAction, for chaining.
* @deprecated use {@link #tap(ActionOptions)} with count=2 instead.
*/
@Deprecated
public IOSTouchAction doubleTap(WebElement el, int x, int y) {
ActionParameter action = new ActionParameter("doubleTap", (HasIdentity) el);
action.addParameter("x", x);
action.addParameter("y", y);
ActionParameter action = new ActionParameter("doubleTap",
Copy link
Contributor

Choose a reason for hiding this comment

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

Also
Won't the doubleTap be supported anymore?

Copy link
Contributor Author

@mykola-mokhnach mykola-mokhnach Oct 28, 2017

Choose a reason for hiding this comment

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

It looks redundant to me. doubleTap can be easily replaced with tap(count:2) or double tap-wait-tap chains.
Would you prefer to keep it?

new DoubleTapOptions()
.withElement(el)
.withRelativeOffset(x, y));
parameterBuilder.add(action);
return this;
}
Expand All @@ -32,10 +51,14 @@ public IOSTouchAction doubleTap(WebElement el, int x, int y) {
* Double taps an element, offset from upper left corner.
*
* @param el element to tap.
* @return this TouchAction, for chaining.
* @return this IOSTouchAction, for chaining.
* @deprecated use {@link #tap(ActionOptions)} with count=2 instead.
*/
@Deprecated
public IOSTouchAction doubleTap(WebElement el) {
ActionParameter action = new ActionParameter("doubleTap", (HasIdentity) el);
ActionParameter action = new ActionParameter("doubleTap",
new DoubleTapOptions()
.withElement(el));
parameterBuilder.add(action);
return this;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* See the NOTICE file distributed with this work for additional
* information regarding copyright ownership.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.appium.java_client.ios.touch;

import io.appium.java_client.touch.OptionsWithAbsolutePositioning;
Copy link
Contributor

@TikhomirovSergey TikhomirovSergey Oct 27, 2017

Choose a reason for hiding this comment

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

If this class was added only for backward compatibility so I think it might be better to make it nested in IOSTouchAction

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but this should be visible for TouchAction options as well.


/**
* @deprecated this class will be removed
*/
@Deprecated
public class DoubleTapOptions extends OptionsWithAbsolutePositioning<DoubleTapOptions> {
}
40 changes: 40 additions & 0 deletions src/main/java/io/appium/java_client/touch/ActionOptions.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* See the NOTICE file distributed with this work for additional
* information regarding copyright ownership.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.appium.java_client.touch;

import java.util.HashMap;
import java.util.Map;

public abstract class ActionOptions<T extends ActionOptions<T>> {
/**
* This method is automatically called before building
* options map to verify the consistency of the instance.
*
* @throws IllegalArgumentException if there are problems with this options map.
*/
protected abstract void verify();

/**
* Creates a map based on the provided options.
*
* @return options mapping.
*/
public Map<String, Object> build() {
verify();
return new HashMap<>();
}
}
51 changes: 51 additions & 0 deletions src/main/java/io/appium/java_client/touch/LongPressOptions.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* See the NOTICE file distributed with this work for additional
* information regarding copyright ownership.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.appium.java_client.touch;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;

import java.time.Duration;
import java.util.Map;

public class LongPressOptions extends OptionsWithAbsolutePositioning<LongPressOptions> {
protected Duration duration = null;

/**
* Set the long press duration.
*
* @param duration the duration value to set.
* Time resolution unit is 1 ms.
* @return this instance for chaining.
*/
public LongPressOptions withDuration(Duration duration) {
checkNotNull(duration);
checkArgument(duration.toMillis() >= 0,
"Duration value should be greater or equal to zero");
this.duration = duration;
return this;
}

@Override
public Map<String, Object> build() {
final Map<String, Object> result = super.build();
if (duration != null) {
result.put("duration", this.duration.toMillis());
}
return result;
}
}
20 changes: 20 additions & 0 deletions src/main/java/io/appium/java_client/touch/MoveToOptions.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* See the NOTICE file distributed with this work for additional
* information regarding copyright ownership.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.appium.java_client.touch;

public class MoveToOptions extends OptionsWithRelativePositioning<MoveToOptions> {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* See the NOTICE file distributed with this work for additional
* information regarding copyright ownership.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.appium.java_client.touch;

import static com.google.common.base.Preconditions.checkNotNull;

import org.openqa.selenium.Point;
import org.openqa.selenium.WebElement;
import org.openqa.selenium.internal.HasIdentity;

import java.util.Map;

public abstract class OptionsWithAbsolutePositioning<T extends OptionsWithAbsolutePositioning<T>>
extends ActionOptions<T> {
private String elementId = null;
private Point absoluteOffset = null;
private Point relativeOffset = null;

/**
* Set the destination element for the corresponding action.
*
* @param element the destination element.
* @return this instance for chaining.
*/
public T withElement(WebElement element) {
checkNotNull(element);
this.elementId = ((HasIdentity) element).getId();
//noinspection unchecked
return (T) this;
}

/**
* Set the absolute offset for the corresponding action.
*
* @param xOffset the absolute distance from the left screen corner (the element must not be set).
* @param yOffset the absolute distance from the top screen corner.
* @return this instance for chaining.
*/
public T withAbsoluteOffset(int xOffset, int yOffset) {
this.absoluteOffset = new Point(xOffset, yOffset);
//noinspection unchecked
return (T) this;
}

/**
* Set the relative offset for the corresponding action.
*
* @param xOffset the relative distance from the left element corner (the element must be set).
* @param yOffset the relative distance from the top element corner (the element must be set).
* @return this instance for chaining.
*/
public T withRelativeOffset(int xOffset, int yOffset) {
this.relativeOffset = new Point(xOffset, yOffset);
//noinspection unchecked
return (T) this;
}

@Override
protected void verify() {
if (elementId == null) {
if (absoluteOffset == null) {
throw new IllegalArgumentException(
"Absolute offset must be defined if 'element' option is not set");
}
if (relativeOffset != null) {
throw new IllegalArgumentException(
"Relative offset must not be defined if 'element' option is not set");
}
} else {
if (absoluteOffset != null) {
throw new IllegalArgumentException(
"Absolute offset must not be defined if 'element' option set");
}
}
}

@Override
public Map<String, Object> build() {
final Map<String, Object> result = super.build();
if (absoluteOffset != null) {
result.put("x", absoluteOffset.x);
result.put("y", absoluteOffset.y);
}
if (relativeOffset != null) {
result.put("x", relativeOffset.x);
result.put("y", relativeOffset.y);
}
if (elementId != null) {
result.put("element", elementId);
}
return result;
}
}
Loading