Add padding support to element screenshot (#4440)

[sebinsua]

• Closes Add more advanced screenshot clipping options #4440
• Closes unhelpful error message when taking screenshot of element with zero height #5149

Pre-merge Tasks

• Have tests been added/updated for the changes in this PR?
• Has a PR to cypress-documentation been submitted to document any user-facing changes? Add padding support to element screenshot cypress-documentation#2139
• Have the type definitions been updated with any user-facing API changes? These are within the PR.
• Has the cypress.schema.json been updated with any new configuration options? There are no changes to the configuration that require changes to the schema.
• Has the original issue been tagged with a release in ZenHub?

Why?

Screenshotting an element will often crop out edges of the element (e.g. box-shadow might not be visible). It should be possible to tell Cypress to capture a padding around an element.

Approach

This implementation only adds padding support to element screenshots, since we did not feel it makes sense to modify screenshots of the fullPage, viewport or runner.

We tried two different approaches:

1. We tried to create an extra element with adjusted dimensions from the original element.
2. We tried to adjust the elPosition values to account for padding.

They are both committed.

We're leaning towards the second approach, because it doesn't mutate the DOM.

[CLAassistant]

All committers have signed the CLA.

[sebinsua]

@chrisbreiding @brian-mann

I noticed that you've both worked on the screenshot feature before. I've attempted an implementation that allows users to add padding to element screenshots and would like to get your opinion on: potential issues, API design, etc.

[chrisbreiding]

This looks really good. I like the API and the implementation and tests look solid.

@brian-mann What do you think about the API?

[jennifer-shehane]

This is awesome work. Left a couple of comments.

Does this also address the issue of screenshotting elements with 0 width/height by chance ? #5149

[jennifer-shehane]

@sebinsua Ok, so I checked with @brian-mann and we are officially approving the API change that is being proposed with the padding option. So, feel free to move forward with the schemas, docs, and type definitions needed.

There are some comments for updates to the code and further review wanted to the code that we'll continue.

Again, thanks for the contribution - just need to clean up some things based off of our code conventions.

[sebinsua]

@sebinsua Ok, so I checked with @brian-mann and we are officially approving the API change that is being proposed with the padding option. So, feel free to move forward with the schemas, docs, and type definitions needed.

Do you want them as part of this PR? Where are the schemas, docs and type definitions defined?

[chrisbreiding]

@sebinsua Ok, so I checked with @brian-mann and we are officially approving the API change that is being proposed with the padding option. So, feel free to move forward with the schemas, docs, and type definitions needed.

Do you want them as part of this PR? Where are the schemas, docs and type definitions defined?

The type definitions can be part of this PR. Augment cli/types/index.d.ts with the new padding option. There should be no need for schema changes; that's only needed for config (cypress.json) changes.

For documentation, that's a separate repo, so it will need to be a separate PR: https://github.com/cypress-io/cypress-documentation/blob/develop/source/api/commands/screenshot.md

[NMinhNguyen]

@jennifer-shehane @brian-mann @chrisbreiding thanks a lot for your help thus far. I believe this is now ready for another review 🙂

[sebinsua]

@jennifer-shehane @chrisbreiding We've updated the PR with types and changes related to your comments. (/cc @NMinhNguyen)

[sebinsua]

@jennifer-shehane Will anybody get time to review the code we wrote in response to your comments this week? Also, I've put up a PR for some docs here.

[jennifer-shehane]

@sebinsua Yes, we have your PR on our radar for review.

[chrisbreiding]

@sebinsua I requested a couple changes on the docs PR. Once those are addressed, this should be good to go.

Changes have been addressed and re-reviewed

[jennifer-shehane]

Sorry, we are introducing a few things to our builds that are causing some hiccups, specifically #5312 😢 We'll get it in though.