-
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
Added isVisible Missing Command (Fixes #4037) #4065
Added isVisible Missing Command (Fixes #4037) #4065
Conversation
|
I've added the |
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.
Writing tests to test the functionality of the code you added is important whenever you raise a PR.
There's already a PR under work for this command so we won't be merging this, but you're encouraged to learn how the tests work and then try to work on some other issue.
* @example | ||
* export default { | ||
* demoTest(browser: NightwatchAPI): void { | ||
* const result = browser.element('#main ul li a.first').isVisible(); | ||
* .assert.valueEquals('custom text'); | ||
* }, | ||
* | ||
* async demoTestAsync(browser: NightwatchAPI): Promise<void> { | ||
* const result = await browser.element('#main ul li a.first').isVisible(); | ||
* console.log('element text:', result); | ||
* } | ||
* } |
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 adding some example to JSDoc or anywhere else, it should be tested by you first by running them in one of the example test files. This example code won't work.
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.
alrighty ill take care of this in future
* @since 3.0.0 | ||
* @method isVisible | ||
* @memberof ScopedWebElement | ||
* @instance | ||
* @syntax browser.element(selector).isVisible() | ||
* @see https://www.w3.org/TR/webdriver#dfn-get-element-text | ||
* @returns {ScopedValue<string>} | ||
* @alias isDisplayed |
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.
This needs to be updated as well.
async isVisibleElement(id) { | ||
const element = await this.getWebElement(id); | ||
const elementVisible = await element.isDisplayed(); | ||
|
||
return elementVisible; | ||
}, |
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.
We don't need to create this again, there's already a isElementDisplayed
method which does the same.
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.
Oh i didnt notice isElementDisplayed because i was taking reference from getText()
Thank you for the feedback and guidance regarding writing tests for the code I added. I will make sure to include all the changes in future which u have mentioned in the comments above |
features/my-new-feature
orissue/123-my-bugfix
);