-
Notifications
You must be signed in to change notification settings - Fork 25
feat: add update check for mobile view #1323
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
Conversation
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.
Pull Request Overview
This PR adds update checking functionality to the mobile navigation view in OpenCloud. The implementation moves update checking from being handled asynchronously within components to being handled during application bootstrap and then displayed in the mobile navigation interface.
- Adds update check capability to mobile navigation with version display
- Refactors update checking to be performed during application bootstrap rather than in individual components
- Updates configuration store to handle update information from external service
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web-runtime/src/index.ts | Adds call to announceUpdates during bootstrap |
| packages/web-runtime/src/container/bootstrap.ts | Implements announceUpdates function to fetch update data |
| packages/web-runtime/src/components/VersionCheck.vue | Refactors to use pre-loaded update data instead of async loading |
| packages/web-runtime/src/components/MobileNav.vue | Adds version display and update check to mobile navigation |
| packages/web-pkg/src/composables/piniaStores/config/types.ts | Adds UpdatesConfigSchema for update data structure |
| packages/web-pkg/src/composables/piniaStores/config/config.ts | Adds updates store and loadConfigUpdates method |
| packages/web-pkg/src/components/FilesList/ResourceListItem.vue | Reorders v-oc-tooltip directive |
| packages/web-pkg/src/components/FilesList/ResourceLink.vue | Reorders v-if directive |
| packages/web-app-admin-settings/src/views/Users.vue | Reorders v-oc-tooltip directive |
| packages/web-runtime/tests/unit/components/SidebarNav/snapshots/SidebarNav.spec.ts.snap | Updates snapshot to reflect new update check display |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
@kulmann to have a clean loading and error state: instead of using the config store we might want to implement an updateStore (updates, isLoading, error...). We can also make the request non blocking in the bootstrap process, I think. |
kulmann
left a comment
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.
I like this approach 👍
| } | ||
|
|
||
| try { | ||
| updatesStore.setIsLoading(true) |
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.
Why do you set loading to true here? It's initialized with true.
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.
just in case, if we re-use this function, e.G polling or something else, this would come in handy
kulmann
left a comment
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.
nice 🚀 thank you!
Description
Related Issue
How Has This Been Tested?
Types of changes