Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Fix multi-dom capture when using the snapshot command #1097

Merged
merged 1 commit into from
Oct 6, 2022

Conversation

wwilsman
Copy link
Contributor

@wwilsman wwilsman commented Oct 6, 2022

What is this?

While testing multi-dom support with the snapshot command, I noticed two issues that this PR fixes.

  1. When both delayUploads and deferUploads are true, the snapshot queue acts delayed but not deferred. This is fixed by checking for !deferUploads before automatically flushing the queue.

  2. When the first snapshot is taken at the initial width, it is missing it's singular width. This happened because width was undefined for the first loop, and fixed by setting it to the first width.

One test was updated to test for both of these issues. First delayUploads was added to ensure it does not affect deferUploads. Second, debug logging was added to test for a specific snapshot log to verify each singular width.

@wwilsman wwilsman added the 🐛 bug Something isn't working label Oct 6, 2022
@wwilsman wwilsman requested a review from Robdel12 October 6, 2022 17:27
Copy link
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 c13f7d1 into master Oct 6, 2022
@wwilsman wwilsman deleted the ww/fix-multi-capture-bug branch October 6, 2022 17:32
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.

2 participants