Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Android setting engine#477

Merged
TikhomirovSergey merged 5 commits intoappium:masterappium/java-client:masterfrom
TikhomirovSergey:masterCopy head branch name to clipboard
Sep 27, 2016
Merged

Android setting engine#477
TikhomirovSergey merged 5 commits intoappium:masterappium/java-client:masterfrom
TikhomirovSergey:masterCopy head branch name to clipboard

Conversation

@TikhomirovSergey
Copy link
Contributor

@TikhomirovSergey TikhomirovSergey commented Sep 25, 2016 •
edited
Loading

Change list

  • io.appium.java_client.AppiumSetting became deprecated. It was moved to io.appium.java_client.android.Setting.
  • the method io.appium.java_client.AppiumDriver#getSettings() was completely moved to io.appium.java_client.android.AndroidDriver.
  • the new interface io.appium.java_client.android.HasSettings was added. It has default implementation. And this interface is implemented by AndroidDriver.
    Methods:
    • void setSetting(Setting setting, Object value)
    • JsonObject getSettings()
    • void ignoreUnimportantViews(Boolean compress)
    • void configuratorSetWaitForIdleTimeout(int timeout)
    • void configuratorSetWaitForSelectorTimeout(int timeout)
    • void configuratorSetScrollAcknowledgmentTimeout(int timeout)
    • void configuratorSetKeyInjectionDelay(int delay)
    • void configuratorSetActionAcknowledgmentTimeout(int timeout)
      This change was proposed at add support to set UiAutomator Congfigurator values #410 by @truebit

Types of changes

What types of changes are you proposing/introducing to Java client?
Put an x in the boxes that apply

  • 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)
    Windows Mobile automation support.  #471

TikhomirovSergey and others added 5 commits September 23, 2016 23:29
@TikhomirovSergey
Refactoring of setting engine
0042636 
- AppiumSetting became deprected. It was moved to io.appium.java_client.android.settings.Setting
- the new interface io.appium.java_client.android.HasSettings was added
- set/getSettings methods were moved to AndroidDriver
- AndroidDriver#ignoreUnimportantViews became deprecated
@TikhomirovSergey
The new API refactoring
0db2636
@TikhomirovSergey
Interpretation of #410 to new architecture
283950b
@TikhomirovSergey
Test coverage
263a0c5
@TikhomirovSergey
Check style issue fix
c005127
@TikhomirovSergey
Copy link
Contributor Author

TikhomirovSergey commented Sep 25, 2016

@SrinivasanTarget @truebit
Could you review this PR?

@TikhomirovSergey TikhomirovSergey added this to the 5.0.0 milestone Sep 25, 2016
SrinivasanTarget
SrinivasanTarget approved these changes Sep 25, 2016
View reviewed changes
Copy link
Member

@SrinivasanTarget SrinivasanTarget left a comment

Choose a reason for hiding this comment

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

I like the changes 👍 LGTM

@truebit
Copy link
Contributor

truebit commented Sep 26, 2016 •
edited
Loading

the settings api has been changed to top level as requested by @jlipps in appium/appium-android-driver#153
you could find the doc in https://github.com/truebit/appium/blob/master/docs/en/advanced-concepts/settings.md#supported-settings

So these settings api requests should be the same like ignoreUnimportantViews in top level instead of configurator/xxxTimeout

@TikhomirovSergey
Copy link
Contributor Author

TikhomirovSergey commented Sep 26, 2016

@truebit
Yes and proposed code actualy does that:
https://github.com/appium/java-client/pull/477/files#diff-7daaa6d2742d959db92558292282ea56R41
https://github.com/appium/java-client/pull/477/files#diff-7daaa6d2742d959db92558292282ea56R78
https://github.com/appium/java-client/pull/477/files#diff-cebdf3eaee01b491314c7101b3103f7bR298
https://github.com/appium/java-client/blob/master/src/main/java/io/appium/java_client/MobileCommand.java#L92

Isn't it?

@TikhomirovSergey TikhomirovSergey merged commit ab2ff11 into appium:master Sep 27, 2016
truebit
truebit reviewed Sep 27, 2016
View reviewed changes
src/main/java/io/appium/java_client/android/Setting.java
public enum Setting {

IGNORE_UNIMPORTANT_VIEWS("ignoreUnimportantViews"),
CONFIGURATOR("configurator");
Copy link
Contributor

@truebit truebit Sep 27, 2016

Choose a reason for hiding this comment

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

no, this code is still the two level configurator. Now it should be like ignoreUnimportantViews. Every setting here for configurators should be individivual

@truebit
Copy link
Contributor

truebit commented Sep 27, 2016

all below settings are at same level like ignoreUnimportantViews. there is no 'configurator' sub setting

"actionAcknowledgmentTimeout" - Int which is the same as setActionAcknowledgmentTimeout. If a negative value is given, it would set to default(3 * 1000 milliseconds)

"keyInjectionDelay" - Int which is the same as setKeyInjectionDelay. If a negative value is given, it would set to default(0 milliseconds)

"scrollAcknowledgmentTimeout" - Int which is the same as setScrollAcknowledgmentTimeout. If a negative value is given, it would set to default(200 milliseconds)

"waitForIdleTimeout" - Int which is the same as setWaitForIdleTimeout. If a negative value is given, it would set to default(10 * 1000 milliseconds)

"waitForSelectorTimeout" - Int which is the same as setWaitForSelectorTimeout. If a negative value is given, it would set to default(10 * 1000 milliseconds)

@TikhomirovSergey
Copy link
Contributor Author

TikhomirovSergey commented Sep 27, 2016

Ok. Will iptrove this a bit later.

@TikhomirovSergey TikhomirovSergey mentioned this pull request Sep 28, 2016
Merged
3 tasks
TikhomirovSergey added a commit that referenced this pull request Sep 30, 2016
@TikhomirovSergey
The addition to #477 (#480)
847a222 
* The addition to #477

- the enum `io.appium.java_client.android.ConfiguratorParameters` was removed
- new elements were added to the `io.appium.java_client.android.Settings`
- refactoring of the `io.appium.java_client.HasSettings`.

* Work on remarks. Addition to #477
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SrinivasanTarget SrinivasanTarget SrinivasanTarget approved these changes

+1 more reviewer

@truebit truebit truebit left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement refactor

Projects

None yet

Milestone

5.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@TikhomirovSergey @truebit @SrinivasanTarget
Morty Proxy This is a proxified and sanitized view of the page, visit original site.