fix: fix remaining issues in new assertView algorithm and implement cropMargins options#1277
Merged
shadowusr merged 21 commits intoJun 15, 2026
Merged
Conversation
shadowusr
changed the title
✅ Testplane browser-env run succeed
|
✅ Testplane E2E run succeed
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 096821c8be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| viewportSize: page.viewportSize, | ||
| viewportOffset: currentState.viewportOffset, | ||
| screenshotDelay: opts.screenshotDelay, | ||
| cropMargins: opts.cropMargins, |
There was a problem hiding this comment.
When cropMargins is not provided, this still adds an own cropMargins: undefined property to the camera call. The existing unit test in test/src/browser/screen-shooter/index.js asserts this call with calledOnceWithExactly and only expects viewportSize, viewportOffset, and screenshotDelay, so the normal screenshot path now breaks that test unless the property is omitted when unset or the expectation is updated.
Useful? React with 👍 / 👎.
…ctors to stabilize
…e elements are out of viewport
…s with fixed/sticky/absolute elements participating in capture
… area settle wait
…d of on the whole safe area size to prevent infinite rollback-scroll trap
…tween safe area and capture area
…ite and unit tests
shadowusr
force-pushed
the
users/shadowusr/TESTPLANE-672.fix-issues-3
branch
from
096821c to
2b88b5b
Compare
commit: |
KuznetsovRoman
approved these changes
Jun 15, 2026
shadowusr
added a commit
that referenced
this pull request
Jun 16, 2026
…ropMargins options (#1277) * fix: handle a case when setTimeout is stubbed during waiting for selectors to stabilize * fix: fix safe area computation for fixed blocks * fix: disable animations and hovers in page screenshots by default * fix: do not move pointer on mobile devices, because it causes some browsers to freeze * fix: handle a case when capture area needs to be expanded, but capture elements are out of viewport * fix: fix handling of sticky/fixed positioned elements inside capture elements * fix: when computing average shift, only take non-zero shifts into account * fix: print helpful message if element to capture is hidden/disappeared mid-capture * fix: ignore transparent blocks via css filter opacity when computing safe area * fix: capture fixed-positioned elements and fix various inconsistencies with fixed/sticky/absolute elements participating in capture * fix: handle missing timeout value in safari simulators during capture area settle wait * fix: rollback only on needed amount of px if safe area shrinks instead of on the whole safe area size to prevent infinite rollback-scroll trap * fix: use correct computation to determine instersection percentage between safe area and capture area * fix: revert introducing synthetic capture specs for fixed-positioned descendants * feat: implement cropMargins options * chore: enable screenshot verbose logging only with TESTPLANE_DEBUG_SCREENSHOTS * fix: fix zero maxDelta non-renderable capture spec case during composite and unit tests * test: fix unit tests * fix: fix review issues * fix!: set default tolerance value to 3 and ignoreDiffPixelCount to 4 * docs: actualize screenshots dev readme
shadowusr
added a commit
that referenced
this pull request
Jun 16, 2026
* Revert "fix: calibrate errors" This reverts commit 431d8a5. * feat!: implement new assertView version * fix: fix image decoding issues and safe area computation * fix: revert rounding changes * feat: implement disableHover, improve safeArea computation and other assertView fixes (#1205) * feat: implement the disableHover option and refactor existing-browser * fix: improve safe area z-index computation and take box-shadow/outlines into account * fix: fix whole page screenshots and screenshots by coords * fix: fix loss of error cause when passing error between worker and master * test: implement assertView e2e fixture, add more e2e tests, fix failing unit tests * fix: fix full page auto detection condition * feat: implement type-safe isomorphic coords helpers (#1233) * feat: implement type-safe isomorphic coords helpers * fix: fix rounding helper issues and add tests * ci: use github runners instead of self-hosted * ci: run standalone tests on github runners * ci: fix fixtures generation script * refactor: refactor client-bridge to support multiple instances and namespaces (#1235) * feat: implement type-safe isomorphic coords helpers * fix: fix rounding helper issues and add tests * ci: use github runners instead of self-hosted * ci: run standalone tests on github runners * ci: fix fixtures generation script * refactor: refactor client-bridge to support multiple instances and namespaces * feat: rewrite client-scripts to typescript and refactor them (#1236) * test: implement browser-env tests on client-scripts (#1237) * feat: rewrite composite-image and implement robust unit tests (#1238) * feat: rewrite composite-image and implement robust unit tests * fix: handle edge cases in image class, including overfowing ignore areas and invalid sizes * chore: add composite-image test fixtures to gitignore * feat: improve calibrator to work correctly on ios simulators and camera cropping logic (#1239) * feat: improve calibrator to work correctly on ios simulators and camera cropping logic * test: fix camera tests * feat: improve and refactor screen-shooter and split it into 3 versions (#1240) * test: implement screen-shooter integration tests (#1241) * fix: exclude certain dependencies from prebundling * test: specify concrete browser version in standalone integration tests * test: implement screen-shooter integration tests * fix: fix remaining issues and checks (#1242) * fix: remove duplicate screenshot delay * fix: correctly handle absence of capture rects in safe area computation * fix: remove delay in screen-shooter and wait for settle when capturing checkpoints * fix: fix safe area computation and failing e2e tests * test: fix unit tests and formatting issues * fix: simplify return types in client-scripts and use correct logging topics * chore: use better variable names when computing pseudo element coords and strip unnecessary comments * ci: use separate comment tag for browser-env tests * chore: do not collect coverage for browser-env tests * test: tiny fixes of browser-env tests and removing unnecessary console log * fix: add a few explanatory comments and fix a typo in image class * test: commit composite-image fixtures instead of generating them on the fly each time * docs: add info about generating debug images to dev docs * fix: use correct viewport offset for a case when driver returns full-page screenshot * refactor: move composite iterations limit to constants, minor review fixes * fix: fix off-by-one errors in calibrator and leave a comment clearing why coord helpers use specific height/width convention * test: fix a couple of ever-green tests on screen-shooter * test: run fractional scroll test only in chrome * fix: fix calibration script and add debug logging to calibrator * chore: enhance logging in elements screenshooter and fix tests * ci: add tsc-out to prettierignore * fix: do not apply calibration if viewport size changed * fix: add more robust logic for computing layout shifts and fix tests * test: exclude fixtures from unit tests * test: fix tests with node types stripping * test: fix integration screenshot tests * test: build browser bundle before test * test: build project before test * test: do not run screenshot integration tests in firefox * fix!: use gridUrl: "local" by default * feat: make CLI and NEW_BROWSER events async * fix!: set default resetCursor value to false * fix!: remove global helpers (it,describe,...) * chore!: purge devtools * feat!: add async function config support * fix: calibrate errors (port) * fix: fix remaining issues in new assertView algorithm and implement cropMargins options (#1277) * fix: handle a case when setTimeout is stubbed during waiting for selectors to stabilize * fix: fix safe area computation for fixed blocks * fix: disable animations and hovers in page screenshots by default * fix: do not move pointer on mobile devices, because it causes some browsers to freeze * fix: handle a case when capture area needs to be expanded, but capture elements are out of viewport * fix: fix handling of sticky/fixed positioned elements inside capture elements * fix: when computing average shift, only take non-zero shifts into account * fix: print helpful message if element to capture is hidden/disappeared mid-capture * fix: ignore transparent blocks via css filter opacity when computing safe area * fix: capture fixed-positioned elements and fix various inconsistencies with fixed/sticky/absolute elements participating in capture * fix: handle missing timeout value in safari simulators during capture area settle wait * fix: rollback only on needed amount of px if safe area shrinks instead of on the whole safe area size to prevent infinite rollback-scroll trap * fix: use correct computation to determine instersection percentage between safe area and capture area * fix: revert introducing synthetic capture specs for fixed-positioned descendants * feat: implement cropMargins options * chore: enable screenshot verbose logging only with TESTPLANE_DEBUG_SCREENSHOTS * fix: fix zero maxDelta non-renderable capture spec case during composite and unit tests * test: fix unit tests * fix: fix review issues * fix!: set default tolerance value to 3 and ignoreDiffPixelCount to 4 * docs: actualize screenshots dev readme * chore: update package-lock * test: fix unit tests * feat!: useWsDriver is turned on by default * chore!: bump required node version to 22 * ci: actualizd github workflows * chore: revert rc version bump * fix: perform waitForStaticToLoad call at the correct stage during assertView --------- Co-authored-by: Roman Kuznetsov <kuznetsov.roman.orenburg@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What's done?
Implemented the
cropMarginsoptions that lets users specify crop areas at the camera level. Intended to be used instead of old hacks like generating cover elements to circumvent scrollbars. Now, you can just specifycropMargins: { right: 8 }to always cut 8 pixels off each screenshotFixed lots of edge cases, each edge case is described at the corresponing commit message. Note: at first, i decided to auto-include fixed-positioned descendants in capture specs, but later reverted this, so don't spend time on this if you are going to review this PR commit-by-commit