-
Notifications
You must be signed in to change notification settings - Fork 2.3k
docs(expectedconditions): Fix description #3775
Conversation
massimocode
commented
Nov 30, 2016
- See issue Protractor invisibilityOf flakiness, throwing NoSuchElementError #2138 which I just experienced as well because the documentation here (http://www.protractortest.org/#/api?view=ProtractorExpectedConditions.prototype.invisibilityOf) for invisibilityOf said it would support the element not being visible on the DOM, but it doesn't.
The build failed because apparently it stopped responding? Not sure how my changes could have caused that |
@@ -404,8 +403,8 @@ export class ProtractorExpectedConditions { | |||
} | |||
|
|||
/** | |||
* An expectation for checking that an element is either invisible or not | |||
* present on the DOM. This is the opposite of 'visibilityOf'. | |||
* An expectation for checking that an element is invisible on the DOM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should still have "either invisible or not present on the DOM". If invisibilityOf
returns true, that means visibility
returned false. So either presenceOf
is false or isDisplayed
is false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I initially opened this PR exactly to change this piece of text. The minor formatting fix above was just some "gardening".
You are quite right, on closer examination of the source code, the intent of invisibilityOf
is as you say.
However, I have since opened this issue #3777 because invisibilityOf seems to have a race condition, which is why it cannot be used in its current state for waiting for elements to be removed from the DOM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather fix the bug than document it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The text should be changed back.
@@ -404,8 +403,8 @@ export class ProtractorExpectedConditions { | |||
} | |||
|
|||
/** | |||
* An expectation for checking that an element is either invisible or not | |||
* present on the DOM. This is the opposite of 'visibilityOf'. | |||
* An expectation for checking that an element is invisible on the DOM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix.
@@ -404,8 +403,8 @@ export class ProtractorExpectedConditions { | |||
} | |||
|
|||
/** | |||
* An expectation for checking that an element is either invisible or not | |||
* present on the DOM. This is the opposite of 'visibilityOf'. | |||
* An expectation for checking that an element is invisible on the DOM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather fix the bug than document it
Closing this PR as I should create a new PR with the fix @sjelin suggested |