Skip to content

🐛 Wait for network idle for responsive asset discovery#512

Merged
wwilsman merged 1 commit intomasterfrom
ww/fix-responsive-discovery
Aug 19, 2021
Merged

🐛 Wait for network idle for responsive asset discovery#512
wwilsman merged 1 commit intomasterfrom
ww/fix-responsive-discovery

Conversation

@wwilsman
Copy link
Copy Markdown
Contributor

What is this?

Thanks to @cancan101, it was pointed out that the browser may cancel responsive image requests between resizes. While we have a test for this, the test wasn't quite comprehensive enough. Adding a third width to the test reveals that intermediary responsive asset requests between the first and the last are indeed cancelled because the viewport is resized too quickly. The first asset in this test likely only succeeds due to page navigation waiting for the load event.

This PR adjusts the test so it accurately fails, then duplicates discovery's idle check before resizing. Unfortunately, this fix has a drawback that causes snapshots to take slightly longer than they used to, by a magnitude of networkIdleTimeout * (widths.length - 1). So a network idle timeout of 100ms across 3 widths would result in the snapshot taking 200ms longer than it previously did without this change.

Fixes #511

@wwilsman wwilsman added the 🐛 bug Something isn't working label Aug 19, 2021
@wwilsman wwilsman requested a review from Robdel12 August 19, 2021 21:01
@wwilsman wwilsman enabled auto-merge (squash) August 19, 2021 21:09
Copy link
Copy Markdown
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏁

@wwilsman wwilsman merged commit c9b8edc into master Aug 19, 2021
@wwilsman wwilsman deleted the ww/fix-responsive-discovery branch August 19, 2021 21:11
samarsault pushed a commit that referenced this pull request Mar 3, 2023
* ⬆️ Bump cypress from 9.7.0 to 10.0.2

Bumps [cypress](https://github.com/cypress-io/cypress) from 9.7.0 to 10.0.2.
- [Release notes](https://github.com/cypress-io/cypress/releases)
- [Changelog](https://github.com/cypress-io/cypress/blob/develop/.releaserc.base.js)
- [Commits](cypress-io/cypress@v9.7.0...v10.0.2)

---
updated-dependencies:
- dependency-name: cypress
  dependency-type: direct:development
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

* ⬆️ Upgrade to Cypress 10.x

* ✂️ Remove unneeded comment in config file

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Percy CLI Snapshot should wait for network to be idle between resizes

2 participants