qunit-dom icon indicating copy to clipboard operation
qunit-dom copied to clipboard

isVisible triggers on opacity 0

Open Alex-Aralis opened this issue 6 years ago • 6 comments

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

This seems wrong.

Alex-Aralis avatar Jul 11 '19 15:07 Alex-Aralis

see https://github.com/simplabs/qunit-dom/blob/master/API.md#isvisible

Turbo87 avatar Jul 11 '19 15:07 Turbo87

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).

steveszc avatar Aug 20 '19 18:08 steveszc

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 avatar Aug 23 '19 08:08 Turbo87

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'

scalvert avatar Oct 03 '19 17:10 scalvert

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!?

simonihmig avatar Aug 13 '21 09:08 simonihmig

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.

Techn1x avatar Oct 23 '23 06:10 Techn1x