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

Mykola mokhnach's actions params: The addition to the #756 #760

Merged
merged 15 commits into from Nov 12, 2017

Conversation

Projects
None yet
4 participants
@TikhomirovSergey
Member

TikhomirovSergey commented Nov 3, 2017

Change list

The addition to the #756

Types of changes

  • No changes in production code.
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

mykola-mokhnach and others added some commits Oct 16, 2017

The addition to the #756:
- the draft of architectural improvements
The addition to the #756:
- reversion of accidentally changed strings/javadocs.
- javadocs were added to new-designed static methods
- ability to take x&y offsets by RelativeOffsetOption
The addition to the #756:
- old integration tests were updated
@TikhomirovSergey

This comment has been minimized.

Show comment
Hide comment
@TikhomirovSergey

TikhomirovSergey Nov 3, 2017

Member

@mykola-mokhnach @SrinivasanTarget
I finished. Could you take a look at this one?
@SrinivasanTarget could you run iOS tests?

Member

TikhomirovSergey commented Nov 3, 2017

@mykola-mokhnach @SrinivasanTarget
I finished. Could you take a look at this one?
@SrinivasanTarget could you run iOS tests?

@mykola-mokhnach mykola-mokhnach referenced this pull request Nov 4, 2017

Closed

Refactor setting of touch actions options #756

1 of 4 tasks complete
The addition to the #756. Code improvements:
- removal of redundant code from the WaitOptions.
- code of test was optimized.
The addition to the #756. archetetural improvement:
- PositionOffsetOption and WebElementOption instead of AbsoluteOffsetOption and RelativeOffsetOption.

@appium appium deleted a comment from codacy-bot Nov 5, 2017

* @param longPressOptions see {@link LongPressOptions}.
* @return this TouchAction, for chaining.
*/
public T longPress(LongPressOptions longPressOptions) {

This comment has been minimized.

@mykola-mokhnach

mykola-mokhnach Nov 6, 2017

Contributor

do we still need this method?

@mykola-mokhnach

mykola-mokhnach Nov 6, 2017

Contributor

do we still need this method?

This comment has been minimized.

@TikhomirovSergey

TikhomirovSergey Nov 6, 2017

Member

@mykola-mokhnach
Yes because it has the additional

withDuration
@TikhomirovSergey

TikhomirovSergey Nov 6, 2017

Member

@mykola-mokhnach
Yes because it has the additional

withDuration
The addition to the #756. Archetectural improvement:
- offsets were divided to absolute(PointOption) relative(OffsetOption)
and relative to an element (ElementOption)
- The PointOption which may take any position option
- signature of TouchAction was changed

@appium appium deleted a comment from codacy-bot Nov 8, 2017

The addition to the #756. Archetectural improvement.
AbstractPositionOption, OffsetOption, Position were removed.
The set of class was simplified.
Signature of TouchAction methods was changed and simplified.
@TikhomirovSergey

This comment has been minimized.

Show comment
Hide comment
@TikhomirovSergey

TikhomirovSergey Nov 10, 2017

Member

@mykola-mokhnach @SrinivasanTarget
There is one more change. It was done according our decision to use only absolute coordinates or an offset from an element. Please review the last change.

...Lets wait the new beta of appium with fixed appium/appium#7486 before the publishing of the 6.0.0-BETA.

Member

TikhomirovSergey commented Nov 10, 2017

@mykola-mokhnach @SrinivasanTarget
There is one more change. It was done according our decision to use only absolute coordinates or an offset from an element. Please review the last change.

...Lets wait the new beta of appium with fixed appium/appium#7486 before the publishing of the 6.0.0-BETA.

@appium appium deleted a comment from codacy-bot Nov 10, 2017

@mykola-mokhnach

minor comments. I like the way it looks now.

public static <E extends Throwable> Matcher<Runnable> failsWith(
final Class<E> throwableType, final Matcher<? super E> throwableMatcher) {
return new FailsWithMatcher<>(allOf(instanceOf(throwableType), throwableMatcher));

This comment has been minimized.

@SrinivasanTarget

SrinivasanTarget Nov 11, 2017

Member

Also compilation error here

@SrinivasanTarget

SrinivasanTarget Nov 11, 2017

Member

Also compilation error here

This comment has been minimized.

@TikhomirovSergey

TikhomirovSergey Nov 11, 2017

Member

??? ок

@TikhomirovSergey

This comment has been minimized.

@TikhomirovSergey

TikhomirovSergey Nov 11, 2017

Member

@SrinivasanTarget
I can't reproduce it even after removal of the project from the disc and cloning it again.
Could you try again? If you are facing the same issue could you provide some text of the error.

@TikhomirovSergey

TikhomirovSergey Nov 11, 2017

Member

@SrinivasanTarget
I can't reproduce it even after removal of the project from the disc and cloning it again.
Could you try again? If you are facing the same issue could you provide some text of the error.

This comment has been minimized.

@TikhomirovSergey

TikhomirovSergey Nov 11, 2017

Member

@SrinivasanTarget even Travis could compile the project

@TikhomirovSergey

TikhomirovSergey Nov 11, 2017

Member

@SrinivasanTarget even Travis could compile the project

This comment has been minimized.

@SrinivasanTarget

SrinivasanTarget Nov 12, 2017

Member

Yes travis wasn't able to catch it. Only IDE throws it on file level but still project builds fine. no instance of type variable exists so that E conforms to capture of ? super.... is error.

@SrinivasanTarget

SrinivasanTarget Nov 12, 2017

Member

Yes travis wasn't able to catch it. Only IDE throws it on file level but still project builds fine. no instance of type variable exists so that E conforms to capture of ? super.... is error.

This comment has been minimized.

@SrinivasanTarget

SrinivasanTarget Nov 12, 2017

Member

looks fine in latest IDE

@SrinivasanTarget

SrinivasanTarget Nov 12, 2017

Member

looks fine in latest IDE

This comment has been minimized.

@mykola-mokhnach

mykola-mokhnach Nov 12, 2017

Contributor

Do you use Eclipse?

@mykola-mokhnach

mykola-mokhnach Nov 12, 2017

Contributor

Do you use Eclipse?

This comment has been minimized.

@SrinivasanTarget

SrinivasanTarget Nov 12, 2017

Member

Intellij IDEA 2017.1.5 throws this error but no errors in Intellij IDEA 2017.2

@SrinivasanTarget

SrinivasanTarget Nov 12, 2017

Member

Intellij IDEA 2017.1.5 throws this error but no errors in Intellij IDEA 2017.2

The addition to the #756. The fixing out of minor issues:
- the `coordinates` method was renamed to `withCoordinates`
- removal of unused imports from ElementOption
@TikhomirovSergey

This comment has been minimized.

Show comment
Hide comment
@TikhomirovSergey

TikhomirovSergey Nov 11, 2017

Member

I am waiting for @SrinivasanTarget 's approve.

Member

TikhomirovSergey commented Nov 11, 2017

I am waiting for @SrinivasanTarget 's approve.

@codacy-bot

This comment has been minimized.

Show comment
Hide comment
@codacy-bot

codacy-bot Nov 11, 2017

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- src/main/java/io/appium/java_client/ios/IOSTouchAction.java  1
- src/main/java/io/appium/java_client/TouchAction.java  6
         

Complexity decreasing per file
==============================
+ src/test/java/io/appium/java_client/ios/XCUIAutomationTest.java  -1
         

See the complete overview on Codacy

codacy-bot commented Nov 11, 2017

Codacy Here is an overview of what got changed by this pull request:

Complexity increasing per file
==============================
- src/main/java/io/appium/java_client/ios/IOSTouchAction.java  1
- src/main/java/io/appium/java_client/TouchAction.java  6
         

Complexity decreasing per file
==============================
+ src/test/java/io/appium/java_client/ios/XCUIAutomationTest.java  -1
         

See the complete overview on Codacy

@appium appium deleted a comment from codacy-bot Nov 11, 2017

@TikhomirovSergey TikhomirovSergey merged commit a433896 into appium:master Nov 12, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment