-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Enable global element api to accept element properties as argument. #3835
Conversation
Status
|
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.
Looks good! Can you add a test?
While the changes in this PR look good, the sample test mentioned in the original issue is still not working (it always returns the first matched element instead of the element on index 24). It seems that the index property is never taken into consideration when finding the element. |
125d607
to
b6de5d5
Compare
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.
The changes look great now! Just one last fix:
lib/api/_loaders/element-global.js
Outdated
@@ -111,7 +128,7 @@ class ElementGlobal { | |||
abortOnFailure, retryInterval, timeout, suppressNotFoundErrors, index | |||
} = element; | |||
|
|||
if (isDefined(index)) { | |||
if (isDefined(index) && isNumber(index)) { |
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.
add an additional check for NaN
lib/api/_loaders/element-global.js
Outdated
if (locator instanceof Element) { | ||
this.setPropertiesFromElement(locator); | ||
|
||
locator = Locator.create(locator); |
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.
There are multiple calls to Locator.create
. We should refactor this module and everything should be handled in setLocator
method.
Any updates on this? |
…t from findElement to setLocator method
@gravityvi some tests are failing.Please help me out. |
the global element api will be deprecated and replaced by the new browser.element apis. |
This pull request enables global elment api to accept element properties as argument.