Add progress spinner while checking single addon#179
Conversation
4dabc42 to
c8644ed
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds a visual progress spinner to indicate when an addon is being checked for updates in the detailed view. The implementation replaces the previous "Check for Update" button with an animated spinner widget that appears automatically when update checks are in progress.
Key changes:
- Created a new reusable Spinner widget with SVG-based animation and automatic foreground/background color detection
- Integrated the spinner into the addon button bar to replace the manual update check button
- Added automatic spinner activation when addon status is UNCHECKED and deactivation when checks complete
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| addonmanager_package_details_controller.py | Added calls to show/hide spinner during update check lifecycle |
| Widgets/spinner.py | New animated SVG spinner widget with color adaptation and customizable speed |
| Widgets/addonmanager_widget_addon_buttons.py | Replaced manual update button with automatic spinner widget |
| Widgets/CMakeLists.txt | Added spinner.py to build files |
| Resources/icons/CMakeLists.txt | Added spinner.svg icon to resources |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| # * <https://www.gnu.org/licenses/>. * | ||
| # * * | ||
| # *************************************************************************** | ||
| import os |
There was a problem hiding this comment.
[nitpick] The os import is only used once for path joining. Consider using pathlib.Path instead for better cross-platform compatibility and more modern Python practices.
| import os | |
| from pathlib import Path |
| super().__init__(parent) | ||
| spinner_file = os.path.join( | ||
| os.path.dirname(__file__), "..", "Resources", "icons", "spinner.svg" | ||
| ) |
There was a problem hiding this comment.
Hard-coded relative path '../Resources/icons/spinner.svg' may break if the file structure changes. Consider using a more robust path resolution method or making the spinner icon path configurable.
| ) | |
| import importlib.resources | |
| from PySideWrapper import QtCore, QtGui, QtWidgets, QtSvg | |
| class Spinner(QtWidgets.QWidget): | |
| def __init__(self, parent=None, fps=60, speed_deg_per_sec=180, spinner_svg_path=None): | |
| super().__init__(parent) | |
| if spinner_svg_path is not None: | |
| spinner_file = spinner_svg_path | |
| else: | |
| # Use importlib.resources to locate the spinner.svg file | |
| try: | |
| # Adjust the package name below to match where spinner.svg is located | |
| # For example, if spinner.svg is in Widgets/Resources/icons/ | |
| # the package would be 'Widgets.Resources.icons' | |
| package = "Widgets.Resources.icons" | |
| resource = "spinner.svg" | |
| with importlib.resources.path(package, resource) as svg_path: | |
| spinner_file = str(svg_path) | |
| except (ImportError, FileNotFoundError): | |
| # Fallback to previous method if importlib.resources fails | |
| spinner_file = os.path.join( | |
| os.path.dirname(__file__), "..", "Resources", "icons", "spinner.svg" | |
| ) |
| else QtGui.QColor(QtCore.Qt.black) | ||
| ) | ||
|
|
||
| def paintEvent(self, _evt): |
There was a problem hiding this comment.
The paintEvent method creates a new QImage, QPainter, and performs SVG rendering on every paint cycle. Consider caching the rendered image and only regenerating when the angle changes significantly to improve performance.
| if self.addon.status() == Addon.Status.UNCHECKED: | ||
| # If the user is looking at this addon, and it has not been checked for updates (e.g., | ||
| # it is a custom addon that has to directly query its git repo), then do that check now. | ||
| self.ui.button_bar.set_update_check_status(True) |
There was a problem hiding this comment.
The spinner is started before checking if self.update_check_thread exists. If thread creation fails after this point, the spinner will remain visible indefinitely. Move this call after successful thread creation or add proper error handling.
Should only display in very rare circumstances
c8644ed to
c6baedc
Compare
Update checks happen automatically on startup (for most addons it is just part of the supplied metadata in the catalog, only custom addons need to be checked at their source). In rare circumstances something might happen to interrupt this: if that happened, automatically run an update check when the addon's detailed view is shown, and display a small spinner to indicate that is happening.