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

Method added to capture screenshot for element #2507

Closed

Conversation

suniljaiswal01
Copy link
Contributor

@suniljaiswal01 suniljaiswal01 commented Jul 29, 2020

Motivation/Description of the PR

  • Description of this PR, which problem it solves
    User will be able to capture screenshot of specified element

  • Resolves #issueId (if applicable).
    I screenshot the element #2401

Applicable helpers:

  • WebDriver
  • Puppeteer
  • Nightmare
  • Protractor
  • TestCafe
  • Playwright

Type of change

  • 🚀 New functionality

Checklist:

  • Tests have been added
  • Documentation has been added (Run npm run docs)
  • Lint checking (Run npm run lint)
  • Local tests are passed (Run npm test)

@kobenguyent
Copy link
Collaborator

@suniljaiswal01 would be nice if you could support other helpers like puppeteet, playwright. I do believe they would have similar API to do so.

@suniljaiswal01
Copy link
Contributor Author

@suniljaiswal01 would be nice if you could support other helpers like puppeteet, playwright. I do believe they would have similar API to do so.

@peterngtr Add support for other helpers

@DavertMik
Copy link
Contributor

Wow, that's good piece of work. Thank you very much. Looks good so far

DavertMik
DavertMik previously approved these changes Aug 2, 2020
@@ -1066,6 +1066,28 @@ class Nightmare extends Helper {
return this.browser.refresh();
}

/**
* {{> saveElementScreenshot }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

where is file with ext. mustache?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a command to generate .mustache files?, or do we have to manually create one

Copy link
Collaborator

Choose a reason for hiding this comment

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

Need add to manually. You can see example here

Copy link
Collaborator

Choose a reason for hiding this comment

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

And then run npm run docs

Copy link
Contributor

Choose a reason for hiding this comment

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

@suniljaiswal01 yes. this is a placeholder for a shared template. Because all standard actions have same documentation, we store them in docs/webapi/*.mustache so when you add saveElementScreenshot you are referring to: docs/webapi/saveElementScreenshot.mustache file to be included as documentation.

So please add this documentation file and I am ready to merge this PR and make a release

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mustache file added

Comment on lines +33 to +34
sudo apt-get update
sudo apt-get install libgbm1 libgbm-dev libwoff1 libopus0 libwebp6 libwebpdemux2 libenchant1c2a libgudev-1.0-0 libsecret-1-0 libhyphen0 libgdk-pixbuf2.0-0 libegl1 libgles2 libevent-2.1-6 libnotify4 libxslt1.1
Copy link
Collaborator

Choose a reason for hiding this comment

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

any reasons why we update this?

Comment on lines -1 to -9
## 2.6.7

* Add REST helper into `standardActingHelpers` array #2474 by @PeterNgTr
* Add missing `--invert` option for `run-workers` command #2504 by @pablopaul
* [WebDriver] Introduce `forceRightClick` method #2485 bylsuniljaiswal01
* [Playwright] Fix `setCookie` method #2491 by @bmbarker90
* [TypeScript] Update compilerOptions.target to es2017 #2483 by @shanplourde
* [Mocha] Honor reporter configuration #2465 by @trinhpham

Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we remove this? 🤔

@@ -1089,6 +1111,34 @@ class Nightmare extends Helper {
if (withinStatus !== false) await this._withinEnd();
}

async grabElementBoundingRect(locator, prop) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

you introduce this, would you mind adding it to the document?

@kobenguyent
Copy link
Collaborator

@suniljaiswal01 seems like you didn’t rebase with latest changes of master branch (v 2.6.7) but with v2.6.6

@suniljaiswal01
Copy link
Contributor Author

@peterngtr is it ok if i create a new PR incorporating all the mentioned changes?

@kobenguyent
Copy link
Collaborator

Hey @suniljaiswal01 just go with any that fits you best.

@suniljaiswal01
Copy link
Contributor Author

PR Created: #2521.

Closing this PR

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.

4 participants