-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat:loading_status_templates #68
Conversation
extracted from system PHAL for re-use and uncoupling of GUI resources from skills/plugins companion PR adding this to GIInterface ```python def show_loading_animation(self, text: str, override_idle: Union[int, bool] = None, override_animations: bool = False): """ Display a GUI loading animation Arguments: text (str): Main text content. override_idle (boolean, int): True: Takes over the resting page indefinitely (int): Delays resting page for the specified number of seconds. override_animations (boolean): True: Disables showing all platform skill animations. False: 'Default' always show animations. """ self["label"] = text self.show_page("SYSTEM_Loading", override_idle, override_animations) def show_status_animation(self, text: str, success: bool, override_idle: Union[int, bool] = None, override_animations: bool = False): """ Display a success/failure animation. Arguments: text (str): Main text content. success (bool): Success (True) or Failure (False) animation override_idle (boolean, int): True: Takes over the resting page indefinitely (int): Delays resting page for the specified number of seconds. override_animations (boolean): True: Disables showing all platform skill animations. False: 'Default' always show animations. """ self["status"] = "Enabled" if success else "Disabled" # string check in QML self["label"] = text self.show_page("SYSTEM_Status", override_idle, override_animations) ```
WalkthroughThis pull request introduces new QML components and Lottie animation JSON files for a loading screen and system status interface in a Qt5 application. The changes include creating three new QML files in the Changes
Sequence DiagramsequenceDiagram
participant UI as User Interface
participant Loading as Loading Screen
participant Status as Status Component
UI->>Loading: Trigger loading state
Loading->>Loading: Display loading animation
UI->>Status: Check system status
Status->>Status: Determine status (Success/Fail)
Status->>Status: Display corresponding animation
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (6)
ovos_gui/res/gui/qt5/SYSTEM_Loading.qml (2)
15-18
: Use a more subtle background for loading overlays.A fully opaque background may obscure the underlying content if this loading view is used as an overlay. Consider adding a semi-transparent overlay or transitional effect to visually hint at the app's current state.
20-29
: Investigate content scaling or adaptive layout.The margin and spacing usage is helpful, but ensure a consistent layout across multiple screen sizes (desktop vs. smaller displays). Verify if larger margins or dynamic scaling is more suitable for smaller resolutions.
ovos_gui/res/gui/qt5/SYSTEM_Status.qml (3)
19-19
: Consider data type forsuccess
property.
success
is declared asproperty var success: sessionData.status
. If only two states are expected (“Enabled” / “Disabled”), a boolean might suffice. This can help avoid string-based condition checks.
22-28
: Refine method naming.
checkstatus
effectively returns a file path to an animation. A more descriptive name likestatusAnimationSource
oranimationSourceForStatus
may improve readability.
30-65
: Ensure fallback or error state.If
sessionData.status
is neither “Enabled” nor “Disabled”, thecheckstatus
function defaults to returningundefined
. Consider a default fallback animation or a “no status” message.ovos_gui/res/gui/qt5/animations/status-fail.json (1)
1-1
: Optimize JSON size if performance is critical.Large Lottie JSON files can impact load times. Check if vector paths or unused layers can be simplified without compromising visual quality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
ovos_gui/res/gui/qt5/SYSTEM_Loading.qml
(1 hunks)ovos_gui/res/gui/qt5/SYSTEM_Status.qml
(1 hunks)ovos_gui/res/gui/qt5/animations/loading.json
(1 hunks)ovos_gui/res/gui/qt5/animations/status-fail.json
(1 hunks)ovos_gui/res/gui/qt5/animations/status-success.json
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- ovos_gui/res/gui/qt5/animations/status-success.json
🔇 Additional comments (7)
ovos_gui/res/gui/qt5/SYSTEM_Loading.qml (4)
1-7
: Consider verifying import version compatibility.
These QML import versions (e.g., QtQuick 2.12
, QtQuick.Controls 2.12
) may not always be consistent across different deployments. Confirm that these specific versions are available in all target environments or consider fallback logic where necessary.
9-9
: Ensure the parent container usage is valid.
By using Mycroft.Delegate
as the root, confirm that any external references or theming rely on this type and that it can successfully inherit the parent properties.
30-40
: Provide i18n support for text labels.
The sessionData.label
is displayed directly. If localization is required, remember to integrate translation mechanisms or placeholders for future expansions in multiple languages.
42-53
: Confirm resource availability at runtime.
The loading animation is sourced from "animations/loading.json"
. Verify that this file path is correct and that the resource is included in the deployment process.
ovos_gui/res/gui/qt5/SYSTEM_Status.qml (2)
1-7
: Confirm consistent QtQuick imports.
Versions differ slightly from SYSTEM_Loading.qml
(e.g., QtQuick 2.4
vs. QtQuick 2.12
). Verify if standardizing on a single version is desired or needed for uniform behavior.
9-9
: Ensure theming consistency.
Using Mycroft.Delegate
for both loading and status screens is good for consistency, but check whether there are any specific style overrides or behaviors that differ between the two screens.
ovos_gui/res/gui/qt5/animations/loading.json (1)
1-1
: Ensure seamless looping and performance.
This loading animation is set to loop. Verify CPU/GPU usage during prolonged loops, especially on lower-end platforms or embedded devices. Consider limiting frame rate or offering a simpler alternative in resource-constrained environments.
extracted from system PHAL for re-use and uncoupling of GUI resources from skills/plugins companion PR OpenVoiceOS/ovos-gui#68
extracted from system PHAL for re-use and uncoupling of GUI resources from skills/plugins companion PR OpenVoiceOS/ovos-gui#68
extracted from system PHAL for re-use and uncoupling of GUI resources from skills/plugins
companion PR adding this to GIInterface
Summary by CodeRabbit
New Features
Documentation