Skip to content

Commit

Permalink
replace instances of $.fn.width/height with respective dom APIs
Browse files Browse the repository at this point in the history
jQuery 3 changed how it does width and height calculations (it now uses getBoundingClientRect), which takes into account the scaling and returns non-integers. We want the absolute integer value without scaling, so we're better off directly using the DOM properties.
  • Loading branch information
chrisbreiding committed Sep 24, 2019
1 parent d900c32 commit 72c543e
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 17 deletions.
8 changes: 4 additions & 4 deletions packages/driver/src/cy/commands/screenshot.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ $utils = require("../../cypress/utils")

getViewportHeight = (state) ->
## TODO this doesn't seem correct
Math.min(state("viewportHeight"), $(window).height())
Math.min(state("viewportHeight"), window.innerHeight)

getViewportWidth = (state) ->
Math.min(state("viewportWidth"), $(window).width())
Math.min(state("viewportWidth"), window.innerWidth)

automateScreenshot = (state, options = {}) ->
{ runnable, timeout } = options
Expand Down Expand Up @@ -238,8 +238,8 @@ takeScreenshot = (Cypress, state, screenshotConfig, options = {}) ->
}
userClip: clip
viewport: {
width: $(window).width()
height: $(window).height()
width: window.innerWidth
height: window.innerHeight
}
scaled: getShouldScale(screenshotConfig)
blackout: getBlackout(screenshotConfig)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,8 @@ describe "src/cy/commands/screenshot", ->
.screenshot()
.then ->
expect(Cypress.automation.withArgs("take:screenshot").args[0][1].viewport).to.eql({
width: $(window.parent).width()
height: $(window.parent).height()
width: window.parent.innerWidth
height: window.parent.innerHeight
})

it "can handle window w/length > 1 as a subject", ->
Expand Down
14 changes: 10 additions & 4 deletions packages/runner/src/app/app.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,21 +74,21 @@ class App extends Component {

_monitorWindowResize () {
const state = this.props.state
const win = this.props.window

const $window = $(this.props.window)
const $header = $(findDOMNode(this.refs.header))
const $reporterWrap = $(this.refs.reporterWrap)

this._onWindowResize = () => {
state.updateWindowDimensions({
windowWidth: $window.width(),
windowHeight: $window.height(),
windowWidth: win.innerWidth,
windowHeight: win.innerHeight,
reporterWidth: $reporterWrap.outerWidth(),
headerHeight: $header.outerHeight(),
})
}

$window.on('resize', this._onWindowResize).trigger('resize')
$(win).on('resize', this._onWindowResize).trigger('resize')
}

_onReporterResizeStart = () => {
Expand All @@ -98,6 +98,12 @@ class App extends Component {
_onReporterResize = (reporterWidth) => {
this.props.state.reporterWidth = reporterWidth
this.props.state.absoluteReporterWidth = reporterWidth

const $header = $(findDOMNode(this.refs.header))

this.props.state.updateWindowDimensions({
headerHeight: $header.outerHeight(),
})
}

_onReporterResizeEnd = () => {
Expand Down
13 changes: 6 additions & 7 deletions packages/runner/src/lib/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,8 @@ function dimensionsMatchPreviousLayer (obj, container) {
return
}

return obj.width === previousLayer.width() &&
obj.height === previousLayer.height()
return obj.width === previousLayer[0].offsetWidth &&
obj.height === previousLayer[0].offsetHeight
}

function getDimensionsFor (dimensions, attr, dimension) {
Expand All @@ -255,14 +255,13 @@ function getZIndex (el) {
}

return _.toNumber(el.css('zIndex'))

}

function getElementDimensions ($el) {
const dimensions = {
offset: $el.offset(), // offset disregards margin but takes into account border + padding
height: $el.height(), // we want to use height here (because that always returns just the content hight) instead of .css() because .css('height') is altered based on whether box-sizing: border-box is set
width: $el.width(),
height: $el[0].offsetHeight, // we want to use offsetHeight here (because that always returns just the content hight) instead of .css() because .css('height') is altered based on whether box-sizing: border-box is set
width: $el[0].offsetWidth,
paddingTop: getPadding($el, 'top'),
paddingRight: getPadding($el, 'right'),
paddingBottom: getPadding($el, 'bottom'),
Expand Down Expand Up @@ -338,8 +337,8 @@ function isInViewport (win, el) {
return (
rect.top >= 0 &&
rect.left >= 0 &&
rect.bottom <= $(win).height() &&
rect.right <= $(win).width()
rect.bottom <= win.innerHeight &&
rect.right <= win.innerWidth
)
}

Expand Down

0 comments on commit 72c543e

Please sign in to comment.