feat: improve assertView command behavior #1112
Conversation
commit: |
shadowusr
force-pushed
the
users/shadowusr/TESTPLANE-632
branch
from
d8911a4 to
6b48ea1
Compare
shadowusr
force-pushed
the
users/shadowusr/TESTPLANE-632
branch
from
d459a9e to
b68fa42
Compare
KuznetsovRoman
left a comment
KuznetsovRoman
left a comment
There was a problem hiding this comment.
LGTM
Logic is hard, though, it is expected, when dealing with screenshot logic
|
|
||
| var interferingRects = []; | ||
|
|
||
| allElements.forEach(function (el) { |
There was a problem hiding this comment.
This operation looks very heavy
Did you check "how much time it takes" on page with large amount of nodes?
There was a problem hiding this comment.
Doesn't look too heavy to me... Average page has 5-10k elements, that's just 10k iterations with pretty cheap logic inside.
I've tried measuring how much time we spend on this on different pages and turns out it's between 20ms to 80ms.
But I think we can add an option to disable this logic in the future
| Math.min(nextNotCapturedArea.height, page.safeArea.height), | ||
| 2 * page.pixelRatio, | ||
| ); | ||
| const logicalScrollHeight = Math.ceil(physicalScrollHeight / page.pixelRatio) - 1; |
There was a problem hiding this comment.
Added a comment. That's to account for fractional top edge of scroll container. If it's fractional, without -1 we'd get 1px lines on resulting screenshot. But this way, we'll never accidentally capture that top fractional edge.
shadowusr
force-pushed
the
users/shadowusr/TESTPLANE-632
branch
from
588cb86 to
adbeabc
Compare
✅ Testplane run succeed
|
shadowusr
changed the title
No description provided.