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

isVisible triggers on opacity 0 #418

Open
Alex-Aralis opened this issue Jul 11, 2019 · 6 comments
Open

isVisible triggers on opacity 0 #418

Alex-Aralis opened this issue Jul 11, 2019 · 6 comments
Labels

Comments

@Alex-Aralis
Copy link

The current isVisible considers elements with opacity: 0 to be visible.

This seems wrong.

@Turbo87
Copy link
Collaborator

Turbo87 commented Jul 11, 2019

@steveszc
Copy link

The current implementation of isVisible is based on jquery's :visible pseudo-selector which states the following:

Elements are considered visible if they consume space in the document. Visible elements have a width or height that is greater than zero.

Elements with visibility: hidden or opacity: 0 are considered visible, since they still consume space in the layout.

source

I can see definite utility in altering isVisible (or introducing a new assertion) to also check for opacity: 0 and visibility: hidden (maybe other properties I'm not thinking of).

@Turbo87
Copy link
Collaborator

Turbo87 commented Aug 23, 2019

the problem is that there are soooo many ways to make an element invisible that it's not really viable to support all of them. it's correct that the current implementation is based on the jQuery implementation, but apparently that is not quite sufficient. I'm open to adjusting the logic, but that would most likely have to be a breaking change then.

@Turbo87 Turbo87 added the bug label Aug 23, 2019
@scalvert
Copy link
Collaborator

scalvert commented Oct 3, 2019

We could also consider having options to isVisible, whereby you'd specify that you want to also assert on additional properties (opacity: 0, visibility: hidden). Or, we could have a completely separate assertion that is for CSS visibility:

isCSSVisible - passes if opacity !== 0 and visibility !== 'hidden'
isNotCSSVisible - passes if opacity === 0 or `visibility === 'hidden'

@simonihmig
Copy link
Contributor

Just stumbled upon this. I was also expected opacity to be taken into account.

Elements with visibility: hidden or opacity: 0 are considered visible, since they still consume space in the layout.

Tbh, that definition of visibility totally contradicts my "common sense" understanding of what visible is! If a button consumes space, but I cannot see it nor read its text, I would certainly not consider it visible! 😆
But let's not bike shed about this...

I did however remember that Selenium/Webdriver based solutions handle visibility in a IMHO smarter way. Quick google found this: https://github.com/thefrontside/element-is-visible. Maybe we can defer the decision what visible means to this (quite elaborate) util, which seems to mimic the Selenium semantics!?

@Techn1x
Copy link

Techn1x commented Oct 23, 2023

I have just been bitten by this as well (visibility: hidden; on my element, yet isVisible() / isNotVisible() thinks it is visible)

My 2c; I think that the current implementation of isVisible() / isNotVisible() is very misleading, no matter how good the docs are, and definitely leads to invalid tests being written where isVisible() will always succeed, especially in applications that do not use jquery. If not implemented to cover the obvious scenarios (opacity & visiblity styles), then it should probably be removed or at least deprecated.. force the dev to check visibility with other assertion methods like hasClass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants