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

Allow Appium to Scroll To a particular element on screen #109

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nkjith
Copy link

@nkjith nkjith commented Nov 23, 2020

Adding Scroll To functionality which will help OSX Applications to scroll to a particular element.
The code increments the scroll bar and checks if the passed on element is visible.

@mykola-mokhnach
Copy link
Contributor

Thanks for your contribution @nkjith

Just FYI: we are currently working on a next-gen mac driver based on XCTest - https://github.com/appium/appium-mac2-driver
It implements the most of the features that, for example, mobile xcuitest driver has. Perhaps, it makes sense to support its development rather than trying to fix this one

@@ -891,6 +891,55 @@ -(PFUIElement*) windowForHandle:(NSString*)windowHandle
return [windows objectAtIndex:windowIndex];
}

-(BOOL) scrollPage:(id)element
{
float increment_by = 0.25;
Copy link
Contributor

Choose a reason for hiding this comment

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

could you please use camelCase naming to match with the current project coding style?

-(BOOL) scrollPage:(id)element
{
float increment_by = 0.25;
bool scroll_found = false;
Copy link
Contributor

@mykola-mokhnach mykola-mokhnach Nov 23, 2020

Choose a reason for hiding this comment

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

Please use YES/NO constants with BOOL type

float increment_by = 0.25;
bool scroll_found = false;
PFUIElement* scroll;
float axValue = 0,adder = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

please define variables closer to the block where they used, also each variable should be defined in its own row

PFUIElement* scroll;
float axValue = 0,adder = 0;

id parent = [(PFUIElement*)element AXParent];
Copy link
Contributor

Choose a reason for hiding this comment

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

can the parent's type be more specific?

{
for(id child in siblings)
{
if([[(PFUIElement*)child AXRole] isEqualToString:@"AXScrollBar"])
Copy link
Contributor

Choose a reason for hiding this comment

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

is there any reason to define the child element type as id in the loop and cast it to PFUIElement* here?

NSPoint middlePoint = [[element AXPosition] pointValue];
if(middlePoint.y < 0)
{

Copy link
Contributor

Choose a reason for hiding this comment

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

should this block be empty?

id parent = [(PFUIElement*)element AXParent];
NSArray* siblings = [parent AXChildren];

while(scroll == nil && parent != nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

please add space after while, if and other similar keywords to match with the common project style

siblings = [parent AXChildren];
}

NSPoint middlePoint = [[element AXPosition] pointValue];
Copy link
Contributor

Choose a reason for hiding this comment

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

what middlePoint is used for?

while(adder<=1)
{
axValue = [scroll.AXValue floatValue];
adder = axValue + increment_by;
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be possible to have more descriptive name for the adder variable?


}

if(scroll_found)
Copy link
Contributor

Choose a reason for hiding this comment

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

we could immediately return NO here if the var is false

@@ -963,6 +963,28 @@ - (AppiumMacHTTPJSONResponse *)post_doubleclick:(NSString*)path data:(NSData *)p
}];
}


// POST /session/:sessionId/element/:id/scrollTo
// Scroll to the particular element passed to Appium. Scrolls till the bottom of the screen till the element becomes visible
Copy link
Contributor

Choose a reason for hiding this comment

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

is it always the case we need to scroll to the bottom? what if the destination element is located in the other direction, for example upper or at the right side?

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

Successfully merging this pull request may close these issues.

2 participants