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 trigger resets with fast reruns #7283
Fix trigger resets with fast reruns #7283
Conversation
# Update ourselves with the new widget_states. The old widget states, | ||
# used to skip callbacks if values haven't changed, are also preserved. | ||
# Clear any triggers that weren't reset because the script was disconnected | ||
self._reset_triggers() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an alternative, I considered splitting on_script_finished
in two, and calling the part with the trigger reset from request_stop
, before the disconnect. I opted to do it here instead to be more sure that the reset will happen, and to avoid having to create a bunch of plumbing methods that now correspond to calling just one function on the actual session state.
87e6233
to
1081c13
Compare
Describe your changes
Resetting trigger values of session state before we copy in the new client state. If fast reruns is enabled, the old script run can't do its
on_script_finished
cleanup, because it has been disconnected. For removing stale widgets, that is mostly not a problem, but failing to reset triggers basically prevents buttons from working until the script cleanly finishes. Resetting duringon_script_will_rerun
prevents this problem, making the behavior consistent with or without fast reruns.GitHub Issue Link (if applicable)
Fixes #6643
Testing Plan
Verified with manual tests. I should probably add an e2e regression test for this.
Contribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.