Add padding support to element#screenshot

Closes cypress-io#4440 Co-authored-by: Minh Nguyen <[email protected]> Co-authored-by: Jennifer Shehane <[email protected]>
sebinsua · Sep 26, 2019 · f2e21c6 · f2e21c6
1 parent f7eeba6
commit f2e21c6
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 54 deletions.
diff --git a/packages/driver/src/cy/commands/screenshot.coffee b/packages/driver/src/cy/commands/screenshot.coffee
@@ -81,12 +81,13 @@ scrollOverrides = (win, doc) ->
       doc.body.style.overflowY = originalBodyOverflowY
     win.scrollTo(originalX, originalY)
 
-takeScrollingScreenshots = (scrolls, win, state, automationOptions) ->
-  if scrolls.length is 0
-    return Promise.reject(
-      new Error("It was not possible to take a screenshot, since the number of scrolls calculated to do so was zero.")
-    )
+validateNumScreenshots = (numScreenshots, automationOptions) ->
+  if numScreenshots < 1
+    $utils.throwErrByPath("screenshot.invalid_height", {
+      log: automationOptions.log
+    })
 
+takeScrollingScreenshots = (scrolls, win, state, automationOptions) ->
   scrollAndTake = ({ y, clip, afterScroll }, index) ->
     win.scrollTo(0, y)
     if afterScroll
@@ -113,6 +114,8 @@ takeFullPageScreenshot = (state, automationOptions) ->
   viewportHeight = getViewportHeight(state)
   numScreenshots = Math.ceil(docHeight / viewportHeight)
 
+  validateNumScreenshots(numScreenshots, automationOptions)
+
   scrolls = _.map _.times(numScreenshots), (index) ->
     y = viewportHeight * index
     clip = if index + 1 is numScreenshots
@@ -131,51 +134,48 @@ takeFullPageScreenshot = (state, automationOptions) ->
   takeScrollingScreenshots(scrolls, win, state, automationOptions)
   .finally(resetScrollOverrides)
 
-getElementWithPadding = ($el, doc, { padding }) ->
-  if not padding
-    return $el
-
-  [ paddingTop, paddingRight, paddingBottom, paddingLeft ] = padding
+applyPaddingToElementPositioning = (elPosition, automationOptions) ->
+  if not automationOptions.padding
+    return elPosition
 
-  originalElPosition = $dom.getElementPositioning($el)
+  [ paddingTop, paddingRight, paddingBottom, paddingLeft ] = automationOptions.padding
 
-  width = originalElPosition.width
-  height = originalElPosition.height
-  top = originalElPosition.fromWindow.top
-  left = originalElPosition.fromWindow.left
-
-  elWithPadding = doc.createElement('div')
-  elWithPadding.style.position = 'absolute'
-  elWithPadding.style.visibility = 'hidden'
-  elWithPadding.style.width = "#{width + paddingLeft + paddingRight}px"
-  elWithPadding.style.height = "#{height + paddingTop + paddingBottom}px"
-  elWithPadding.style.top = "#{top - paddingTop}px"
-  elWithPadding.style.left = "#{left - paddingLeft}px"
-
-  doc.body.appendChild(elWithPadding)
-
-  $(elWithPadding)
+  return {
+    width: elPosition.width + paddingLeft + paddingRight
+    height: elPosition.height + paddingTop + paddingBottom
+    fromViewport: {
+      top: elPosition.fromViewport.top - paddingTop
+      left: elPosition.fromViewport.left - paddingLeft
+      bottom: elPosition.fromViewport.bottom + paddingBottom
+    }
+    fromWindow: {
+      top: elPosition.fromWindow.top - paddingTop
+    }
+  }
 
-takeElementScreenshot = ($originalEl, state, automationOptions) ->
+takeElementScreenshot = ($el, state, automationOptions) ->
   win = state("window")
   doc = state("document")
 
   resetScrollOverrides = scrollOverrides(win, doc)
 
-  viewportHeight = getViewportHeight(state)
-  viewportWidth = getViewportWidth(state)
-  $el = getElementWithPadding(
-    $originalEl
-    doc
+  elPosition = applyPaddingToElementPositioning(
+    $dom.getElementPositioning($el),
     automationOptions
   )
-  elPosition = $dom.getElementPositioning($el)
+  viewportHeight = getViewportHeight(state)
+  viewportWidth = getViewportWidth(state)
   numScreenshots = Math.ceil(elPosition.height / viewportHeight)
 
+  validateNumScreenshots(numScreenshots, automationOptions)
+
   scrolls = _.map _.times(numScreenshots), (index) ->
     y = elPosition.fromWindow.top + (viewportHeight * index)
     afterScroll = ->
-      elPosition = $dom.getElementPositioning($el)
+      elPosition = applyPaddingToElementPositioning(
+        $dom.getElementPositioning($el),
+        automationOptions
+      )
       x = Math.min(viewportWidth, elPosition.fromViewport.left)
       width = Math.min(viewportWidth - x, elPosition.width)
 
@@ -208,11 +208,7 @@ takeElementScreenshot = ($originalEl, state, automationOptions) ->
     { y, afterScroll }
 
   takeScrollingScreenshots(scrolls, win, state, automationOptions)
-  .finally(() ->
-    resetScrollOverrides()
-    if $el isnt $originalEl
-      doc.body.removeChild($el[0])
-  )
+  .finally(resetScrollOverrides)
 
 ## "app only" means we're hiding the runner UI
 isAppOnly = ({ capture }) ->

diff --git a/packages/driver/src/cypress/error_messages.coffee b/packages/driver/src/cypress/error_messages.coffee
@@ -774,7 +774,8 @@ module.exports = {
     invalid_boolean: "{{cmd}}() '{{option}}' option must be a boolean. You passed: {{arg}}"
     invalid_blackout: "{{cmd}}() 'blackout' option must be an array of strings. You passed: {{arg}}"
     invalid_clip: "{{cmd}}() 'clip' option must be an object with the keys { width, height, x, y } and number values. You passed: {{arg}}"
-    invalid_padding: "{{cmd}}() 'padding' option must be either a number or an array of numbers with a length between 1 and 4. You passed: {{arg}}"
+    invalid_height: "{{cmd}}() only works with a screenshot area whose height is greater than zero."
+    invalid_padding: "{{cmd}}() 'padding' option must be either a number or an array of numbers with a maximum length of 4. You passed: {{arg}}"
     invalid_callback: "{{cmd}}() '{{callback}}' option must be a function. You passed: {{arg}}"
     multiple_elements: "#{cmd('screenshot')} only works for a single element. You attempted to screenshot {{numElements}} elements."
     timed_out: "#{cmd('screenshot')} timed out waiting '{{timeout}}ms' to complete."

diff --git a/packages/driver/src/cypress/screenshot.coffee b/packages/driver/src/cypress/screenshot.coffee
@@ -17,8 +17,7 @@ defaults = reset()
 validCaptures = ["fullPage", "viewport", "runner"]
 
 normalizePadding = (padding) ->
-  if not padding
-    padding = 0
+  padding ||= 0
 
   if _.isArray(padding)
     # CSS shorthand
@@ -38,8 +37,6 @@ normalizePadding = (padding) ->
         right = padding[1]
         bottom = padding[2]
         left = padding[3]
-      else
-        top = right = bottom = left = 0
   else
     top = right = bottom = left = padding
 
@@ -129,7 +126,13 @@ validate = (props, cmd, log) ->
     values.clip = clip
 
   if padding = props.padding
-    if not (_.isFinite(padding) or (_.isArray(padding) and padding.length >=1 && padding.length <= 4 and _.every(padding, _.isFinite)))
+    if not (
+      _.isFinite(padding) or
+      (_.isArray(padding) and
+        padding.length >= 1 and
+        padding.length <= 4 and
+        _.every(padding, _.isFinite))
+    )
       $utils.throwErrByPath("screenshot.invalid_padding", {
         log: log
         args: { cmd: cmd, arg: $utils.stringify(padding) }

diff --git a/packages/driver/test/cypress/integration/commands/screenshot_spec.coffee b/packages/driver/test/cypress/integration/commands/screenshot_spec.coffee
@@ -704,24 +704,24 @@ describe "src/cy/commands/screenshot", ->
         cy.screenshot({ blackout: [true] })
 
       it "throws if there is a 0px tall element height", (done) ->
-        @assertErrorMessage("It was not possible to take a screenshot, since the number of scrolls calculated to do so was zero.", done)
+        @assertErrorMessage("cy.screenshot() only works with a screenshot area whose height is greater than zero.", done)
         cy.visit("/fixtures/screenshots.html")
         cy.get('.empty-element').screenshot()
 
       it "throws if padding is not a number", (done) ->
-        @assertErrorMessage("cy.screenshot() 'padding' option must be either a number or an array of numbers with a length between 1 and 4. You passed: 50px", done)
+        @assertErrorMessage("cy.screenshot() 'padding' option must be either a number or an array of numbers with a maximum length of 4. You passed: 50px", done)
         cy.screenshot({ padding: '50px' })
 
       it "throws if padding is not an array of numbers", (done) ->
-        @assertErrorMessage("cy.screenshot() 'padding' option must be either a number or an array of numbers with a length between 1 and 4. You passed: bad, bad, bad, bad", done)
+        @assertErrorMessage("cy.screenshot() 'padding' option must be either a number or an array of numbers with a maximum length of 4. You passed: bad, bad, bad, bad", done)
         cy.screenshot({ padding: ['bad', 'bad', 'bad', 'bad'] })
 
       it "throws if padding is not an array with a length between 1 and 4", (done) ->
-        @assertErrorMessage("cy.screenshot() 'padding' option must be either a number or an array of numbers with a length between 1 and 4. You passed: 20, 10, 20, 10, 50", done)
+        @assertErrorMessage("cy.screenshot() 'padding' option must be either a number or an array of numbers with a maximum length of 4. You passed: 20, 10, 20, 10, 50", done)
         cy.screenshot({ padding: [20, 10, 20, 10, 50] })
 
       it "throws if padding is a large negative number that causes a 0px tall element height", (done) ->
-        @assertErrorMessage("It was not possible to take a screenshot, since the number of scrolls calculated to do so was zero.", done)
+        @assertErrorMessage("cy.screenshot() only works with a screenshot area whose height is greater than zero.", done)
         cy.visit("/fixtures/screenshots.html")
         cy.get('.tall-element').screenshot({ padding: -161 })
 

diff --git a/packages/driver/test/cypress/integration/cypress/screenshot_spec.coffee b/packages/driver/test/cypress/integration/cypress/screenshot_spec.coffee
@@ -141,13 +141,13 @@ describe "src/cypress/screenshot", ->
       it "throws if padding is not a number or an array of numbers with a length between 1 and 4", ->
         expect =>
           Screenshot.defaults({ padding: '50px' })
-        .to.throw("Cypress.Screenshot.defaults() 'padding' option must be either a number or an array of numbers with a length between 1 and 4. You passed: 50px")
+        .to.throw("Cypress.Screenshot.defaults() 'padding' option must be either a number or an array of numbers with a maximum length of 4. You passed: 50px")
         expect =>
           Screenshot.defaults({ padding: ['bad', 'bad', 'bad', 'bad'] })
-        .to.throw("Cypress.Screenshot.defaults() 'padding' option must be either a number or an array of numbers with a length between 1 and 4. You passed: bad, bad, bad, bad")
+        .to.throw("Cypress.Screenshot.defaults() 'padding' option must be either a number or an array of numbers with a maximum length of 4. You passed: bad, bad, bad, bad")
         expect =>
           Screenshot.defaults({ padding: [20, 10, 20, 10, 50] })
-        .to.throw("Cypress.Screenshot.defaults() 'padding' option must be either a number or an array of numbers with a length between 1 and 4. You passed: 20, 10, 20, 10, 50")
+        .to.throw("Cypress.Screenshot.defaults() 'padding' option must be either a number or an array of numbers with a maximum length of 4. You passed: 20, 10, 20, 10, 50")
 
       it "throws if clip is lacking proper keys", ->
         expect =>