Provide selector methods for filtering nodes by visibility (closes #1018) #2093
Conversation
|
❌ Tests for the commit e5a5b85 have failed. See details: |
|
❌ Tests for the commit e73867a have failed. See details: |
|
✅ Tests for the commit e73867a have passed. See details: |
AndreyBelym
changed the title
| import ClientFunctionExecutor from '../client-function-executor'; | ||
| import { exists, visible } from '../visibility-function'; |
There was a problem hiding this comment.
Maybe it's better to rename visibility-function.js to something like element-constraints.js? \cc @AlexanderMoskovkin
There was a problem hiding this comment.
I doubt about the constraints word but I agree with the idea to rename this since exists is not visibility function.
Maybe something like check-element or element-utils
|
✅ Tests for the commit e73867a have passed. See details: |
| @@ -41,6 +42,14 @@ hammerhead.nativeMethods.objectDefineProperty.call(window, window, '%testCafeSel | |||
| else | |||
| throw new InvalidSelectorResultError(); | |||
|
|
|||
| filtered = [].slice.call(filtered).filter(n => exists(n)); | |||
There was a problem hiding this comment.
It's better to use arrayUtils.filter, like here. It this case you need not to use the [].slice.call.
One more reason: Array.prototype.slice can be overwritten by a script on the tested page. In this case your script will fail. But arrayUtils contains initial functions from Array.prototype (like filter, forEach etc..)
| import ClientFunctionExecutor from '../client-function-executor'; | ||
| import { exists, visible } from '../visibility-function'; |
There was a problem hiding this comment.
I doubt about the constraints word but I agree with the idea to rename this since exists is not visibility function.
Maybe something like check-element or element-utils
| @@ -15,6 +15,34 @@ const isIEFunction = ClientFunction(() => { | |||
| appName === 'Netscape' && isIE11Re.exec(userAgent) !== null; | |||
| }); | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
Let's place this tests near other filter tests in the file (for example after the Combination of filter methods test).
|
|
||
| elements = elements.filterVisible().child('p'); | ||
|
|
||
| await t.expect(elements.count).eql(6); |
There was a problem hiding this comment.
...
<p class="class1">2</p>
<p class="class1">3</p>
<p class="class1" style="display: none;">4</p>
...Let's add a check for a compound filtered selector. For example:
await t.expect(elements.filterVisible().filter('.class1').count).eql(2);
|
❌ Tests for the commit 0d0f949 have failed. See details: |
|
@testcafe-build-bot \retest |
|
❌ Tests for the commit 0d0f949 have failed. See details: |
|
@testcafe-build-bot \retest |
|
❌ Tests for the commit 0d0f949 have failed. See details: |
1 similar comment
|
❌ Tests for the commit 0d0f949 have failed. See details: |
|
✅ Tests for the commit 0d0f949 have passed. See details: |
|
FPR |
|
@testcafe-build-bot \retest |
|
❌ Tests for the commit 0d0f949 have failed. See details: |
1 similar comment
|
❌ Tests for the commit 0d0f949 have failed. See details: |
|
✅ Tests for the commit 0d0f949 have passed. See details: |
|
/cc @AndreyBelym |
|
@AndreyBelym please review |
No description provided.