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

Opacity is not considered for visibility checks #4474

Closed
danielkcz opened this issue Jun 17, 2019 · 6 comments · Fixed by #8244
Closed

Opacity is not considered for visibility checks #4474

danielkcz opened this issue Jun 17, 2019 · 6 comments · Fixed by #8244
Assignees
Labels
pkg/driver This is due to an issue in the packages/driver directory topic: visibility 👁 type: breaking change Requires a new major release version

Comments

@danielkcz
Copy link

danielkcz commented Jun 17, 2019

Current behavior:

image

Desired behavior:

When opacity === 0, the element should be considered as not visible to avoid evaluating the next command sooner than expected and causing flakiness.

Steps to reproduce: (app code and test code)

I think the issue is pretty clear from the image, but I can provide reproduction later if you consider it necessary.

Versions

[email protected]
Electron 61
Windows 10 x64

@jennifer-shehane
Copy link
Member

Nice. Thanks for opening this issue. Was going to do this myself.

I am currently working on visibility fixes - one of which was implementing this change for opacity. We had initially decided not to consider elements with opacity 0 as 'not visible', because they are still interact-able, but we're going to be pulling this all out into a broader issue so that there can be different checks for 'can be seen', 'can be seen in viewpor', and 'can be interacted with'

Read more here: #1242

Some of our previous reasoning: #677

I actually have already written the code for the opacity check (commented out in this PR #4421) - as this would be implemented as part of the 'can be seen' code, but I think due to the change, this would have to go into a major code version update as it would be breaking our users current expectations.

Workaround Today

Make sure that 0 is a string in your assertion, not a number - as all css properties evaluate to strings.

cy.get('div').should('have.css', 'opacity', '0')

@jennifer-shehane jennifer-shehane added type: breaking change Requires a new major release version topic: visibility 👁 labels Jun 17, 2019
@jennifer-shehane jennifer-shehane added the pkg/driver This is due to an issue in the packages/driver directory label Jun 17, 2019
@mvriel
Copy link

mvriel commented Jan 2, 2020

We had initially decided not to consider elements with opacity 0 as 'not visible', because they are still interact-able

I have actually worked on a piece of code that also set the pointer-events to none; effectively making it non-interactable. This was done to have an invisible element fade in using CSS transitions. (display: none will kill the fade)

@asciidiego
Copy link

@jennifer-shehane I think that the fact that opacity: 0 is considered visible due to the interactivity, is a fairly good argument, as long as two things are taken into account:

  1. The user can modify this behaviour with some option argument, ie, just like we have timeout, we could have opacity-is-visible (bad name, but good for the example).
  2. This is properly documented in the visibility section of the interaction guide, which is not done nowadays, it does not even mention it.

What do you think?

@cypress-bot cypress-bot bot added stage: proposal 💡 No work has been done of this issue and removed stage: work in progress labels Apr 21, 2020
@cypress-bot cypress-bot bot added stage: work in progress stage: needs review The PR code is done & tested, needs review and removed stage: proposal 💡 No work has been done of this issue stage: work in progress stage: needs review The PR code is done & tested, needs review labels Aug 10, 2020
@cypress-bot cypress-bot bot added stage: work in progress and removed stage: needs review The PR code is done & tested, needs review labels Aug 21, 2020
@sync-by-unito sync-by-unito bot assigned flotwig and unassigned panzarino Aug 27, 2020
@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review and removed stage: work in progress labels Aug 27, 2020
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Aug 28, 2020

The code for this is done in cypress-io/cypress#8244, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@cypress-bot cypress-bot bot added stage: pending release and removed stage: needs review The PR code is done & tested, needs review labels Aug 28, 2020
@jennifer-shehane
Copy link
Member

This fix requires a breaking change, so is targeted for our next breaking change release. You can follow along with progress on this release here: #8437

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Nov 24, 2020

Released in 6.0.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v6.0.0, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pkg/driver This is due to an issue in the packages/driver directory topic: visibility 👁 type: breaking change Requires a new major release version
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants