refactor: webgui sync script to copy uui build #1318
Conversation
…cesses - Changed the import of chalk to use dynamic import for better compatibility. - Updated paths for web components and UI components in the sync functions. - Enhanced error handling and success messages during the build and sync processes for both web and UI components. - Modified the menu options to reflect the new functionality for building and syncing UI components. This refactor improves the overall structure and clarity of the code, ensuring better maintainability and user experience.
- Replaced `devModeEnabled` with `enableThemeSwitcher` to control the visibility of the theme switcher based on session and local storage values. - This change enhances user experience by allowing theme switching to persist across sessions. This refactor improves the functionality of the theme switcher component, ensuring it behaves as expected based on user preferences.
|
Caution Review failedThe pull request is closed. WalkthroughThe changes update several components of the project. In the theme switcher, a new constant based on storage checks replaces the previous Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
web/components/ThemeSwitcher.ce.vue (1)
37-38: Use a single source of truth or name to unify usage and reduce confusion.
These lines introduce a new storage key and boolean logic by checking both sessionStorage and localStorage. Consider whether you actually need both or if you should prioritize one. If a user toggles local storage but not session storage, this code still evaluates true. If that’s acceptable, then this is fine as is.web/scripts/sync-webgui-repo.js (6)
7-10: Handle potential import errors for chalk.
Using dynamic import without error handling can fail silently if the module is missing or incompatible, causing runtime errors later. Consider wrapping this in a try-catch or checking ifchalkis loaded successfully.
290-301: Enhance logging for build process flow.
The new build logic uses an async child process and logs upon success or failure. Consider adding more detail or structured logs to quickly troubleshoot build failures, such as build duration or environment info.
304-331: Consolidate duplicate build logic.
The UI build code mirrors the web build code. If they share a common structure, you could introduce a helper to handle the repetitive steps, letting you pass in different commands or directories.
363-385: Abstract common code between web and UI sync.
This sync function is almost identical tohandleWebComponentSync(). Consolidating them into a single function with parameters for each subpath can reduce maintenance overhead.
571-571: Avoid infinite loop risk.
While awhile (true)menu loop is functional, consider a do-while with a defined exit condition to reduce the chance of unexpected infinite loops if the user input is unhandled.
577-581: Menu clarity improvement.
The new menu options are clear, but consider unifying them under a single “Build & Sync” sub-menu or grouping them logically to reduce confusion for new contributors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/components/ThemeSwitcher.ce.vue(2 hunks)web/scripts/sync-webgui-repo.js(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build Web App
- GitHub Check: Test API
- GitHub Check: Build API
- GitHub Check: Cloudflare Pages
🔇 Additional comments (4)
web/components/ThemeSwitcher.ce.vue (1)
77-77: Ensure consistent fallback behavior for theme selection.
Currently, the theme switcher is shown ifenableThemeSwitcheris true in storage. If no storage values exist, it remains hidden. Double-check you have an intended default or fallback to avoid confusion if neither storage is set.web/scripts/sync-webgui-repo.js (3)
184-194: Confirm chalk availability before logging with color.
These logs rely onchalk.default; if the async import fails or doesn’t complete in time, the calls may throw an error. Verify thatchalkis loaded and fallback to plain console logs if not.
334-361: Validate removal of existing directories is deliberate.
The script forcefully removes previous web component directories before copying new ones, which can lead to data loss if invoked accidentally. Confirm that wiping the entire directory is the intended approach. Optionally, prompt the user before deletion or back up old builds.
583-583: Validate numeric input.
Relying solely on user-entered numeric strings can cause hidden issues if a user enters characters or out-of-range options. Confirm that your validation or fallback logic adequately covers these cases.
this fixes the callbacks into unraid.net
…vation-code-extractor.php correctly (#1320) --- - To see the specific tasks where the Asana app for GitHub is being used, see below: - https://app.asana.com/0/0/1209867047971138
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
pnpm sync-webgui-repoto include Unraid UI build + sync optionsSummary by CodeRabbit
New Features
Bug Fixes