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.

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

Feature/win #158

Merged
merged 12 commits into from
Sep 4, 2016
Merged

Feature/win #158

merged 12 commits into from
Sep 4, 2016

Conversation

stoneman
Copy link
Contributor

Change list

Add WindowsDriver and WindowsElement for use with https://github.com/appium/appium-windows-driver

Types of changes

What types of changes are you proposing/introducing to .NET 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)

Details

Some methods throw NotImplementedExceptions due to https://github.com/Microsoft/WinAppDriver/ (and therefore https://github.com/appium/appium-windows-driver) not supporting the functionality.

@stoneman stoneman mentioned this pull request Aug 23, 2016
3 tasks
@stoneman
Copy link
Contributor Author

@yodurr Can you add the AccessibilityID test to this branch please?

@yodurr
Copy link
Contributor

yodurr commented Aug 24, 2016

what about code like this in WindowsMultiSelectControlTest.cs? What other Accessibility ID API do you want to test?
AlarmClockSession.FindElementByAccessibilityId("AlarmPivotItem").Click();

Also I still don't follow why we need the IFindByWindowsUIAutomation namespace if the default API for finding by Accessibility ID works.

@stoneman
Copy link
Contributor Author

@yodurr IFindByWindowsUIAutomation allows for much more complex queries than just locating an element by it's AccessibilityID. It could also allow for complex interactions. For example, here is an Android query that would scroll a scrollable view so that a view with a description containing my_description is made visible:

driver.FindElementByAndroidUIAutomator("new UiScrollable(new UiSelector().scrollable(true).instance(0)).scrollIntoView(new UiSelector().descriptionContains(\"my_description\").instance(0));");

@TikhomirovSergey Do you have any thoughts on this and are you happy with @yodurr's proposed AccessibilityId example test?

{
//return CollectionConverterUnility.ConvertToExtendedWebElementCollection<W>(
// this.FindElements("-windows uiautomation", selector));
throw new NotImplementedException();
Copy link
Contributor

Choose a reason for hiding this comment

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

May be there is sence remove this string and make this string not commented

return CollectionConverterUnility.ConvertToExtendedWebElementCollection<W>(
                 this.FindElements("-windows uiautomation", selector));

It depends on server-side and when it would be available then it should work immediately

@TikhomirovSergey
Copy link
Contributor

It looks ok. I want to publish one more (not major) build and then merge this PR.

I would like to ask you for a sample of the windows automation searching (you can post it as a comment).
Later it will be included to integration tests/

PS: I would like to add similar changes to the Java client with @JonStoneman authorship. Is it ok?

/// Finds a list of elements that match the Windows UIAutomation selector supplied
/// </summary>
/// <param name="selector">a Windows UIAutomation selector</param>
/// <returns>IWebElement object so that you can interact that object</returns>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please correct summary

@TikhomirovSergey
Copy link
Contributor

@JonStoneman
I'm going to publish the v.2.0.1.1 on this weekend. And then I'm merging this PR.
There are conflicts. Sorry. It is my fault. I will resolve it on my own

@TikhomirovSergey TikhomirovSergey added this to the 3.0.0.1 milestone Sep 2, 2016
@TikhomirovSergey TikhomirovSergey merged commit 4655675 into appium:master Sep 4, 2016
@TikhomirovSergey
Copy link
Contributor

@JonStoneman This PR was mergen successfully. Now you are free to develop it further. Also I'm going to improve some thing. Thank you for the contribution. I hope we will cooperate further. :)

@stoneman
Copy link
Contributor Author

stoneman commented Sep 5, 2016

Great! - thanks @TikhomirovSergey :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants