search

mainmatter / qunit-dom

High Level DOM Assertions for QUnit
MIT License
178 stars 123 forks source link

isVisible triggers on opacity 0 #418

Open Alex-Aralis opened 5 years ago

Alex-Aralis commented 5 years ago

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

This seems wrong.

Turbo87 commented 5 years ago

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

steveszc commented 5 years ago

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 commented 5 years ago

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.

scalvert commented 4 years ago

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 commented 3 years ago

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 commented 11 months ago

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.