feat: show loading spinner on Click Here buttons while app is running#117
Open
Phqen1x wants to merge 2 commits into
Open
feat: show loading spinner on Click Here buttons while app is running#117Phqen1x wants to merge 2 commits into
Phqen1x wants to merge 2 commits into
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors “Click Here” button handlers in src/manualtest.py to launch external applications asynchronously while providing UI feedback (spinner + disabled button) to prevent duplicate clicks.
Changes:
- Added
_launch_app_with_spinner()helper to run external commands in a background thread and manage button state. - Updated ScreenTest, WebCam, and Browser click handlers to use the new helper.
- Consolidated
subprocess/threadingimports at the top of the module.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+429
to
+431
| proc = subprocess.Popen( | ||
| command, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE | ||
| ) |
Comment on lines
+433
to
+434
| except Exception: | ||
| pass |
Comment on lines
+437
to
+440
| def _restore(): | ||
| button.set_label("Click Here") | ||
| button.set_sensitive(True) | ||
| return False |
Comment on lines
493
to
495
| if click_button is not None: | ||
| click_button.set_label("Click Here") | ||
| click_button.set_sensitive(True) |
- Use communicate() instead of wait() to drain stdout/stderr pipes and prevent potential deadlock when launched apps produce output - Log exceptions instead of silently swallowing them - Stop spinner before restoring button label in both _restore and _on_touchscreen_test_complete to avoid leaving spinner running Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.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
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.
This pull request refactors how external applications are launched from the UI in
src/manualtest.py. The main improvement is the introduction of a new method to launch apps with a loading spinner and proper button state management, which enhances user feedback and code maintainability. Several button click handlers are updated to use this new approach.Refactoring and UI Improvements:
_launch_app_with_spinnermethod that launches external applications in a background thread, displays aGtk.Spinneron the button, and disables the button until the process completes. This provides clear feedback to the user and prevents duplicate clicks.on_screentest_clicked,on_webcam_clicked, andon_browser_clickedmethods to use_launch_app_with_spinnerinstead of directly launching apps, ensuring consistent UI behavior across these actions. [1] [2] [3]Code Cleanup:
subprocessandthreadingfrom within functions, consolidating them at the top of the file. [1] [2]