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

Pass previous subject through .screenshot() in new screenshot usage #1726

Closed
jennifer-shehane opened this issue May 16, 2018 · 4 comments
Closed
Assignees
Milestone

Comments

@jennifer-shehane
Copy link
Member

The new implementation of .screenshot() from #1504 documents .screenshot() always yielding null.

In the new use case of cy.get('table').screenshot(), I would like the .screenshot() to yield the previous DOM subject, the table.

This would technically be a breaking change - if someone were not following the recommendations within the docs about chaining, but honestly, I couldn't even come up with an example that would work -> then break after these updates.

@chrisbreiding
Copy link
Contributor

After some discussion, we decided to yield the following object:

{
  el, // previous subject if cy.get().screenshot(), else undefined for cy.screenshot()
  path, // path to screenshot
  size, // size of screenshot (bytes)
  dimensions, // { width, height } of screenshot
  takenAt, // ISO8601 date string
  duration, // duration in milliseconds of how long it took
  pixelRatio, // pixel ratio of screenshot (determined by comparing screenshot dimensions w/ viewport dimensions)
  multipart, // boolean as to whether scrolling & stitching was necessary
  ...screenshotConfig, // all the config properties, merged from defaults, user defaults, and overrides passed to cy.screenshot()
}

@brian-mann
Copy link
Member

screenshotConfig should just be options since they are the resolved/merged in values including what's been overridden or passed as options to cy.screenshot

@chrisbreiding
Copy link
Contributor

chrisbreiding commented May 17, 2018

The idea was to have it be a flat object with each option key, hence the .... I just didn't want to list every key in the screenshot config.

@jennifer-shehane jennifer-shehane added stage: pending release and removed stage: pending release stage: investigating Someone from Cypress is looking into this stage: needs investigating Someone from Cypress needs to look at this stage: ready for work The issue is reproducible and in scope labels May 24, 2018
@brian-mann
Copy link
Member

Released in 3.0.0.

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

No branches or pull requests

3 participants