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: only adjust autoscrolling in interactive mode #23053

Merged
merged 17 commits into from Sep 13, 2022
Merged

Conversation

marktnoonan
Copy link
Contributor

@marktnoonan marktnoonan commented Aug 2, 2022

User facing changelog

In run mode, autoscrolling of the command log can no longer become disabled.

Additional details

In the linked issues several users have reported issues with the command log autoscroll becoming disabled in run mode intermittently, or in some cases every time. This is particularly painful when reviewing the video for a failed test, as without autoscrolling, the error message associated with the failure is not visible. This makes debugging a lot harder.

The underlying cause is that for whatever reason, especially when resources are low, the browser sometimes seems to incorrectly detect user scrolling when elements are rapidly being added to the command log and Cypress is autoscrolling, which triggers the temporarilySetAutoScrolling(false) code that is helpful in open mode, since in open mode it's natural for to manually scroll the reporter while tests are running, and autoscroll would contradict that.

Since this has no purpose in run mode (or a very narrow one, when running with the --headed flag), it seems best to just turn off the scroll handling when we know that any scrolling is not possibly coming from the user.

Steps to test

This one is difficult to test manually as we have almost never been able to reproduce it. But users who have tried pre-release binaries implementing the fix report that it works.

How has the user experience changed?

PR Tasks

  • Have tests been added/updated?
  • Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • Has a PR for user-facing changes been opened in cypress-documentation?
  • Have API changes been updated in the type definitions?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Aug 2, 2022

Thanks for taking the time to open a PR!

@cypress
Copy link

cypress bot commented Aug 2, 2022



Test summary

40020 0 3359 0Flakiness 0


Run details

Project cypress
Status Passed
Commit d72ceb7
Started Sep 13, 2022 12:16 PM
Ended Sep 13, 2022 12:32 PM
Duration 16:01 💡
OS Linux Debian - 11.3
Browser Multiple

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@marktnoonan marktnoonan marked this pull request as ready for review September 10, 2022 02:04
@lmiller1990
Copy link
Contributor

Going to add @emilyrohrbough who has been working on the reporter a lot lately for some additional 👀

@marktnoonan
Copy link
Contributor Author

Updated with @lmiller1990's much better tests 🙏

Copy link
Contributor

@mike-plummer mike-plummer left a comment

Choose a reason for hiding this comment

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

Verified scrolling is working as expected in Electron, Chrome, FF, and no issues in headless. Looks good 👍

@lmiller1990 lmiller1990 removed the request for review from emilyrohrbough September 13, 2022 00:51
@marktnoonan marktnoonan merged commit 25a47aa into develop Sep 13, 2022
@marktnoonan marktnoonan deleted the marktnoonan/16098 branch September 13, 2022 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto Scrolling no longer works after some time Auto scroll is turned off sometimes
3 participants