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

Add missing isPresent command in new Element API #4055

Closed
4 tasks done
garg3133 opened this issue Feb 26, 2024 · 16 comments
Closed
4 tasks done

Add missing isPresent command in new Element API #4055

garg3133 opened this issue Feb 26, 2024 · 16 comments

Comments

@garg3133
Copy link
Member

garg3133 commented Feb 26, 2024

Description of the bug/issue

The isPresent command is currently missing from the new Element API. To support this command, a new file named isPresent.js needs to be added inside lib/api/web-element/commands directory, whose content should be similar to the get* command files present in the same directory.

The argument passed to this.runQueuedCommandScoped in these files is the name of the corresponding method to be called from the lib/transport/selenium-webdriver/method-mappings.js file. Currently, there's no such method so it needs to be created.

For the isPresent command, we just need to check whether the element is an instance of WebElement or ShadowRoot.

Steps to reproduce

Go to any test file present under examples/tests directory (ecosia.js and duckDuckGo.js are good examples to work with) and try to add const result = await browser.element('input[name=q]').isPresent() and then run the test (see contributing guide on how to run the example tests). This would result in an error right now, while if you replace isPresent() with getText(), it should work fine.

After adding the command implementation, the .isPresent() command should work fine.

TODO

  • Add missing command
  • Fix JSDoc of the command
  • Add tests for the new command
  • Add types for the new command
@aditya123473892
Copy link

pls can i be assigned to work on this issue

@garg3133
Copy link
Member Author

@aditya123473892 I'll assign you this issue as soon as you come up with a potential solution for this issue. Feel free to open a PR.

@subhajit20
Copy link
Contributor

hey @garg3133 do I have to create the function isElementPresent on method-mapping.js file also?

@garg3133
Copy link
Member Author

garg3133 commented Feb 26, 2024

@subhajit20 You've already worked on similar PRs, it'd be better if you work on a different issue now. I'm creating these issues for beginners who are new to the project and not just to get them done.

I'll try to review your PRs tomorrow.

@subhajit20
Copy link
Contributor

@subhajit20 You've already worked on similar PRs, it'd be better if you work on a different issue now. I'm creating these issues for beginners who are new to the project and not just to get them done.

I'll try to review your PRs tomorrow.

Cool, but still I have made a pr, if you will have time then check that once.
Thanks!!

aditya123473892 added a commit to aditya123473892/nightwatch that referenced this issue Feb 26, 2024
@Jai0401 Jai0401 mentioned this issue Feb 26, 2024
8 tasks
aditya123473892 added a commit to aditya123473892/nightwatch that referenced this issue Feb 26, 2024
@Jai0401
Copy link

Jai0401 commented Feb 26, 2024

hey @garg3133 ,
I've completed the necessary changes. Could you please take some time to review them when you get a chance? I appreciate your feedback.

thanks!

This was referenced Feb 29, 2024
@AritraLeo
Copy link
Contributor

It seems like a good first issue to try out and get used to the codebase. The guidelines provided seems very apt and precise but is the issue still open to PRs? Because quite a few PRs were raised so I am doubtful . It'd be very nice if you let me know @garg3133

@AritraLeo
Copy link
Contributor

@garg3133 please have a look at my PR - #4098 linked to this issue.

@AritraLeo
Copy link
Contributor

@garg3133 the test is passing in ecosia.js file after initializing nightwatch

@Ankuristic
Copy link

could i start working on this issue or this issue is already closed

@Visbhavesh
Copy link

hey assign these issue to me and I will resolve these

@garg3133
Copy link
Member Author

In order to discourage duplicate PRs on an issue, assigning this issue to @piyushmishra1416 since he is the first one to come up with a good PR which is very close to the solution. Also, adding @ansh21 as one of the reviewers of the PR to take the PR forward. @piyushmishra1416 please feel free to unassign yourself from the issue if you are no longer available to work on the PR.

Moving forward, we will only be reviewing PRs from the person to whom the issue is assigned.

@shubham-2909
Copy link

hii @garg3133 is this issue still open i'd love to work on it

@nikhil-babar
Copy link

hello @garg3133 I would like to contribute and solve this bug, Please can you assign me this issue?

@sksingh2005
Copy link

hello @garg3133 sir is this issue still open ? As i would love to contribute to it.

@garg3133
Copy link
Member Author

Fixed in #4216. Thanks everyone!

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