Skip to content

Commit

Permalink
fix: update visibility for elements with textContent but without widt…
Browse files Browse the repository at this point in the history
…h/height (#29688)
  • Loading branch information
senpl authored Dec 2, 2024
1 parent b2816f9 commit c3660d4
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 14 deletions.
1 change: 1 addition & 0 deletions cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ in this [GitHub issue](https://github.com/cypress-io/cypress/issues/30447). Addr

- Elements with `display: contents` will no longer use box model calculations for visibility, and correctly show as visible when it is visible. Fixed in [#29680](https://github.com/cypress-io/cypress/pull/29680). Fixes [#29605](https://github.com/cypress-io/cypress/issues/29605).
- The CSS pseudo-class `:dir()` is now supported when testing in Electron. Addresses [#29766](https://github.com/cypress-io/cypress/issues/29766).
- Fixed a visibility issue for elements with `textContent` but without a width or height. Fixed in [#29688](https://github.com/cypress-io/cypress/pull/29688). Fixes [#29687](https://github.com/cypress-io/cypress/issues/29687).
- Elements whose parent elements has `overflow: clip` and no height/width will now correctly show as hidden. Fixed in [#29778](https://github.com/cypress-io/cypress/pull/29778). Fixes [#23852](https://github.com/cypress-io/cypress/issues/23852).

**Dependency Updates:**
Expand Down
1 change: 0 additions & 1 deletion packages/driver/cypress/e2e/commands/actions/click.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -2190,7 +2190,6 @@ describe('src/cy/commands/actions/click', () => {

cy.click()
})
// Array(1).fill().map(()=>

it('throws when any member of the subject isnt visible', function (done) {
// sometimes the command will timeout early with
Expand Down
61 changes: 57 additions & 4 deletions packages/driver/cypress/e2e/dom/visibility.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,10 @@ describe('src/cypress/dom/visibility', () => {
})
})

const add = (el) => {
return $(el).appendTo(cy.$$('body'))
}

context('hidden/visible overrides', () => {
beforeEach(function () {
// ensure all tests run against a scrollable window
Expand All @@ -178,8 +182,8 @@ describe('src/cypress/dom/visibility', () => {
this.$parentVisHidden = add('<div class="invis" style="visibility: hidden;"><button>parent visibility: hidden</button></div>')
this.$displayNone = add('<button style="display: none">display: none</button>')
this.$inputHidden = add('<input type="hidden" value="abcdef">')
this.$divNoWidth = add('<div style="width: 0; height: 100px;">width: 0</div>')
this.$divNoHeight = add('<div style="width: 50px; height: 0px;">height: 0</div>')
this.$divNoWidth = add('<div style="width: 0; height: 100px;"></div>')
this.$divNoHeight = add('<div style="width: 50px; height: 0px;"></div>')
this.$divDetached = $('<div>foo</div>')
this.$divVisible = add(`<div>visible</div>`)

Expand Down Expand Up @@ -691,7 +695,6 @@ describe('src/cypress/dom/visibility', () => {
cy.wrap(this.$optionHiddenInSelect.find('#hidden-opt')).should('not.be.visible')
})

// TODO(webkit): fix+unskip
it('follows regular visibility logic if option outside of select', { browser: '!webkit' }, function () {
expect(this.$optionOutsideSelect.find('#option-hidden').is(':hidden')).to.be.true
expect(this.$optionOutsideSelect.find('#option-hidden')).to.be.hidden
Expand Down Expand Up @@ -740,7 +743,57 @@ describe('src/cypress/dom/visibility', () => {
})

describe('width and height', () => {
it('is hidden if offsetWidth is 0', function () {
it('is visible when el.textContent, even if offsetWidth is 0', function () {
this.$divNoWidthTextContent = add('<div style="width: 0; height: 100px;">width: 0</div>')

expect(this.$divNoWidthTextContent.is(':hidden')).to.be.false
expect(this.$divNoWidthTextContent.is(':visible')).to.be.true

expect(this.$divNoWidthTextContent).to.not.be.hidden
expect(this.$divNoWidthTextContent).to.be.visible

cy.wrap(this.$divNoWidthTextContent).should('be.not.hidden')
cy.wrap(this.$divNoWidthTextContent).should('be.visible')
})

it('is visible when el.textContent, even if offsetHeight is 0', function () {
this.$divNoHeightTextContent = add('<div style="width: 50px; height: 0px;">height: 0</div>')

expect(this.$divNoHeightTextContent.is(':hidden')).to.be.false
expect(this.$divNoHeightTextContent.is(':visible')).to.be.true

expect(this.$divNoHeightTextContent).to.not.be.hidden
expect(this.$divNoHeightTextContent).to.be.visible

cy.wrap(this.$divNoHeightTextContent).should('be.not.hidden')
cy.wrap(this.$divNoHeightTextContent).should('be.visible')
})

it('is hidden when when el.textContent contains only whitespace and offsetWidth is 0', function () {
this.$divNoHeightBlankTextContent = add('<div style="width: 0px; height: 50px;"> \n \t </div>')

expect(this.$divNoHeightBlankTextContent.is(':hidden')).to.be.true
expect(this.$divNoHeightBlankTextContent.is(':visible')).to.be.false

expect(this.$divNoHeightBlankTextContent).to.be.hidden
expect(this.$divNoHeightBlankTextContent).to.not.be.visible

cy.wrap(this.$divNoHeightBlankTextContent).should('be.hidden')
cy.wrap(this.$divNoHeightBlankTextContent).should('not.be.visible')
})

it('is hidden when no el.textContent with offsetHeight is 0', function () {
expect(this.$divNoHeight.is(':hidden')).to.be.true
expect(this.$divNoHeight.is(':visible')).to.be.false

expect(this.$divNoHeight).to.be.hidden
expect(this.$divNoHeight).to.not.be.visible

cy.wrap(this.$divNoHeight).should('be.hidden')
cy.wrap(this.$divNoHeight).should('not.be.visible')
})

it('is hidden when no el.textContent with offsetWidth is 0', function () {
expect(this.$divNoWidth.is(':hidden')).to.be.true
expect(this.$divNoWidth.is(':visible')).to.be.false

Expand Down
17 changes: 8 additions & 9 deletions packages/driver/src/dom/visibility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const ensureEl = (el, methodName) => {
}
}

const isStrictlyHidden = (el, methodName = 'isStrictlyHidden()', options = { checkOpacity: true }, recurse?) => {
const isStrictlyHidden = (el: HTMLElement, methodName = 'isStrictlyHidden()', options = { checkOpacity: true }, recurse?) => {
ensureEl(el, methodName)
const $el = $jquery.wrap(el)

Expand Down Expand Up @@ -117,7 +117,7 @@ const isHiddenByAncestors = (el, methodName = 'isHiddenByAncestors()', options =
return elIsOutOfBoundsOfAncestorsOverflow($el)
}

const elHasNoEffectiveWidthOrHeight = ($el) => {
const elHasNoEffectiveWidthOrHeight = ($el: JQuery) => {
// Is the element's CSS width OR height, including any borders,
// padding, and vertical scrollbars (if rendered) less than 0?
//
Expand All @@ -140,10 +140,12 @@ const elHasNoEffectiveWidthOrHeight = ($el) => {
transform = 'none'
}

const hasTextContent = !!el.textContent?.trim().length

const width = elClientWidth($el)
const height = elClientHeight($el)

return isZeroLengthAndTransformNone(width, height, transform) ||
return (isZeroLengthAndTransformNone(width, height, transform) && !hasTextContent) ||
isZeroLengthAndOverflowHidden(width, height, elHasOverflowHidden($el)) ||
(el.getClientRects().length <= 0)
}
Expand All @@ -153,21 +155,18 @@ const isZeroLengthAndTransformNone = (width, height, transform) => {
// we learned that when an element has non-'none' transform style value like "translate(0, 0)",
// it is visible even with `height: 0` or `width: 0`.
// That's why we're checking `transform === 'none'` together with elClientWidth/Height.

return (width <= 0 && transform === 'none') ||
(height <= 0 && transform === 'none')
return (width <= 0 && transform === 'none') || (height <= 0 && transform === 'none')
}

const isZeroLengthAndOverflowHidden = (width, height, overflowHidden) => {
return (width <= 0 && overflowHidden) ||
(height <= 0 && overflowHidden)
return (width <= 0 && overflowHidden) || (height <= 0 && overflowHidden)
}

const elHasNoClientWidthOrHeight = ($el) => {
return (elClientWidth($el) <= 0) || (elClientHeight($el) <= 0)
}

const elementBoundingRect = ($el) => $el[0].getBoundingClientRect()
const elementBoundingRect = ($el: JQuery) => $el[0].getBoundingClientRect()

const elClientHeight = ($el) => elementBoundingRect($el).height

Expand Down

0 comments on commit c3660d4

Please sign in to comment.