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

Proposal - cy.screenshot enhancements #1424

Closed
brian-mann opened this issue Mar 6, 2018 · 9 comments
Closed

Proposal - cy.screenshot enhancements #1424

brian-mann opened this issue Mar 6, 2018 · 9 comments
Assignees
Labels
Epic Requires breaking up into smaller issues
Milestone

Comments

@brian-mann
Copy link
Member

brian-mann commented Mar 6, 2018

Make cy.screenshot() more useful to go down the path towards screenshot diffing.

Let's create a new set of API's to make it easier to take a picture of the AUT (application under test) without involving the Cypress UI.

To make this happen, we'll also need to give the user the option to remove the artificial "scaling" Cypress applies, and we'll need to black out all of the excess areas.

Bonus points for also going down the route of slicing out the "blacked out" areas of the picture so the resulting screenshot has no excess - and is insulated from resolution differences.

This also leads us towards being able to take pictures of just dom elements instead of the whole window.

Cypress.Screenshot.defaults({
  capture: 'commands | app | both', // default 'commands'
  waitForCommandSynchronization: true, // when taking a picture of the commands, wait until they are synchronized
  scaleAppCaptures: false, // when taking a picture of the app, remove the scaling
  disableTimersAndAnimations: true, // prevent app animations and timers from firing
  screenshotOnRunFailure: true, // config.screenshotOnRunFailure
  blackout: ['selectors'], // whatever matches, we'll blackout their box model
  onScreenshot: ($dom) => {

  },
})

To disable animations - we need to keep references for setTimeout, setInterval, requestAnimationFrame during the initial phase of event binding to the window - and then prevent callbacks from firing until the screenshot has been taken.

onScreenshot would yield you the $elements that are being screenshotted - enabling the user to synchronously perform changes on it (like changing dynamic content).

blackout allows the user to easily pick selectors which we'd use to blackout their box model. This would utilize the logic for layering hitboxes (except layer only a blacked out piece)

@brian-mann
Copy link
Member Author

brian-mann commented Mar 6, 2018

To disable CSS animations insert this <style> into the document:

*, *:before, *:after {
  transition-property: none !important;
  transform: none !important;
  animation: none !important;
}

@brian-mann
Copy link
Member Author

After screenshot perform all the cleanup - remove the CSS, blackouts, and revert timers.

@jennifer-shehane jennifer-shehane added Epic Requires breaking up into smaller issues stage: in progress labels Mar 6, 2018
@chrisbreiding
Copy link
Contributor

After some discussion, changing capture to be an array:

Cypress.Screenshot.defaults({
  capture: ['app', 'everything']
})

Still need a decent name for 'everything'. Ideas:

  • screen
  • all
  • ui
  • app+ui
  • viewport

Also include 'app-scaled' as an option? Or, another idea is to accept an object instead of a string for each item (maybe called it screenshots instead of capture?):

Cypress.Screenshot.defaults({
  screenshots: [
    {
      capture: 'app',
      scale: false
    },
    {
      capture: 'everything',
      scale: true
    }
  ]
})

Perhaps the user could still use the string 'app' or 'everything' and it defaults to scale: true? Then they only need the object if they're turning off scaling.

@brian-mann, @jennifer-shehane, @bahmutov, thoughts?

@chrisbreiding
Copy link
Contributor

chrisbreiding commented Apr 16, 2018

Latest API:

Cypress.Screenshot.defaults({
  capture: 'app', // or 'runner'
  waitForCommandSynchronization: true,
  scaleAppCaptures: false,
  disableTimersAndAnimations: true,
  screenshotOnRunFailure: true,
  blackout: ['selectors'],
  beforeScreenshot: (document) => {},
  afterScreenshot: (document) => {},
})

By default, taking 'app' captures with cy.screenshot(). Only take 'runner' captures on failures. Don't black out anything on 'runner' captures.

@chrisbreiding
Copy link
Contributor

chrisbreiding commented Apr 17, 2018

Some additions:

Allow capturing full page

Add 'fullpage' option for capture.

cy.screenshot({
  capture: 'fullpage'
})

Implement it by scrolling to the top, then taking a screenshot, scrolling, taking a screenshot, and so on, then stitching them together.

When we implement native events, we'll be able to use Chrome's debugger protocol to accomplish this in a better, more succinct way. Its implementation utilizes device emulation overrides. See puppeteer's implementation.

Add element capture support

Make cy.screenshot() a dual command, so you can do:

cy.get('.foo').screenshot()

Implement as follows:

  1. Scroll element into view
  2. Take screenshot, crop out element
  3. Scroll if needed and stitch together

Number 3 will be unnecessary for Chrome once implemented with debugger protocol. See puppeteer's implementation.

@chrisbreiding
Copy link
Contributor

Some more changes to this API before it's released:

  • Removing waitForCommandSynchronization option. Will always be true for runner captures and false for non-runner captures
  • Changing capture: 'app' to capture: 'viewport'
  • Changing scaleAppCaptures: true to scale: true

@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 Author

Released in 3.0.0.

@ModiYesha
Copy link

Can Screenshot give us DOM which can be stored in JSON format the way snapshot https://github.com/cypress-io/snapshot gives?

@jennifer-shehane
Copy link
Member

@ModiYesha No.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Epic Requires breaking up into smaller issues
Projects
None yet
Development

No branches or pull requests

4 participants