-
Notifications
You must be signed in to change notification settings - Fork 188
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
Session details #201
Session details #201
Conversation
- IHasSessionDetails was added and implemented by AppiumDriver
- a new test SessionDetailTest. - bug fixes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just had some minor comments. ^_^ btw, are you still on slack? i'm sorta back.
@@ -425,7 +244,7 @@ public void Rotate(Dictionary<string, int> opts) | |||
|
|||
public void HideKeyboard() => AppiumCommandExecutionHelper.HideKeyboard(this, null, null); | |||
|
|||
/// <sumary> | |||
/// <summary> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you're going to remove all the others, might as well remove this one too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is necessary here because it is not declared by any interface or super class
get | ||
{ | ||
var platform = GetSessionDetail("platformName"); | ||
if (platform == null || string.IsNullOrEmpty(Convert.ToString(platform))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var platform = (String)GetSessionDetails("platformName");
if (string.isnullorempty(platform)) { platform = (String)GetSessionDetail("platform");}
return platform;
OR
i like this way
var platform = (String)GetSessionDetail("platformName");
return (string.IsNullOrEmpty(platform)) ? (String)GetSessionDetail("platform") : platform;
(or use "as" instead of cast)
{ | ||
get | ||
{ | ||
var automation = GetSessionDetail("automationName"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above, why not just cast to a string? then return the casted string. (I haven't looked at the underlying object, are you doing this because the object is not a string type typically?)
also, you can use the "automationName" const in the MobileCapabilityType file.
} | ||
|
||
public bool IsBrowser => GetSessionDetail("browserName") != null | ||
&& Context.IndexOf("NATIVE_APP", StringComparison.OrdinalIgnoreCase) < 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for both "browerName" and "NATIVE_APP" can use their "static readonly" in their respective files.
@TikhomirovSergey are you online? want to get on slack? |
- code style issues were fixed out
@Astro03 There is the change. Could you take a look at this. PS: there is some tech issue. I can't log in to slack cuz I can't receive sms with a code for some reason |
@TikhomirovSergey have you tried to contact Slack's customer support? Perhaps there are other ways of getting the code (like Google Authenticator). There is one important topic I'd like to discuss about W3C stuff support by java client |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. just had that one comment.
public string AutomationName => GetSessionDetail(MobileCapabilityType.AutomationName) as string; | ||
|
||
public bool IsBrowser => GetSessionDetail(MobileCapabilityType.BrowserName) != null | ||
&& Context.IndexOf("NATIVE_APP", StringComparison.OrdinalIgnoreCase) < 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we use the native_app const as well?
btw, my personal preference is to put {} around all blocks, even if it's a one line if stmt or return. |
- code style issues were fixed out
Change list
Types of changes
Details
I have started to add features which have been add to java clint since Jan'17