refactor: unraid ui cleanup and migration #998
Conversation
WalkthroughThis pull request introduces a comprehensive redesign of the Unraid UI library, focusing on component standardization, styling improvements, and library structure optimization. The changes span multiple files across the Changes
Sequence DiagramsequenceDiagram
participant UI Library as Unraid UI Library
participant Web App as Web Application
participant Build Process as GitHub Actions
UI Library->>Build Process: Build library components
Build Process-->>Web App: Deploy UI artifacts
Web App->>UI Library: Import components
UI Library->>Web App: Provide standardized components
Possibly related PRs
Poem
Finishing Touches
🪧 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 (6)
unraid-ui/stories/components/common/Button.stories.ts (1)
17-17: Check consistency for property naming
Changing from atextprop todefaultmight confuse contributors who expect a textual label prop. Ensure the entire codebase and team are aligned with this naming convention.unraid-ui/src/components/common/badge/Badge.vue (1)
3-4: Prefer named imports
You’re importing type { Component } from "vue"; consider also importing any relevant utility types if you plan to define more flexible constraints oniconoriconRightin the future, such as restricting them to valid Vue component types.unraid-ui/stories/components/common/DropdownMenu.stories.ts (2)
10-13: Consider grouping related stories.
Thetitle: "Components/Common"might be too broad for different common components. Placing theDropdownMenustory under a more specific folder (e.g.,"Components/Common/Dropdown") can help keep the Storybook more organized and make it easier to locate related stories.
19-45: Leverage args or controls for better story interactivity.
Using args and controls in Storybook can make theDropdownMenustory more interactive and user-friendly. This lets users dynamically togglevariant,onSelectcallbacks, etc.unraid-ui/src/index.ts (2)
27-27: Confirm proper usage of Lightswitch.
AddingLightswitchin the index export is good, but ensure the naming is consistent across UI components (e.g., “LightSwitch” or “Lightswitch”). Consistency boosts discoverability.
74-74: Export alignment with existing patterns.
WithbuttonVariantsnow included, ensure other variant objects (like for badges or other components) follow a similar pattern. This will avoid confusion when importing or applying these variants in your codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (14)
unraid-ui/src/components/common/badge/Badge.vue(1 hunks)unraid-ui/src/components/common/badge/badge.variants.ts(1 hunks)unraid-ui/src/components/common/badge/index.ts(1 hunks)unraid-ui/src/components/common/button/Button.vue(2 hunks)unraid-ui/src/components/common/button/button.variants.ts(1 hunks)unraid-ui/src/components/common/button/index.ts(1 hunks)unraid-ui/src/components/form/lightswitch/index.ts(1 hunks)unraid-ui/src/components/form/switch/index.ts(1 hunks)unraid-ui/src/index.ts(4 hunks)unraid-ui/src/types/badge.ts(1 hunks)unraid-ui/src/types/button.ts(0 hunks)unraid-ui/stories/components/common/Badge.stories.ts(1 hunks)unraid-ui/stories/components/common/Button.stories.ts(1 hunks)unraid-ui/stories/components/common/DropdownMenu.stories.ts(1 hunks)
💤 Files with no reviewable changes (1)
- unraid-ui/src/types/button.ts
✅ Files skipped from review due to trivial changes (2)
- unraid-ui/src/components/form/lightswitch/index.ts
- unraid-ui/src/components/common/button/button.variants.ts
🔇 Additional comments (19)
unraid-ui/src/components/form/switch/index.ts (2)
1-1: Exports look good
Exporting the components as default is consistent with the project's new approach to simpler imports.
2-2: Verify references to 'SwitchHeadlessUI'
Ensure that all occurrences of 'SwitchHeadlessUI' in the codebase are updated to this new export, and that no references to the old name exist.unraid-ui/src/components/common/badge/index.ts (1)
1-2: Consolidated exports look goodExporting the default
BadgeandbadgeVariantsfrom a single index file simplifies imports for consumers and aligns well with the overall refactor strategy.unraid-ui/src/components/common/button/index.ts (1)
2-2: Check for broken references after renamingRenaming
ButtonVariantstobuttonVariantsis consistent with standard naming conventions. Verify that all references have been updated across the codebase to avoid unexpected import errors.✅ Verification successful
No broken references found - type and value exports are correctly used
The codebase shows a correct pattern where:
buttonVariantsis exported as a value (the cva function result)ButtonVariantsis exported as a type derived from that value usingVariantProps- The Vue component correctly imports and uses both: the type for props typing and the value for styling
The renaming in the original file (
unraid-ui/src/components/common/button/index.ts) appears to be a different implementation, and the found references are from a separate shadcn implementation in theweb/directory, which maintains its own correct naming convention.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Searching for any references to the old name "ButtonVariants" across the codebase rg "ButtonVariants"Length of output: 369
Script:
#!/bin/bash # Let's check the content of the button variants file to understand the type definition cat web/components/shadcn/button/index.tsLength of output: 1335
unraid-ui/src/types/badge.ts (2)
5-7: Extended interface for flexible variant handlingExtending
UiBadgePropsfromVariantProps<typeof badgeVariants>is a clean approach, providing dynamic style configurations. Confirm that previously defined color-based props are intentionally removed and no longer needed, to avoid regressions in styling.
1-3: Use consistent import pathsUsing
class-variance-authorityhere is a good approach for flexible styles. Ensure that usage of thebadgeVariantsimport path matches your existing alias patterns across the entire project, avoiding potential path resolution issues.✅ Verification successful
Import path is consistent with project patterns
The import path
@/components/common/badge/badge.variantsis consistent with the project's established import patterns. The codebase shows consistent usage of the@/componentsalias across multiple files, with similar structuring for common components and their variants. The current import follows the same convention used for other component imports throughout the project.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Checking for potentially mismatched alias references to "badge.variants" rg "@/components/common/badge/badge.variants"Length of output: 148
Script:
#!/bin/bash # Let's check the import patterns used in the project for component imports rg "^import.*from.*@/components" -g "*.{ts,vue}"Length of output: 1153
unraid-ui/src/components/common/button/Button.vue (4)
3-3: Renamed variant import aligns with naming conventionRenaming
ButtonVariantstobuttonVariantsenforces a consistent lower-case-initiated naming convention for variant utilities. This cleanup helps maintain code clarity.
6-6: Structured and exported interface for clarityChanging
Propsto a named and exportedButtonPropsinterface clarifies usage inside and beyond the component. This is a best practice for reusability in broader contexts.
18-18: Use of defaults for component propsDeclaring default values via
withDefaultsensures a fallback forvariantandsize. Verify that all relevant code paths expect these defaults, especially if the component might receive empty props in edge cases.
25-25: Dynamic class generationUtilizing
buttonVariants({ variant: props.variant, size: props.size })combined withcneffectively merges classes. This approach is concise and maintainable, ensuring a single source of truth for styling logic.unraid-ui/stories/components/common/Button.stories.ts (2)
11-11: Use of StoryObj for typed Storybook configuration
DefiningStoryusingStoryObj<typeof meta>is a clean approach for typed Storybook configs, providing better autocomplete and type-checking.
24-32: Confirm usage of :class="args.class"
There is noclassprop defined inargs, so:class="args.class"may be superfluous or a leftover. If you intend to let users pass a custom class, consider documenting it or providing a default value. Otherwise, remove it to keep the component simpler.✅ Verification successful
Let me check the Button component and its story configuration to verify this concern.
Let me check the Button component to see if it accepts a class prop.
:classbinding is correctly implemented and documentedThe
classprop is properly defined in the Button component's interface (ButtonProps) and is used correctly in the component implementation. The story can accept custom classes throughargs.class, which will be merged with the component's base classes using thecnutility. No changes are needed.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for references to `args.class` in the codebase to determine usage. rg -A 3 'args\.class'Length of output: 327
Script:
#!/bin/bash # Check the Button stories file for args definition cat unraid-ui/stories/components/common/Button.stories.tsLength of output: 788
Script:
#!/bin/bash # Check the Button component implementation cat unraid-ui/src/components/common/button/Button.vueLength of output: 729
unraid-ui/stories/components/common/Badge.stories.ts (1)
1-30: New Storybook story for Badge
This story file correctly follows the Storybook approach for default exports, typed stories, and arenderfunction returning the Vue component. Good job ensuring consistency in naming (matching "Badge" with the component name).unraid-ui/src/components/common/badge/Badge.vue (4)
6-13: Optional icon props
Providingicon,iconRight, andiconStylesas optional props is a good approach to keep the badge flexible. Ensure you document these props at the component or story level for consistent usage.
15-17: Use of default props withwithDefaults
Setting defaultvariantandsizeusingwithDefaultsensures users don’t have to provide them every time. This pattern is consistent with composition API best practices. Nice implementation!
23-35: Refactored style logic withbadgeVariants
Moving from manual switch-case statements tobadgeVariantscvausage improves readability and scalability. Good job centralizing style definitions in a single place (badge.variants.ts).
41-41: Conditional icon usage
Using<component :is="icon" v-if="icon" ...>is a neat pattern for optional icons. Ensure consistent styling or spacing when icons are absent.Also applies to: 46-46, 53-53
unraid-ui/src/components/common/badge/badge.variants.ts (1)
1-36: Centralized variant-based styling with CVA
DefiningbadgeVariantswithcvais a solid approach for scalable component styling. The variant and size definitions are well-structured and consistent with the default values inBadge.vue.unraid-ui/src/index.ts (1)
6-6: Verify consistency of naming conventions.
ButtonVariants→buttonVariantsis a valid rename, but ensure all references across the codebase follow the same lowerCamelCase style for variant const exports.✅ Verification successful
All references to
buttonVariantsfollow consistent lowerCamelCase namingThe codebase shows consistent usage of
buttonVariantsin lowerCamelCase across all files:
unraid-ui/src/components/common/button/button.variants.ts(definition)unraid-ui/src/components/common/button/Button.vue(usage)unraid-ui/src/components/common/button/index.ts(export)unraid-ui/src/index.ts(import and re-export)The only PascalCase
ButtonVariantsreferences are for TypeScript type definitions, which follows the correct TypeScript convention of PascalCase for types.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify all references to ensure they match the new naming convention "buttonVariants" rg -A 2 "ButtonVariants" rg -A 2 "buttonVariants"Length of output: 3013
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
unraid-ui/stories/components/common/DropdownMenu.stories.ts (4)
1-20: LGTM! Consider grouping imports with comments.The imports and metadata configuration are well-structured and type-safe.
Consider adding comments to group imports for better readability:
import type { Meta, StoryObj } from "@storybook/vue3"; +// Third-party components import { MoreVertical } from "lucide-vue-next"; +// Local components import Button from "../../../src/components/common/button/Button.vue";
21-47: Consider adding interaction handlers to demonstrate functionality.The story effectively demonstrates the component structure, but it could be enhanced by showing how to handle menu item interactions.
Consider updating the template to include click handlers:
- <DropdownMenuItem>Profile</DropdownMenuItem> - <DropdownMenuItem>Settings</DropdownMenuItem> - <DropdownMenuItem>Logout</DropdownMenuItem> + <DropdownMenuItem @click="onProfileClick">Profile</DropdownMenuItem> + <DropdownMenuItem @click="onSettingsClick">Settings</DropdownMenuItem> + <DropdownMenuItem @click="onLogoutClick">Logout</DropdownMenuItem>And add the corresponding methods to the story:
setup() { return { onProfileClick: () => console.log('Profile clicked'), onSettingsClick: () => console.log('Settings clicked'), onLogoutClick: () => console.log('Logout clicked'), } }
49-78: Consider extracting icon sizing to props and adding interactions.The story effectively demonstrates an icon-based dropdown, but has two areas for improvement:
- The icon sizing is hardcoded in the template
- Like the previous story, it lacks interaction handlers
Consider these improvements:
- Extract icon sizing to props:
- <MoreVertical class="h-4 w-4" /> + <MoreVertical :class="iconClass" />Add to the story:
setup() { return { iconClass: 'h-4 w-4', onEditClick: () => console.log('Edit clicked'), onDuplicateClick: () => console.log('Duplicate clicked'), onDeleteClick: () => console.log('Delete clicked'), } }
- Add interaction handlers:
- <DropdownMenuItem>Edit</DropdownMenuItem> - <DropdownMenuItem>Duplicate</DropdownMenuItem> - <DropdownMenuItem>Delete</DropdownMenuItem> + <DropdownMenuItem @click="onEditClick">Edit</DropdownMenuItem> + <DropdownMenuItem @click="onDuplicateClick">Duplicate</DropdownMenuItem> + <DropdownMenuItem @click="onDeleteClick">Delete</DropdownMenuItem>
33-44: Consider adding ARIA attributes for better accessibility.While the dropdown menu implementation is solid, consider enhancing accessibility by adding ARIA attributes to provide better screen reader support.
Consider adding these attributes to both stories:
<DropdownMenu> - <DropdownMenuTrigger> + <DropdownMenuTrigger aria-label="Open menu"> <Button variant="secondary">Open Menu</Button> </DropdownMenuTrigger> - <DropdownMenuContent> + <DropdownMenuContent aria-label="Navigation menu">Also applies to: 62-75
unraid-ui/stories/components/common/ScrollArea.stories.ts (1)
15-28: Consider parametrizing the item count.The array of items is hardcoded to 30. For better flexibility and reusability, you could leverage Storybook controls to let users adjust the number of items in the vertical scroll area.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (3)
unraid-ui/stories/components/common/DropdownMenu.stories.ts(1 hunks)unraid-ui/stories/components/common/Loading.stories.ts(1 hunks)unraid-ui/stories/components/common/ScrollArea.stories.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
unraid-ui/stories/components/common/Loading.stories.ts
[error] 3-3: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (4)
unraid-ui/stories/components/common/Loading.stories.ts (2)
2-2: Usage of structured imports is good.
The import ofBarfrom its dedicated file improves clarity and maintainability.
34-53: Configuration of separate stories for loading states is clear.
Each story is well-structured and provides a clear example of how the component behaves in different scenarios, such as loading, error, and fallback content states.unraid-ui/stories/components/common/ScrollArea.stories.ts (2)
1-3: Imports and file structure look fine.You're importing the correct types and components for Storybook usage. Structuring your import statements this way helps maintain clarity and ensures type safety with Vue 3.
5-9: Meta object is well-defined.Using the
satisfies Meta<typeof ScrollArea>syntax is a neat TypeScript approach to validate thatmetafulfills Storybook’s metadata requirements. Good job!
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
unraid-ui/src/components/common/sheet/sheet.variants.ts (1)
3-23: Consider adding type definitions for variant props to improve maintainability.Although
cvausage here is straightforward, explicitly defining TypeScript types forsideandpaddingmay increase clarity. For instance, exporting aSheetOptionstype that listssideandpaddingwould help ensure that the usage ofsheetVariantsremains consistent across your codebase.unraid-ui/src/components/common/sheet/SheetContent.vue (1)
36-36: Efficient property forwarding.Using
useForwardPropsEmitsis an efficient approach, but watch for potential name collisions if you add more props in the future. Consider centralizing definitions or employing specific names to mitigate conflicts.unraid-ui/stories/components/common/Sheet.stories.ts (1)
1-84: Add more interactive stories or controls for automatically verifying open/close animations.Though these static examples are satisfactory, consider adding interactive or combination stories to thoroughly test all variant states (e.g., changing
sideorpaddingvia Storybook controls). This helps ensure that styling variations remain consistent and discoverable under different states.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (4)
unraid-ui/src/components/common/sheet/SheetContent.vue(1 hunks)unraid-ui/src/components/common/sheet/index.ts(1 hunks)unraid-ui/src/components/common/sheet/sheet.variants.ts(1 hunks)unraid-ui/stories/components/common/Sheet.stories.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- unraid-ui/src/components/common/sheet/index.ts
🔇 Additional comments (4)
unraid-ui/src/components/common/sheet/SheetContent.vue (4)
13-13: Good job extracting variant logic into a separate file.This import clarifies the separation of concerns, making it easier to maintain and iterate on styling variants independently. Great practice!
18-19: Validate prop values at runtime if necessary.While TypeScript definitions protect at compile time, ensure that nonexistent prop values (e.g., an invalid side) won't cause runtime styling issues. Alternatively, consider validating them or providing a fallback in case side/padding does not match expected values.
22-22: Examine if removingElementmight introduce regressions.Ensure no existing part of the code relies on passing an
Elementobject as a validtodestination. Otherwise, removing this type might break code using older references.
Line range hint
41-59: Story usage is consistent with your new variant props.The usage of
sideandpaddingis coherent, and the Storybook examples illustrate the component’s possible configurations. This fosters clarity for your team or external consumers. Great job!Also applies to: 61-71, 73-81
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
unraid-ui/src/components/common/sheet/SheetContent.vue (2)
12-12: Good separation of concerns!Moving the sheet variants to a dedicated file improves modularity and maintainability.
47-51: Clean template structure with good accessibility!The implementation maintains proper accessibility attributes and transitions while simplifying prop handling.
Consider destructuring the disabled and forceMount props in the script section for better readability:
- <DialogPortal :disabled="disabled" :force-mount="forceMount" :to="to"> + <DialogPortal v-bind="{ disabled, forceMount, to }">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
unraid-ui/src/components/common/sheet/SheetContent.vue(1 hunks)
🔇 Additional comments (2)
unraid-ui/src/components/common/sheet/SheetContent.vue (2)
24-36: Well-structured component setup!The implementation shows good practices:
- Type-safe default props using withDefaults
- Clean computed property for class composition
- Proper use of the cn utility for class merging
15-21: Verify DialogContentProps properties are not needed.The interface no longer extends DialogContentProps. Ensure that no required props from DialogContentProps are being used in the component.
Consider using string union type constants.
The literal types for
sideandpaddingcould be moved to type constants for better reusability across components.+export type SheetSide = "top" | "bottom" | "left" | "right"; +export type SheetPadding = "none" | "md"; + export interface SheetContentProps { - side?: "top" | "bottom" | "left" | "right"; - padding?: "none" | "md"; + side?: SheetSide; + padding?: SheetPadding;✅ Verification successful
Removal of DialogContentProps extension is safe
After analyzing both implementations, I can confirm that:
- All necessary props from DialogContentProps are already included in the new interface
- The component correctly uses DialogContent, DialogPortal, and other Radix components
- The props forwarding mechanism (
delegatedPropsandforwarded) ensures all required props are passed to the underlying componentsThe changes actually improve the interface by:
- Making it more explicit about which props are actually used
- Removing unused inherited props that were coming from DialogContentProps
- Maintaining the same functionality with a cleaner interface
Regarding the suggestion about type constants: while it would be good for reusability, the current implementation aligns with the component's self-contained nature and matches how the props are used in the variants system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any DialogContentProps usage in the component ast-grep --pattern 'interface DialogContentProps { $$$ }'Length of output: 61
Script:
#!/bin/bash # Let's try a different approach to find DialogContentProps usage # First, search for any imports or references to DialogContentProps rg "DialogContentProps" -A 3 # Also search for any props actually used in the component rg -t vue "props:" -A 10 "SheetContent.vue"Length of output: 707
Script:
#!/bin/bash # Let's check the content of the SheetContent.vue file to see the props usage cat unraid-ui/src/components/common/sheet/SheetContent.vue # Also check the shadcn implementation for comparison cat web/components/shadcn/sheet/SheetContent.vueLength of output: 3753
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
unraid-ui/package.json (1)
57-62: Maintain consistent versioning strategyThe versioning strategy is inconsistent between dependencies:
- ESLint packages use caret ranges (
^)- Prettier uses a fixed version (
3.4.2)Consider using the same versioning strategy across all formatting/linting tools to make updates more predictable.
- "eslint": "^9.17.0", - "eslint-config-prettier": "^9.1.0", - "eslint-plugin-vue": "^9.32.0", - "prettier": "3.4.2", + "eslint": "^8.56.0", + "eslint-config-prettier": "^9.1.0", + "eslint-plugin-vue": "^9.32.0", + "prettier": "^3.4.2",unraid-ui/.storybook/main.ts (1)
20-25: Consider removing redundant PostCSS addonThe new
@storybook/addon-stylingwith PostCSS support is added while keeping the older@storybook/addon-postcss. Since both handle PostCSS processing, consider removing the older addon to avoid potential conflicts.addons: [ "@storybook/addon-links", "@storybook/addon-essentials", "@storybook/addon-interactions", "@storybook/addon-controls", - { - name: "@storybook/addon-postcss", - options: { - postcssLoaderOptions: { - implementation: require("postcss"), - }, - }, - }, { name: "@storybook/addon-styling", options: { postCss: true, }, }, ],unraid-ui/src/components/common/sheet/SheetContent.vue (2)
12-12: Good architectural decision!Moving variant definitions to a dedicated file improves code organization and maintainability.
31-36: Well-structured styling logic!The computed
sheetClassproperty effectively combines variants with custom classes. Consider adding a comment explaining the variant combinations for better maintainability.const sheetClass = computed(() => { + // Combines sheet variants (side/padding) with custom classes return cn( sheetVariants({ side: props.side, padding: props.padding }), props.class, ); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (1)
unraid-ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
.vscode/extensions.json(0 hunks).vscode/settings.json(0 hunks).vscode/sftp-template.json(0 hunks)unraid-ui/.eslintrc.js(1 hunks)unraid-ui/.prettierrc.json(1 hunks)unraid-ui/.storybook/main.ts(3 hunks)unraid-ui/.storybook/tailwind.config.ts(1 hunks)unraid-ui/package.json(1 hunks)unraid-ui/src/components/common/sheet/SheetContent.vue(1 hunks)unraid-ui/stories/components/common/Sheet.stories.ts(1 hunks)unraid-ui/tailwind.config.ts(1 hunks)
💤 Files with no reviewable changes (3)
- .vscode/settings.json
- .vscode/extensions.json
- .vscode/sftp-template.json
✅ Files skipped from review due to trivial changes (2)
- unraid-ui/.prettierrc.json
- unraid-ui/.eslintrc.js
🚧 Files skipped from review as they are similar to previous changes (1)
- unraid-ui/stories/components/common/Sheet.stories.ts
🔇 Additional comments (8)
unraid-ui/tailwind.config.ts (1)
9-13: LGTM! Verify Storybook story paths.The addition of Storybook story files to Tailwind's content configuration is appropriate and follows best practices. The pattern covers all common story file extensions.
Let's verify the story file paths are correct:
✅ Verification successful
Story paths in Tailwind config are correctly configured
The configuration correctly matches all story files, which are properly organized in
unraid-ui/stories/components/common/. The paths and patterns in the Tailwind config accurately cover all existing story files.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence and location of story files # Check if the stories directory exists and contains story files echo "Checking for story files..." fd -e stories.js -e stories.ts -e stories.jsx -e stories.tsx -e stories.mdx . stories/ # Verify no story files are missed in other locations echo "Checking for potentially missed story files..." fd -e stories.js -e stories.ts -e stories.jsx -e stories.tsx -e stories.mdx . src/Length of output: 529
Script:
#!/bin/bash # First, let's understand the repository structure echo "Repository structure:" fd --type d --max-depth 3 echo -e "\nLooking for story files anywhere in the repository:" fd -e stories.js -e stories.ts -e stories.jsx -e stories.tsx -e stories.mdx echo -e "\nLooking for package.json files to understand project structure:" fd package.jsonLength of output: 2099
unraid-ui/package.json (1)
57-62: Verify ESLint and Prettier configurationsThe addition of ESLint and Prettier dependencies suggests new code quality tooling. Please ensure that the corresponding configuration files (
.eslintrc.js,.prettierrc) are included in this PR.✅ Verification successful
Configuration files are properly included ✅
Both
.eslintrc.jsand.prettierrc.jsonconfiguration files are present in theunraid-uidirectory, matching the location where the dependencies were added.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for ESLint and Prettier config files echo "Checking for config files..." fd -t f "^\.eslintrc" && fd -t f "^\.prettierrc" # Check if the configs are referenced in package.json echo "Checking for config references in package.json..." jq '.eslintConfig, .prettier' package.jsonLength of output: 523
Script:
#!/bin/bash # Search for ESLint config files echo "ESLint config files:" fd -H -t f "^\.eslintrc\.(js|json|yaml|yml)$|^\.eslintrc$" ./ # Search for Prettier config files echo -e "\nPrettier config files:" fd -H -t f "^\.prettierrc\.(js|json|yaml|yml)$|^\.prettierrc$|^prettier\.config\.js$" ./ # Check package.json in unraid-ui directory echo -e "\nChecking package.json configs:" if [ -f "unraid-ui/package.json" ]; then jq '.eslintConfig, .prettier' unraid-ui/package.json fiLength of output: 498
unraid-ui/.storybook/tailwind.config.ts (1)
6-8: LGTM! Content paths are now more specific and comprehensive.The updated paths properly target Vue components and story files while maintaining a clean directory structure.
Let's verify that all component files are covered by these paths:
✅ Verification successful
✓ Content paths are correctly configured and all files are properly organized
The verification confirms that:
- All Vue components and related files are within the designated directories
- Story files are properly placed and follow the correct naming pattern
- No component files exist outside the specified paths
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all relevant files are covered by the Tailwind content paths # and that there are no component files outside the specified directories. echo "Files matching the configured content paths:" fd -e js -e vue -e ts . ../src/components ../src/composables fd -e js -e ts -e jsx -e tsx -e mdx -g "*stories*" ../stories echo -e "\nChecking for potentially missed component files:" # Look for Vue/JS/TS files outside the specified directories that might be components fd -e js -e vue -e ts . ../src -E "components/**" -E "composables/**" -x grep -l "export.*default.*{" {} \;Length of output: 813
Script:
#!/bin/bash # First, verify the directory structure echo "Directory structure:" ls -la unraid-ui/src ls -la unraid-ui/stories 2>/dev/null || echo "stories directory not found" echo -e "\nFiles matching the configured content paths:" # Check components and composables fd -e js -e vue -e ts . unraid-ui/src/components unraid-ui/src/composables 2>/dev/null || echo "No matching files found in components/composables" # Check story files fd -e js -e ts -e jsx -e tsx -e mdx -g "*stories*" unraid-ui/stories unraid-ui/src 2>/dev/null || echo "No story files found" echo -e "\nChecking for potentially missed component files:" # Look for Vue/JS/TS files outside the specified directories that might be components fd -e js -e vue -e ts . unraid-ui/src -E "components/**" -E "composables/**" -x grep -l "export.*default.*{" {} \; 2>/dev/null || echo "No potential component files found outside designated directories"Length of output: 5858
unraid-ui/.storybook/main.ts (1)
46-53: Verify Tailwind configuration file and consider using absolute pathThe PostCSS configuration references a Tailwind config file. Let's verify its existence and consider using an absolute path for better reliability.
Consider using an absolute path for better reliability:
require('tailwindcss')({ - config: './tailwind.config.ts' + config: resolve(__dirname, '../tailwind.config.ts') }),unraid-ui/src/components/common/sheet/SheetContent.vue (4)
15-17: Well-structured prop definitions!Using literal unions instead of importing types makes the interface more explicit and self-contained.
24-26: Good use of withDefaults!Using
withDefaultsimproves type safety and makes default values explicit.
21-21: Verify the impact of removingElementtype fromtopropThe
toprop type has been simplified to only acceptstring | HTMLElement. Ensure this change doesn't break existing usage whereElementtype might have been used.✅ Verification successful
Type change to
string | HTMLElementis safe ✅The codebase consistently uses either string values or
HTMLElementtypes for thetoprop. No instances of using the baseElementtype were found, making this type definition change more precise without breaking existing usage.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usage of SheetContent with 'to' prop rg -l "SheetContent.*to=" --type vue # Search for any Element type usage with SheetContent ast-grep --pattern 'SheetContent $$$to={$_: Element}$$$'Length of output: 125
Script:
#!/bin/bash # Search for SheetContent usage with context rg -p "SheetContent" --glob "*.vue" -B 2 -A 2 # Search for Element type imports rg "import.*Element.*from" --glob "*.{vue,ts,js}" # Search specifically for to prop in vue files rg "to=[\"'{]" --glob "*.vue" -B 1 -A 1Length of output: 5682
47-47: Clean template implementation!The simplified prop binding and direct use of computed properties improve readability. However, verify that removing type casting from the
toprop binding doesn't cause type inference issues in the build process.Also applies to: 51-51
✅ Verification successful
Type safety is maintained through radix-vue's type system
The
toprop is properly typed by the radix-vue library (v1.9.11), making explicit type casting unnecessary. The implementation follows the standard portal pattern used across the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any type-related build errors in the logs rg "TS2322|TS2345|TS2739" --type logLength of output: 187
Script:
#!/bin/bash # Find DialogPortal component definition and type information ast-grep --pattern 'interface DialogPortalProps { $$$ }' # Find all DialogPortal usages rg '<DialogPortal.*to=' -A 2 # Look for type definitions related to DialogPortal rg 'DialogPortal.*Props' --type typescriptLength of output: 593
Script:
#!/bin/bash # Find DialogPortal import rg "import.*DialogPortal" --type vue # Check package.json for UI-related dependencies cat package.json | grep -A 10 '"dependencies":'Length of output: 161
Script:
#!/bin/bash # Check package.json in the unraid-ui directory cat unraid-ui/package.json | grep -A 10 '"dependencies":' # Find any references to DialogPortal rg "DialogPortal" -A 2 --type-add 'vue:*.vue' --type vue # Find import statements in Vue files rg "import.*from.*Dialog" -A 1 --type-add 'vue:*.vue' --type vueLength of output: 1847
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (16)
unraid-ui/stories/components/common/Tooltip.stories.ts (3)
1-10: Consider using path aliases for importsThe relative imports (../../../) can become difficult to maintain. Consider setting up path aliases for better maintainability.
Example configuration:
-import { Tooltip as TooltipComponent, TooltipTrigger, TooltipContent, TooltipProvider } from "../../../src/components/common/tooltip"; -import { Button } from "../../../src/components/common/button"; +import { Tooltip as TooltipComponent, TooltipTrigger, TooltipContent, TooltipProvider } from "@/components/common/tooltip"; +import { Button } from "@/components/common/button";
26-26: Consider extracting Tailwind classes to component stylesDirect usage of utility classes like
p-20 flex items-center justify-startin the story might not reflect real-world usage. Consider moving these to the component's default styling.
14-41: Add JSDoc documentation for the storyInclude documentation about the tooltip's behavior, props, and usage examples.
+/** + * Demonstrates a basic tooltip triggered by hovering over a button. + * The tooltip can be configured to be open by default using the `defaultOpen` prop. + * + * @example + * <Tooltip defaultOpen={false}> + * <TooltipTrigger><Button>Hover me</Button></TooltipTrigger> + * <TooltipContent>Add to library</TooltipContent> + * </Tooltip> + */ export const Tooltip: Story = {unraid-ui/stories/components/form/Select.stories.ts (2)
39-39: Avoid hardcoding width classes in story templates.The hardcoded
w-[180px]class reduces component reusability. Consider making it configurable through story args.- <SelectTrigger class="w-[180px]"> + <SelectTrigger :class="args.triggerClass">Then add to the story configuration:
args: { triggerClass: 'w-[180px]' }, argTypes: { triggerClass: { control: 'text', description: 'CSS classes for the trigger element' } }
21-55: Enhance story documentation and controls.Consider adding:
- Story parameters with component documentation
- Controls for customization (e.g., placeholder text, disabled state)
- Documentation about keyboard navigation and accessibility features
Example enhancement:
export const Select: Story = { args: { placeholder: 'Select a fruit', disabled: false, triggerClass: 'w-[180px]' }, argTypes: { placeholder: { control: 'text' }, disabled: { control: 'boolean' }, triggerClass: { control: 'text' } }, parameters: { docs: { description: { component: 'A customizable select component that supports keyboard navigation and screen readers.', } } }, render: (args) => ({ // ... existing render code with args usage }) };unraid-ui/stories/components/common/Tabs.stories.ts (3)
4-7: Consider a more specific story title.The current title "Components/Common" is quite generic. Consider making it more specific to tabs, such as "Components/Common/Tabs" to improve navigation in Storybook.
const meta = { - title: "Components/Common", + title: "Components/Common/Tabs", component: TabsComponent, } satisfies Meta<typeof TabsComponent>;
13-21: Enhance story with additional configurations.Consider adding more stories to demonstrate different tab configurations:
- Disabled state for tabs
- Different styling variants
- Controlled mode with v-model
- Error states
Example implementation:
export const DisabledTabs: Story = { args: { defaultValue: "tab1", disabled: true, }, // ... render implementation }; export const ControlledTabs: Story = { args: { modelValue: "tab1", }, // ... render implementation with v-model };
22-39: Improve template implementation for better demonstration.Several improvements could enhance the story:
- The hardcoded width (
w-[400px]) limits reusability- Tab values could be more semantic (e.g., "account" instead of "tab1")
- Content could better demonstrate real-world usage
- <TabsComponent :default-value="args.defaultValue" class="w-[400px]"> + <TabsComponent :default-value="args.defaultValue" :class="args.className"> <TabsList> - <TabsTrigger value="tab1">Account</TabsTrigger> - <TabsTrigger value="tab2">Password</TabsTrigger> - <TabsTrigger value="tab3">Settings</TabsTrigger> + <TabsTrigger value="account">Account</TabsTrigger> + <TabsTrigger value="password">Password</TabsTrigger> + <TabsTrigger value="settings">Settings</TabsTrigger> </TabsList> - <TabsContent value="tab1"> - <div class="p-4">Account settings content</div> + <TabsContent value="account"> + <div class="space-y-4 p-4"> + <h3 class="text-lg font-medium">Account Settings</h3> + <div class="grid gap-2"> + <label>Username</label> + <input type="text" placeholder="Enter username" /> + </div> + </div> </TabsContent> // ... similar improvements for other tabsunraid-ui/stories/components/form/Label.stories.ts (1)
4-7: Update story title for better organizationConsider updating the title to be more specific, like "Components/Form/Label" to maintain consistency with the Input story structure.
const meta = { - title: "Components/Form", + title: "Components/Form/Label", component: LabelComponent, } satisfies Meta<typeof LabelComponent>;unraid-ui/stories/components/form/Input.stories.ts (1)
24-24: Add newline at end of fileAdd a newline at the end of the file to follow standard coding practices.
unraid-ui/stories/components/layout/CardWrapper.stories.ts (1)
13-62: Consider reducing code duplication in story implementations.The render functions share identical setup code and component registration. Consider extracting the common logic into a shared template or helper function.
const createStoryTemplate = (title: string, description: string) => ` <CardWrapperComponent v-bind="args"> <h3 class="text-lg font-semibold mb-2">${title}</h3> <p>${description}</p> </CardWrapperComponent> `; const createStory = (template: string): Story['render'] => (args) => ({ components: { CardWrapperComponent }, setup() { return { args }; }, template, });🧰 Tools
🪛 Biome (1.9.4)
[error] 28-28: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
unraid-ui/stories/components/layout/PageContainer.stories.ts (3)
1-4: Optimize component imports.Consider combining the layout component imports into a single statement for better maintainability.
import type { Meta, StoryObj } from "@storybook/vue3"; -import { PageContainer as PageContainerComponent } from "../../../src/components/layout"; -import { CardWrapper } from "../../../src/components/layout"; +import { PageContainer as PageContainerComponent, CardWrapper } from "../../../src/components/layout";
21-33: Extract common styling into a wrapper component.The background styling (
bg-muted/20 p-4) is duplicated across stories. Consider extracting it into a wrapper component or decorator to maintain consistency and reduce duplication.const withBackground = (storyFn) => ({ components: { Story: storyFn }, template: '<div class="bg-muted/20 p-4"><Story /></div>', }); // Apply to stories: export const PageContainer: Story = { decorators: [withBackground], // ... rest of the story configuration };Also applies to: 47-55
5-8: Enhance story documentation.Consider adding component documentation using Storybook parameters:
- Document available props and their types
- Provide usage guidelines
- Include examples of common use cases
const meta = { title: "Components/Layout/PageContainer", component: PageContainerComponent, + parameters: { + docs: { + description: { + component: 'A container component that provides consistent max-width and spacing for page content.', + }, + }, + }, + argTypes: { + maxWidth: { + description: 'Custom max-width using Tailwind classes', + control: 'text', + table: { + type: { summary: 'string' }, + defaultValue: { summary: 'max-w-7xl' }, + }, + }, + }, } satisfies Meta<typeof PageContainerComponent>;unraid-ui/stories/components/form/Lightswitch.stories.ts (2)
1-2: Consider using more descriptive component aliasThe alias
LightswitchComponentis good for clarity, but consider using a more specific name likeBaseLightswitchorLightswitchControlto better indicate its role in the component hierarchy.
13-26: Add more story variants and documentationThe story only includes a basic usage example. Consider adding:
- Different states (checked/unchecked)
- Disabled state
- Error state
- Different label positions
- Component description and usage guidelines
Example enhancement:
export const Lightswitch: Story = { + parameters: { + docs: { + description: { + component: 'A toggle switch component for boolean inputs with customizable label.', + }, + }, + }, args: { label: "Enable notifications", }, render: (args) => ({ components: { LightswitchComponent }, setup() { return { args }; }, template: ` <LightswitchComponent v-bind="args" /> `, }), }; + +export const Checked: Story = { + args: { + label: "Enable notifications", + modelValue: true, + }, +}; + +export const Disabled: Story = { + args: { + label: "Enable notifications", + disabled: true, + }, +};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (9)
unraid-ui/stories/components/common/Tabs.stories.ts(1 hunks)unraid-ui/stories/components/common/Tooltip.stories.ts(1 hunks)unraid-ui/stories/components/form/Input.stories.ts(1 hunks)unraid-ui/stories/components/form/Label.stories.ts(1 hunks)unraid-ui/stories/components/form/Lightswitch.stories.ts(1 hunks)unraid-ui/stories/components/form/Select.stories.ts(1 hunks)unraid-ui/stories/components/form/Switch.stories.ts(1 hunks)unraid-ui/stories/components/layout/CardWrapper.stories.ts(1 hunks)unraid-ui/stories/components/layout/PageContainer.stories.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
unraid-ui/stories/components/layout/CardWrapper.stories.ts
[error] 28-28: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build Web App
- GitHub Check: Build and Test API
🔇 Additional comments (8)
unraid-ui/stories/components/common/Tooltip.stories.ts (2)
12-12: LGTM! Type definition is well structuredThe Story type is properly defined using TypeScript generics for type safety.
24-26: Clarify the purpose of the modals divThe
<div id="modals"></div>seems disconnected from the tooltip functionality. If it's required for portal rendering, please add a comment explaining its purpose.unraid-ui/stories/components/form/Select.stories.ts (1)
1-20: LGTM! Well-structured Storybook configuration.The imports and meta configuration follow Storybook best practices with proper type safety.
unraid-ui/stories/components/common/Tabs.stories.ts (2)
1-2: LGTM! Well-structured imports.The imports are properly organized, using explicit type imports and clean component destructuring.
1-41: Verify accessibility implementation.Please ensure that the tabs implementation follows accessibility best practices:
- Proper ARIA roles and attributes
- Keyboard navigation support
- Focus management
Run the following to check for accessibility-related implementations:
unraid-ui/stories/components/layout/CardWrapper.stories.ts (1)
1-12: LGTM! Well-structured story setup.The imports and metadata configuration follow Storybook best practices with proper type definitions.
unraid-ui/stories/components/layout/PageContainer.stories.ts (1)
37-41: Verify maxWidth prop type validation.The
maxWidthprop accepts a Tailwind class directly. Ensure that the component has proper prop validation to prevent invalid class names.unraid-ui/stories/components/form/Lightswitch.stories.ts (1)
4-8: Verify component title hierarchyThe title path "Components/Form/Lightswitch" implies this is a form control, but the component might be better categorized under "Components/Inputs" or "Components/Controls" depending on your UI taxonomy.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
unraid-ui/.storybook/main.ts (1)
25-27: Consider simplifying path aliases resolutionThe current path resolution is correct but could be simplified by extracting the common base path.
+ const basePath = dirname(new URL(import.meta.url).pathname); resolve: { alias: { - "@": join(dirname(new URL(import.meta.url).pathname), "../src"), - "@/components": join(dirname(new URL(import.meta.url).pathname), "../src/components"), - "@/lib": join(dirname(new URL(import.meta.url).pathname), "../src/lib"), + "@": join(basePath, "../src"), + "@/components": join(basePath, "../src/components"), + "@/lib": join(basePath, "../src/lib"), }, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (1)
unraid-ui/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
unraid-ui/.storybook/main.ts(2 hunks)unraid-ui/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- unraid-ui/package.json
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build Web App
- GitHub Check: Build and Test API
🔇 Additional comments (2)
unraid-ui/.storybook/main.ts (2)
5-5: Remove unnecessary JSX/TSX extensions from stories patternThe stories pattern includes JSX/TSX extensions, but let's verify if they are actually needed.
Let's check the current story file extensions:
#!/bin/bash # Check current story file extensions in use echo "Story files by extension:" fd -t f . stories -x sh -c 'echo "{}" | grep -o "\.[^.]*$"' | sort | uniq -c
31-38: Verify tailwind configuration pathThe PostCSS configuration looks good, but let's verify the tailwind config path.
✅ Verification successful
Tailwind configuration path is correct ✅
The relative path "./tailwind.config.ts" in the Storybook configuration correctly resolves to the existing tailwind configuration at "unraid-ui/tailwind.config.ts".
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify tailwind configuration echo "Checking tailwind config location:" fd -t f "tailwind.config.ts" echo -e "\nChecking for any other tailwind configs:" fd -t f "tailwind.config"Length of output: 334
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
web/components/UpdateOs.ce.vue (1)
Line range hint
1-20: Address critical TODO items regarding OS updates.The TODO comments indicate missing functionality for:
- Updating test, beta, and stable releases for internal testing
- Handling third-party drivers after install/downgrade
These are critical features that should be implemented before deployment.
Would you like me to help create GitHub issues to track these tasks or assist in implementing these features?
web/pages/index.vue (1)
Line range hint
77-77: Fix the ternary expression in the click handlerThe static analysis tool correctly identified an issue. The ternary expression should be wrapped in parentheses for clarity.
- props.showExternalDowngrade ? accountStore.downgradeOs() : accountStore.updateOs(); + (props.showExternalDowngrade ? accountStore.downgradeOs : accountStore.updateOs)();
🧹 Nitpick comments (18)
web/types/unraid-ui.d.ts (1)
2-7: Consider enhancing type declarations for better maintainability.While the current declaration is functional, consider these improvements:
- Add version information in JSDoc
- Consider using a namespace to prevent potential naming conflicts
- Add explicit type exports for component props
Example enhancement:
declare module 'unraid-ui' { import type { Component } from 'vue'; + import type { BadgeProps } from './components/common/badge/Badge.vue'; export const Badge: Component; + export type { BadgeProps }; // Add components as needed }unraid-ui/stories/components/brand/BrandLoadingWhite.stories.ts (2)
4-17: Consider adding interactive controls for better story configuration.The story setup is good, but could be enhanced with interactive controls for better component testing.
Add argTypes to enable interactive controls:
const meta = { title: "Components/Brand", component: BrandLoadingWhite, + argTypes: { + size: { + control: { type: 'range', min: 100, max: 300, step: 10 }, + defaultValue: 200 + } + }, decorators: [ () => ({ template: '<div class="bg-black p-8 rounded-md"><story/></div>', }), ], parameters: { backgrounds: { default: 'dark', }, }, } satisfies Meta<typeof BrandLoadingWhite>;
23-32: Consider making the container width dynamic.The fixed width of 200px might not cover all use cases. Consider making it configurable through story args.
export const LoadingWhite: Story = { + args: { + width: 200 + }, render: () => ({ components: { BrandLoadingWhite }, + props: ['width'], template: ` - <div class="w-[200px]"> + <div :style="{ width: width + 'px' }"> <BrandLoadingWhite /> </div> `, }), };unraid-ui/stories/components/brand/BrandButton.stories.ts (2)
13-19: Enhance type safety for button arguments.The args could benefit from better type definitions and validation.
Add argTypes with proper controls and validation:
+const meta = { + title: "Components/Brand", + component: BrandButton, + argTypes: { + variant: { + control: 'select', + options: ['fill', 'outline', 'ghost'], + description: 'Button variant style' + }, + size: { + control: 'select', + options: ['12px', '14px', '16px'], + description: 'Button text size' + }, + padding: { + control: 'select', + options: ['default', 'compact', 'none'], + description: 'Button padding preset' + } + } +} satisfies Meta<typeof BrandButton>;
20-34: Simplify the story template using args spreading.The current template explicitly binds each prop. Consider using v-bind spreading for cleaner code.
render: (args) => ({ components: { BrandButton }, setup() { return { args }; }, template: ` - <BrandButton - :variant="args.variant" - :size="args.size" - :padding="args.padding" - :text="args.text" - :class="args.class" - /> + <BrandButton v-bind="args" /> `, }),unraid-ui/stories/components/brand/BrandLogo.stories.ts (1)
17-36: Consider adding more story variantsWhile the basic story is well implemented, consider adding variants to showcase different use cases:
- Dark background variant
- Different sizes
- Common gradient presets
unraid-ui/stories/components/brand/BrandLogoConnect.stories.ts (1)
4-11: Consider extracting common story configurationThe story configuration is nearly identical to BrandLogo.stories.ts. Consider creating a shared configuration for brand components:
- Common gradient controls
- Shared width constraints
- Reusable story templates
Example approach:
// brandStoryConfig.ts export const createBrandMeta = (component: Component) => ({ title: "Components/Brand", component, argTypes: { gradientStart: { control: 'color' }, gradientStop: { control: 'color' }, }, }); export const defaultGradientArgs = { gradientStart: '#e32929', gradientStop: '#ff8d30', };Also applies to: 17-36
unraid-ui/stories/components/brand/BrandLoading.stories.ts (1)
30-30: Standardize width constraints across brand storiesThe width constraint (200px) differs from other brand stories (300px). Consider standardizing these constraints or documenting why they differ.
unraid-ui/src/components/brand/BrandLogo.vue (2)
2-10: Add prop documentationConsider adding JSDoc comments to document the Props interface:
/** * Props for the BrandLogo component * @interface Props */ export interface Props { /** Starting color of the gradient. Default: #e32929 */ gradientStart?: string; /** Ending color of the gradient. Default: #ff8d30 */ gradientStop?: string; }
1-38: Consider adding color validationThe component accepts any string for gradient colors. Consider adding validation to ensure valid color values:
const isValidColor = (color: string) => /^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})$/.test(color); const props = withDefaults(defineProps<Props>(), { gradientStart: '#e32929', gradientStop: '#ff8d30', }); // Add validation watch(() => props.gradientStart, (value) => { if (!isValidColor(value)) { console.warn(`Invalid color value for gradientStart: ${value}`); } }); watch(() => props.gradientStop, (value) => { if (!isValidColor(value)) { console.warn(`Invalid color value for gradientStop: ${value}`); } });unraid-ui/src/components/brand/BrandLogoConnect.vue (1)
14-44: Consider optimizing SVG gradient definitions.The SVG contains multiple similar linear gradients (b through h) that reference gradient 'a'. Consider consolidating these definitions to reduce redundancy and improve maintainability.
- <linearGradient id="b" xlink:href="#a" x2="923.39" /> - <linearGradient id="c" xlink:href="#a" x2="923.39" y1="71.2" y2="71.2" /> - <linearGradient id="d" xlink:href="#a" x2="923.39" y1="71.2" y2="71.2" /> - <linearGradient id="e" xlink:href="#a" x2="923.39" y1="71.2" y2="71.2" /> - <linearGradient id="f" xlink:href="#a" x2="923.39" /> - <linearGradient id="g" xlink:href="#a" y1="12.16" y2="12.16" /> - <linearGradient id="h" xlink:href="#a" x2="923.39" y1="86.94" y2="86.94" /> + <linearGradient id="main-gradient" x1="-57.82" x2="923.39" y1="71.2" y2="71.2" gradientUnits="userSpaceOnUse"> + <stop offset="0" :stop-color="gradientStart" /> + <stop offset="1" :stop-color="gradientStop" /> + </linearGradient>Then update the path fills to use
url(#main-gradient).unraid-ui/src/components/brand/BrandButton.vue (1)
16-17: Avoid using 'any' type for icon props.Consider defining a more specific type for icon components to improve type safety.
- icon?: any; - iconRight?: any; + icon?: Component; + iconRight?: Component;Don't forget to import the Component type:
+import type { Component } from 'vue';unraid-ui/src/components/brand/BrandLoading.vue (1)
86-133: Consider performance optimization for animationsThe animation implementation is clean, but consider adding
will-change: transformto the animated elements for better performance on low-end devices..unraid_mark_2, .unraid_mark_4 { animation: mark_2 1.5s ease infinite; + will-change: transform; } .unraid_mark_3 { animation: mark_3 1.5s ease infinite; + will-change: transform; } .unraid_mark_6, .unraid_mark_8 { animation: mark_6 1.5s ease infinite; + will-change: transform; } .unraid_mark_7 { animation: mark_7 1.5s ease infinite; + will-change: transform; }web/components/UpdateOs/Status.vue (1)
68-70: Consider using more descriptive computed property nameThe name
showRebootButtoncould be more descriptive about what it checks.-const showRebootButton = computed( +const isRebootRequired = computed( () => rebootType.value === 'downgrade' || rebootType.value === 'update' );web/package.json (1)
88-88: Consider publishing unraid-ui package to a private registryUsing a local file dependency (
file:../unraid-ui) can be problematic in production environments. While it works for development, consider publishing this package to a private npm registry for production use.web/components/UserProfile/CallbackFeedback.vue (3)
Line range hint
147-235: Consider extracting complex computed properties into separate composablesThe
keyInstallStatusCopyandaccountActionStatusCopycomputed properties contain complex conditional logic and string manipulations. Consider extracting these into separate composables for better maintainability and reusability.Example structure:
// useKeyInstallStatus.ts export function useKeyInstallStatus(props, keyActionType, keyType) { return computed(() => { const translations = { installing: { trialExtend: 'Installing Extended Trial', recover: 'Installing Recovered', // ... other translations } // ... other states } return { text: getTranslatedText(keyInstallStatus.value, keyActionType.value, keyType.value, translations, props.t) } }) }
313-317: Simplify complex conditional expressionThe nested v-if condition is hard to read. Consider extracting this logic into a computed property.
- v-if=" - stateDataError && - callbackStatus !== 'loading' && - (keyInstallStatus === 'success' || keyInstallStatus === 'failed') - " + v-if="shouldShowStateDataError"Add this computed property:
const shouldShowStateDataError = computed(() => stateDataError.value && callbackStatus.value !== 'loading' && ['success', 'failed'].includes(keyInstallStatus.value) )
Line range hint
427-476: Consider moving animations to a separate fileThe animation keyframes could be moved to a separate file in the unraid-ui package for better organization and reusability across components.
Consider creating
unraid-ui/styles/animations.cssand importing it here:/* Import unraid-ui globals first */ @import 'unraid-ui/styles'; +@import 'unraid-ui/styles/animations';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (1)
web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (23)
unraid-ui/package.json(3 hunks)unraid-ui/src/components/brand/BrandButton.vue(1 hunks)unraid-ui/src/components/brand/BrandLoading.vue(1 hunks)unraid-ui/src/components/brand/BrandLoadingWhite.vue(1 hunks)unraid-ui/src/components/brand/BrandLogo.vue(1 hunks)unraid-ui/src/components/brand/BrandLogoConnect.vue(1 hunks)unraid-ui/src/components/brand/brand-button.variants.ts(1 hunks)unraid-ui/src/components/brand/index.ts(1 hunks)unraid-ui/src/components/common/badge/badge.variants.ts(1 hunks)unraid-ui/src/index.ts(4 hunks)unraid-ui/src/styles/index.css(1 hunks)unraid-ui/stories/components/brand/BrandButton.stories.ts(1 hunks)unraid-ui/stories/components/brand/BrandLoading.stories.ts(1 hunks)unraid-ui/stories/components/brand/BrandLoadingWhite.stories.ts(1 hunks)unraid-ui/stories/components/brand/BrandLogo.stories.ts(1 hunks)unraid-ui/stories/components/brand/BrandLogoConnect.stories.ts(1 hunks)web/components/UpdateOs.ce.vue(3 hunks)web/components/UpdateOs/Status.vue(5 hunks)web/components/UserProfile/CallbackFeedback.vue(13 hunks)web/package.json(1 hunks)web/pages/index.vue(2 hunks)web/tailwind.config.ts(1 hunks)web/types/unraid-ui.d.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- unraid-ui/src/components/brand/BrandLoadingWhite.vue
- unraid-ui/src/styles/index.css
🚧 Files skipped from review as they are similar to previous changes (3)
- unraid-ui/src/components/common/badge/badge.variants.ts
- unraid-ui/src/index.ts
- unraid-ui/package.json
🧰 Additional context used
🪛 GitHub Check: Build Web App
web/components/UpdateOs/Status.vue
[failure] 77-77:
Expected an assignment or function call and instead saw an expression
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and Test API
🔇 Additional comments (15)
unraid-ui/src/components/brand/index.ts (1)
1-6: Well-structured exports following barrel pattern!The exports are well-organized, with clear separation between components and their variants. The naming is consistent and follows good practices.
unraid-ui/stories/components/brand/BrandLogo.stories.ts (1)
4-11: LGTM! Well-structured Storybook configuration.The meta configuration properly defines the component and its controls. The color controls for gradients provide good interactivity for testing different brand variations.
unraid-ui/stories/components/brand/BrandLoading.stories.ts (1)
7-11: LGTM! Well-structured controlsGood addition of the title control alongside gradient customization.
unraid-ui/src/components/brand/BrandLogoConnect.vue (1)
2-10: LGTM! Well-structured props interface.The TypeScript interface with optional props and default values is well implemented.
unraid-ui/src/components/brand/BrandButton.vue (1)
52-91: LGTM! Well-structured component template.The template implementation is clean and follows best practices:
- Dynamic component rendering
- Proper attribute binding
- Accessibility considerations with proper ARIA attributes
unraid-ui/src/components/brand/brand-button.variants.ts (1)
8-17: Verify color contrast ratios for accessibility.Some color combinations (especially in the gray and outline variants) might not meet WCAG 2.1 AA contrast requirements. Please verify the contrast ratios using a color contrast checker.
web/components/UpdateOs.ce.vue (1)
75-76: LGTM! Clean migration to unraid-ui styles.Good job on migrating to the unified styling system from unraid-ui.
unraid-ui/src/components/brand/BrandLoading.vue (2)
2-12: Props interface looks good!The Props interface is well-defined with TypeScript, and the default values are appropriately set using
withDefaults.
16-24: Great accessibility implementation!The SVG includes proper ARIA attributes (
role="img") and descriptive elements (<title>and<desc>).web/pages/index.vue (1)
128-148: Good migration to the new Badge componentThe implementation of the new Badge component from unraid-ui is consistent and follows the new variant-based approach.
web/components/UpdateOs/Status.vue (1)
125-127: Clean migration to the new Badge componentThe migration from UiBadge to Badge is consistent throughout the file. The variant prop is correctly used instead of color.
Also applies to: 136-143, 145-152, 154-156, 158-170, 171-173, 176-178
web/tailwind.config.ts (2)
4-8: Good use of Tailwind presetsThe integration of unraidUiConfig as a preset ensures consistent styling across the application.
10-11: Verify custom element stylesThe addition of
.ce.{js,vue,ts}pattern suggests custom elements. Ensure that the styles are properly scoped and don't leak into the global scope.✅ Verification successful
Custom element styles are properly scoped
The custom elements follow Vue.js and Tailwind CSS best practices for style scoping through:
- Proper use of PostCSS and Tailwind directives
- Component-specific styles with namespaced classes
- Tailwind's utility-first approach preventing global style leaks
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential style leakage in custom elements fd -e ce.vue -e ce.ts -e ce.js . ./components -x grep -l "class="Length of output: 165
Script:
#!/bin/bash # Search for custom element files in the entire repository echo "=== Custom Element Files ===" fd -e ce.vue -e ce.ts -e ce.js # If any files found, check their content for style usage echo -e "\n=== Style Usage in Custom Elements ===" fd -e ce.vue -e ce.ts -e ce.js -x cat {} \; 2>/dev/nullLength of output: 39048
Script:
#!/bin/bash # Check for non-scoped style blocks in custom elements that could leak echo "=== Non-scoped styles in Custom Elements ===" fd -e ce.vue . web/components -x grep -l "<style.*>" {} \; | while read -r file; do echo "=== $file ===" sed -n "/<style.*>/,/<\/style>/p" "$file" doneLength of output: 5906
web/components/UserProfile/CallbackFeedback.vue (2)
Line range hint
1-42: LGTM! Clean import organization and store setupThe imports are well-organized and the store setup follows Vue 3 composition API best practices.
424-425: LGTM! Proper import of unraid-ui globalsThe replacement of Tailwind imports with unraid-ui globals is correct and aligns with the UI migration.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
unraid-ui/package.json (1)
58-58:⚠️ Potential issueIncorrect ESLint version specified
The ESLint version
^9.17.0appears to be incorrect as ESLint is currently at major version 8.x. This will cause installation failures.Update to the latest stable 8.x version:
- "eslint": "^9.17.0", + "eslint": "^8.56.0",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
unraid-ui/package.json(3 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
unraid-ui/package.json
[error] 41-41: Expected a property but instead found '}'.
Expected a property here.
(parse)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and Test API
🔇 Additional comments (2)
unraid-ui/package.json (2)
43-45: Improved Storybook configurationThe migration from individual addons to the essentials bundle is a good improvement. The
@storybook/addon-essentialspackage includes commonly used addons, which helps standardize the development experience.
78-78: Verify the new styles export pathThe export path has been changed from
./dist/style.cssto./src/styles/index.css. Ensure that:
- The build process correctly handles this source file
- All consumers of the package are updated to use the new import path
✅ Verification successful
Export path change is correct and properly configured
The new path
./src/styles/index.cssis valid and correctly configured:
- File exists at the specified location
- Vite's build configuration properly handles CSS processing
- Aligns with the project's source structure
Note: Consider updating references in README.md if they mention the old path.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the new styles path exists and is being processed during build fd -t f "index.css" src/styles/ # Look for any remaining references to the old style.css path rg -l "style\.css" -g '!package.json'Length of output: 215
Script:
#!/bin/bash # Find all CSS files in the repository fd -e css # Check vite config content cat unraid-ui/vite.config.ts # Look for style imports rg -l "import.*\.css"Length of output: 2292
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/main.yml (1)
134-136: Consider workspace optimization for dependency installationWhile the current implementation works, consider using npm workspaces or a more structured approach for managing dependencies between the web and unraid-ui projects.
- - name: Install unraid-ui dependencies - run: cd ../unraid-ui && npm install + - name: Install dependencies + run: | + npm install -w unraid-ui + npm install -w webThis approach would:
- Eliminate the need for directory changes
- Make the dependency relationship more explicit
- Enable better dependency deduplication
- Simplify future maintenance
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
.github/workflows/main.yml(1 hunks)unraid-ui/package.json(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- unraid-ui/package.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and Test API
🔇 Additional comments (2)
.github/workflows/main.yml (2)
129-131: LGTM: Cache configuration updated for unraid-ui dependenciesThe cache-dependency-path configuration has been correctly updated to include the unraid-ui package-lock.json, which will improve build performance by caching dependencies for both web and unraid-ui projects.
134-136: Verify build process with new unraid-ui dependenciesPlease ensure that:
- The web project's build process correctly resolves dependencies from unraid-ui
- There are no conflicts between web and unraid-ui node_modules
- The build completes successfully in CI environment
Run this script to analyze the dependency structure:
✅ Verification successful
Dependencies are correctly structured ✅
The dependency setup is correct with
unraid-uiproperly referenced as a local package in the web project's dependencies. The UNMET DEPENDENCIES are normal and will be resolved during the build process whennpm installis executed.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Analyze dependency relationships between web and unraid-ui projects # Check for potential dependency conflicts echo "Checking for dependency conflicts..." cd web && npm ls 2>&1 | grep -i "UNMET\|CONFLICT" # Verify unraid-ui is properly linked echo "Verifying unraid-ui integration..." cd web && npm ls unraid-ui || true # Check build output for any UI-related errors echo "Analyzing recent build logs for UI-related errors..." gh run list --workflow=main.yml --limit=5 --json conclusion,url | \ jq -r '.[] | select(.conclusion=="failure") | .url' | \ xargs -I {} gh run view {} --log | grep -i "unraid-ui\|failed to resolve\|cannot find module"Length of output: 3375
… badge in HeadOsVersion
…et to consume in web
* feat: begin fixing dark mode in the webcomponents * feat: lots of progress on colors * feat: begin nuking alpha beta gamma * feat: set background color on webcomponents * fix: more color work * feat: eliminate all alpha beta gamma variable usage * feat: move variable declarations to theme.ts * feat: begin fixing dark mode in the webcomponents * feat: lots of progress on colors * feat: begin nuking alpha beta gamma * feat: set background color on webcomponents * fix: more color work * feat: eliminate all alpha beta gamma variable usage * feat: download nodejs and install on legacy OS versions * feat: do not move upgradepkg * feat: array iteration for restoring files * feat: separate install process * feat: extract node to usr/local/ * feat: move variable declarations to theme.ts * feat: remove nghttp3 and only bundle nodejs * feat: validate entries correctly * fix: upgradepkg * feat: copy only needed files for nodejs * fix: install syntax error * fix: strip components from tar line * feat: error when nodejs download fails * feat(php): add OEM data extraction functionality * feat(web): WIP add OEM activation modal and integrate OEM data handling * refactor: replace oem etamology with activation * feat(web): enhance activation modal with header and main title options * refactor(web): update activation modal title to handle partner name conditionally * feat: make partnerName optional in activation code data * refactor: remove activationCodeStore from UserProfile component * feat: integrate activation code data into server store * feat: enhance activation code store to include callback actions and improve modal visibility logic * refactor: remove unused theme reference from ColorSwitcher component * fix: update partnerName in activationCodeData for clarity * refactor: adjust gap spacing in Modal component for improved layout * feat: enhance Activation Modal component with dynamic title and description, and add documentation buttons * feat: implement localization for Activation Modal component titles and button texts * feat: add new translations for activation prompts and welcome messages in the WebComponentTranslations class * feat: implement key sequence to close Activation Modal and persist visibility state * feat: update activation logic to conditionally activate or redeem based on activation code presence * feat(plg): plg install / remove partner banner * feat(plg): on install update config to enable display settings banner and on remove banner setting if no custom banner used * fix(plg): plg remove, display banner no if no custom image * feat(plg): implement partner banner handling and system model identification from activation JSON * feat(web): add loading and error states to notification sidebar * refactor(web): add container for loading & error states * feat(web): add count labels to notification tabs * refactor(web): lift notifications overview query to Sidebar from Indicator * fix(web): refetch notifications for sidebar when new notifications arrive * feat(web): move notification indicator icons to top-right of bell icon previously, icons were placed next to bell icon because the status indicators were not accessible to color-blind users. this commit replaces circular status indicators with the icons. * feat(web): remove notification indicator pulse the pulse was initially added to provide visual feedback when: 1. a new notification arrived 2. an alert notification was unread because we began using the legacy notify script, we now get a toast on new notifications. re:2, feedback on the pulse was mixed, so i'm removing it. * refactor(plg): improve conditional for activation & partner setup * feat(plg): add case model icon handling based on activation JSON * feat(plg): enhance partner setup process with detailed logging and system identification updates and setup flag * feat(plg): add setup flag file creation and logging for activation changes * feat(plg): update activation setup flag checks for banner, case model icon, and system identity * fix(plg): add error handling for invalid activation JSON and improve conditional checks for setup flags * fix(plg): improve partner banner and case model icon setup logic with enhanced checks and comments * fix(plg): update case model icon configuration to prevent newline issues in parsing * fix(plg): remove unnecessary echo debug statements and improve setup flag handling for partner details * fix(plg): remove unnecessary registration page check from activation code modal logic * fix(plg): add overlay opacity prop to modal components for customizable background transparency * fix(plg): enhance modal component with vertical centering option and adjust max-width * fix(plg): enhance configuration handling by dynamically updating multiple header parameters in the config file * fix(plg): improve comments and enhance setup flag handling for custom icons in activation logic * fix(translations): correct capitalization and punctuation in user prompts for consistency * fix(store): enhance activation code data structure with additional properties and correct partner URL reference * fix(modal): enhance activation modal with dynamic partner logo and improved title handling * fix(store): update activation code data structure with additional properties and correct partner details * fix(index): refactor badge and button color handling with typed constants for improved maintainability * fix(dependencies): add vue-tsc for improved TypeScript support in Vue projects * refactor(plugin): remove unnecessary comments and clean up code structure * refactor(plugin): remove todo comment for custom header logo * fix(Modal): add partnerUrl handling for logo link and improve code structure * fix(serverState): update partnerLogoPath to use a placeholder image for development * feat(plugin): add rsync activation script for testing activation modal & installs * refactor(activationCode): remove debug log for modal visibility check * feat(plugin): enhance activation process by adding logo and case model handling * refactor(activationCode): rename partnerLogoPath to partnerLogo and update handling for logo file type * refactor(activationCode): remove debug log for partnerLogo watcher * chore(nuxt.config): add ignore rule for webGui images directory * refactor(plugin): rename params array to DISPLAY_PARAMS and update handling for configuration updates * fix(web): partner logo handling for themes * fix(plg): activation oem, custom case icon cfg handling * fix(plugin): replace sed with awk for config file updates in DISPLAY_PARAMS handling * fix(web): add target and rel attributes to partner logo link for security * feat(plugin): add theme parameter to DISPLAY_PARAMS and activation code interface * fix(plugin): replace sed with parameter expansion and awk for config file updates * fix(plugin): reboot logic for plg install to prevent settings overwrite * feat(plugin): streamline activation and partner setup by abstracting scripts for better maintainability * fix(plugin): update script source paths for activation and partner setup * fix(plugin): update script source paths for activation and partner setup to use relative paths * fix(plugin): update script source paths for activation and partner setup to use absolute paths * fix(plugin): add debug mode support to activation scripts and update setup flag handling * fix(plugin): enhance activation scripts with debug mode and conditional execution for safety * refactor(plugin): reposition execution of activation_code setup / remove scripts * fix(plugin): add checks for activation_code_setup script existence and improve deletion logging * fix(plugin): add option to remove setup flag after script execution * fix(plugin): streamline activation script execution by removing unnecessary checks * fix(plugin): remove duplicate activation_code_setup script sourcing * feat(modal): add overlay color prop and enhance class binding for modal background * feat(activation): update modal overlay color and opacity * feat(plg): add WebComponentsExtractor class for managing JS file retrieval * feat(plugin): myservers1.phpintegrate WebComponentsExtractor for dynamic script tag generation * feat(activation): enhance getData method to support JSON output and add getDataForHtmlAttr for HTML-safe JSON * refactor(activation): simplify getData method by removing parameters * feat(plugin): add welcome-modal.php for displaying server state and web components * refactor(web): activation code store rename modal visibility variables for clarity * feat(web): add WelcomeModal component for user onboarding and server setup * feat(plugin): activation_code setup and remove script support welcome modal * refactor(plugin): streamline activation code setup script by removing some comments * fix(plugin): update activation code setup script to prepend items to auth request allow list array * feat: enhance ActivationCodeExtractor with partner logo handling and metadata extraction - Added constants for document root and web GUI images directory. - Introduced properties for partner name, URL, logo path, and logo file type. - Implemented methods to retrieve partner logo path, render logo string, partner name, and URL. - Enhanced data extraction logic to include partner-related information from JSON data. - Added debug method for outputting internal state for troubleshooting. * refactor: update activation-data.php to include debug output for ActivationCodeExtractor - Removed JSON response and replaced it with a debug output wrapped in <pre> tags. - This change allows for easier troubleshooting by displaying internal state information directly on the page. * feat: inject partner logo into DefaultPageLayout and update JS file paths - Modified the activation code setup script to change the path of JavaScript files to be relative to the web GUI's webroot. - Added functionality to inject a partner logo into the DefaultPageLayout by replacing the existing logo string. - Implemented a conditional backup mechanism for the DefaultPageLayout file before making changes. - Included debug output to confirm successful injection or report errors during the process. * feat: add partner logo display functionality and style adjustments - Introduced a new file for displaying the partner logo, which retrieves the logo and URL based on the activation code. - Added CSS styles to adjust the display properties of the partner logo in the header, ensuring proper sizing and alignment. - Enhanced the integration of the partner logo into the existing plugin structure. * refactor: simplify partner logo injection debug output in activation code setup script - Removed syntax check for the DefaultPageLayout file after injecting the partner logo. - Streamlined debug output to confirm successful injection without additional checks, enhancing clarity and reducing complexity. * style: adjust partner logo height in myservers1.php * style: update partner logo width in PartnerLogoImg.vue - Changed the CSS class for the partner logo image from a max height constraint to a fixed width of 72 units, ensuring better responsiveness and alignment in the layout. * fix: update modal title attribute for conditional close behavior - Modified the title attribute of the modal overlay to conditionally display the close instruction based on the showCloseX property. This change improves accessibility by ensuring the title is only set when the close button is visible. * refactor: update partner logo handling in activation code scripts and extractor - Changed the partner logo file name to 'partner-logo.svg' in ActivationCodeExtractor. - Simplified logo path handling by removing unnecessary file type checks and directly using the SVG file. - Updated CSS in myservers1.php to target only the SVG logo. - Modified activation code setup and removal scripts to consistently reference the SVG logo, improving clarity and maintainability. * feat: enhance activation code setup script to include server name handling - Added logic to retrieve and set the server name from the activation JSON if the current system model or comment is not set, or if the current name is "Tower". - Improved debug output to include the partner server name, enhancing visibility during the activation process. - Sanitized the partner server name to remove quotes and backslashes for better handling. * feat: validate activation JSON before proceeding with setup - Added validation for the activation JSON file to ensure it is correctly formatted before executing the setup process. - Enhanced debug output to indicate whether the JSON is valid or not, improving troubleshooting capabilities. - The setup will now only proceed if the activation JSON is confirmed to be valid, preventing potential errors during execution. * fix(plg): improve warning message for invalid activation JSON in setup script * feat: enhance activation code handling with partner logo integration - Added a new optional property 'partnerLogo' to the ActivationCodeExtractor class and updated related interfaces to support boolean values for logo display. - Removed the PNG logo reference from the activation code setup script, streamlining the logo handling process. - Updated the server state to utilize the new 'partnerLogo' property, ensuring consistent logo display logic across the application. - Adjusted computed properties in the activation code store to reflect the changes in logo handling, improving clarity and maintainability. * fix: correct activation JSON validation in setup script - Updated the activation code setup script to use 'jq empty' for validating the activation JSON file, ensuring proper error handling and validation. - This change improves the robustness of the setup process by accurately checking the JSON format before proceeding. * fix: correct comment formatting in dynamix.unraid.net.plg - Fixed a formatting issue in the comment regarding the use of myservers.cfg values to prevent conflicts during installation. This change improves code readability and clarity. * chore: update rsync activation script comments for clarity - Revised comments in the rsync-activation-dir.sh script to better describe its purpose and usage. - Enhanced documentation with an example usage to improve user understanding of the script's functionality. * refactor: clean up activation data and extractor files - Removed outdated copyright comments from activation-data.php and activation-code-extractor.php for improved readability. - Simplified the debug output in activation-data.php by using short PHP tags. - Updated the comment in the ActivationCodeExtractor class to be more concise, enhancing clarity for future developers. * fix: remove unnecessary CSS properties in myservers1.php - Eliminated redundant display and margin properties from the CSS in myservers1.php to streamline the styling and improve code clarity. * refactor: streamline WebComponentsExtractor class and update method names - Removed outdated copyright comments from web-components-extractor.php for improved readability. - Renamed method getJsFileUrl() to getJSFileRelativePath() to enhance clarity and consistency in naming conventions. - Adjusted indentation for better code formatting and readability. * refactor: simplify partner logo handling in activation code script - Removed unnecessary variable initialization and included only essential requires for improved clarity. - Streamlined the code in partner-logo.php to focus on the core functionality of displaying the partner logo. - Enhanced maintainability by reducing complexity in the script. * refactor: clean up activation and server state files - Removed outdated copyright comments and unnecessary variable initializations from activation-data.php and server-state.php for improved readability. - Streamlined the code by eliminating redundant require statements, focusing on essential functionality. - Enhanced maintainability and clarity by simplifying the structure of both files. * feat: add password creation prompts to translations - Introduced new translation strings for password creation in both PHP and JSON files. - Enhanced user guidance by providing detailed messages about the importance of the password for system access and management. - Improved overall user experience by ensuring clarity in the password setup process. * refactor: update activation code setup script comments * refactor: enhance activation code removal script functionality - Updated the activation code removal script to delete additional related files, including PHP and setup scripts, for a more thorough cleanup. - Improved comments for clarity on script usage and options, particularly regarding the self-delete functionality. - Streamlined the deletion process by using an array to manage files to be removed, enhancing maintainability and readability. * fix: update WelcomeModal to disable close button * refactor: reorganize activation code setup and improve script clarity - Moved the activation and partner setup section to follow the web component timestamp check to ensure correct targeting during setup. - Updated comments for better clarity regarding the activation code setup process. - Retained the warning message about not closing the window yet for user guidance. * refactor: enhance activation code setup script with improved comments and logic - Updated comments to clarify the purpose of server name, model, and description checks. - Modified conditional logic to include additional checks for server identification. - Added a TODO note regarding the necessity of updating the ident.cfg file, ensuring future review for potential optimizations. * refactor: update activation code scripts to use .done flag * refactor: introduce constant for activation modal storage key - Added a new constant for the activation code modal hidden storage key to improve code maintainability and clarity. - Updated the activation code store to utilize the new constant, replacing hardcoded string references. * feat: enhance user onboarding and modal components - Added new translation strings for activating Unraid licenses and creating Unraid.net accounts to improve user onboarding experience. - Updated the WelcomeModal and Activation Modal components to reflect new messaging and improved styling options, including the ability to disable shadows. - Implemented a workaround in the WelcomeModal to address font-size inconsistencies between login and authenticated pages. - Refactored the index page to correctly pass server data to the WelcomeModal component. * chore: comment out WelcomeModalCe component for testing * feat: add disableOverlayClose prop to Modal and WelcomeModal components - Introduced a new prop `disableOverlayClose` to the Modal component, allowing users to prevent closing the modal by clicking on the overlay. - Updated the WelcomeModal component to utilize the new `disableOverlayClose` prop, enhancing modal behavior customization. * refactor: update activation code removal script to delete by default - Changed the activation code removal script to use a dry-run flag instead of a self-delete flag, enhancing safety during execution. - Updated the plugin file to reflect the removal of the --delete option, ensuring consistency with the new script behavior. --------- Co-authored-by: Eli Bosley <ekbosley@gmail.com> Co-authored-by: Pujit Mehrotra <pujit@lime-technology.com>
mdatelle
force-pushed
the
refactor/unraid-ui-cleanup
branch
from
db3613e to
7974ef3
Compare
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/main.yml (1)
104-140:⚠️ Potential issueFix artifact upload path and optimize build configuration.
The artifact upload path appears to be nested too deeply. The
mvcommands move files intounraid-ui-dist/, but the upload path includes an additionalunraid-ui/level.Apply this diff to fix the path and optimize the build configuration:
- path: unraid-ui/unraid-ui-dist + path: unraid-ui-distAdditionally, consider adding build caching to speed up subsequent builds:
- name: Build run: npm run build + env: + TURBO_TEAM: ${{ github.repository_owner }} + TURBO_TOKEN: ${{ secrets.TURBO_TOKEN }}unraid-ui/src/theme/preset.ts (1)
105-114: 🛠️ Refactor suggestionConsider using relative units for font sizes.
Using fixed pixel values for font sizes may cause accessibility issues as it prevents text scaling based on user preferences. Consider using
remunits instead.fontSize: { - "10px": "10px", - "12px": "12px", - "14px": "14px", - "16px": "16px", - "18px": "18px", - "20px": "20px", - "24px": "24px", - "30px": "30px", + "10px": "0.625rem", // 10px + "12px": "0.75rem", // 12px + "14px": "0.875rem", // 14px + "16px": "1rem", // 16px + "18px": "1.125rem", // 18px + "20px": "1.25rem", // 20px + "24px": "1.5rem", // 24px + "30px": "1.875rem", // 30px },
🧹 Nitpick comments (20)
unraid-ui/stories/components/common/Button.stories.ts (2)
13-18: Consider adding more button controls to enhance story documentationWhile the basic props are covered, consider adding controls for common button attributes like:
disabledstateloadingstate (if supported)iconplacement (if supported)This would make the story more comprehensive for documentation purposes.
24-32: Improve template formatting for better readabilityConsider adjusting the template indentation to be more consistent:
- template: ` - <ButtonComponent - :variant="args.variant" - :size="args.size" - :class="args.class" - > - {{ args.default }} - </ButtonComponent> - `, + template: ` + <ButtonComponent + :variant="args.variant" + :size="args.size" + :class="args.class" + > + {{ args.default }} + </ButtonComponent> + `,web/tailwind.config.ts (1)
19-28: Consider adding type-safe environment variable handling.While the documentation for the font size configuration is good, consider adding type safety for the environment variable:
- newFontSize: Number(process.env.VITE_TAILWIND_BASE_FONT_SIZE ?? 10), + newFontSize: Number(process.env.VITE_TAILWIND_BASE_FONT_SIZE || '') || 10,This change ensures proper type coercion and handles empty string cases.
unraid-ui/vite.config.ts (2)
15-21: Optimize DTS plugin configuration.Consider the following adjustments:
- Including
tailwind.config.tsin DTS generation might be unnecessary as it's a configuration file.copyDtsFilesmight be redundant whenrollupTypesis enabled.dts({ insertTypesEntry: true, - include: ['src/**/*.ts', 'src/**/*.vue', 'tailwind.config.ts'], + include: ['src/**/*.ts', 'src/**/*.vue'], outDir: 'dist', rollupTypes: true, - copyDtsFiles: true, }),
24-28: Consider adding commonly used PostCSS plugins.The current PostCSS setup could benefit from additional plugins:
postcss-preset-envfor better browser compatibilitycssnanofor CSS optimization in productioncss: { postcss: { - plugins: [tailwindcss()], + plugins: [ + tailwindcss(), + process.env.NODE_ENV === 'production' && cssnano(), + postcssPresetEnv({ stage: 3, features: { 'nesting-rules': true } }) + ].filter(Boolean), }, },.github/workflows/main.yml (1)
171-174: Improve error handling for UI folder removal.The
rmcommand will fail if the folder doesn't exist. Add error handling to make this step more robust.- - name: Remove Existing Unraid UI folder - run: | - rm -r ../unraid-ui + - name: Clean up existing Unraid UI folder + run: | + rm -rf ../unraid-ui || trueunraid-ui/src/theme/preset.ts (4)
6-15: Consider adding more container breakpoints for better responsive design.While the current configuration is good, consider adding more breakpoints (e.g., 'sm', 'md', 'lg', 'xl') to provide finer control over container widths at different screen sizes.
screens: { + "sm": "640px", + "md": "768px", + "lg": "1024px", + "xl": "1280px", "2xl": "1400px", },
20-104: Consider adding documentation for the color system.The color system is well-organized, but adding documentation about color usage and relationships would improve maintainability. Consider:
- Adding JSDoc comments explaining the purpose of each color category
- Documenting the relationship between CSS variables and their usage
+/** + * Color system configuration + * - Base colors: Standard colors used throughout the application + * - Unraid colors: Brand-specific colors with semantic meaning + * - ShadCN colors: Theme colors using CSS variables for dynamic theming + */ colors: {
115-166: Consider standardizing the spacing system.The current spacing system mixes different units (rem and px). Consider:
- Using a consistent unit system (preferably rem)
- Using a more predictable scale
spacing: { "4.5": "1.125rem", - "-8px": "-8px", - "2px": "2px", + "-8px": "-0.5rem", + "2px": "0.125rem", // ... continue converting other values },
196-241: Consider optimizing typography for dark mode.While the typography configuration is comprehensive, consider adjusting the contrast ratios for dark mode to improve readability. The current configuration uses the same color values for both light and dark modes.
typography: (theme: PluginAPI["theme"]) => ({ DEFAULT: { css: { // ... existing configuration + "@media (prefers-color-scheme: dark)": { + "--tw-prose-body": theme("colors.grey-lighter"), + "--tw-prose-headings": theme("colors.white"), + // ... adjust other dark mode colors + }, }, }, }),unraid-ui/stories/components/layout/CardWrapper.stories.ts (2)
13-26: Enhance story documentation with ArgTypes and description.Consider adding component documentation to improve the Storybook experience:
export const CardWrapper: Story = { + args: { + // Add default args here + }, + parameters: { + docs: { + description: { + story: 'The default CardWrapper component that provides a container for content with optional states.', + }, + }, + }, render: (args) => ({
1-62: Enhance stories with comprehensive documentation and controls.Consider these improvements to make the stories more interactive and informative:
- Add controls for all props
- Document component events
- Add code examples
- Include use-case descriptions
const meta = { title: "Components/Layout/CardWrapper", component: CardWrapperComponent, + argTypes: { + error: { + control: 'boolean', + description: 'Enables error state styling', + table: { + type: { summary: 'boolean' }, + defaultValue: { summary: false }, + }, + }, + warning: { + control: 'boolean', + description: 'Enables warning state styling', + table: { + type: { summary: 'boolean' }, + defaultValue: { summary: false }, + }, + }, + }, + parameters: { + docs: { + description: { + component: 'A wrapper component that provides different visual states for content presentation.', + }, + }, + }, } satisfies Meta<typeof CardWrapperComponent>;🧰 Tools
🪛 Biome (1.9.4)
[error] 28-28: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
unraid-ui/src/components/common/badge/Badge.vue (2)
6-14: Add JSDoc comments to document the BadgeProps interface.Adding documentation will help other developers understand the purpose and usage of each prop.
+/** + * Props for the Badge component + * @interface BadgeProps + * @property {string} [variant="gray"] - Visual style variant of the badge + * @property {string} [size="md"] - Size of the badge + * @property {Component} [icon] - Icon component to display before content + * @property {Component} [iconRight] - Icon component to display after content + * @property {string} [iconStyles] - Additional styles for the icons + * @property {string} [class] - Additional CSS classes + */ export interface BadgeProps { variant?: "red" | "yellow" | "green" | "blue" | "indigo" | "purple" | "pink" | "orange" | "black" | "white" | "transparent" | "current" | "gray" | "custom"; size?: "xs" | "sm" | "md" | "lg" | "xl" | "2xl";
25-38: Consider using a type-safe mapping for icon sizes.Using a const assertion for the icon sizes object improves type safety and maintainability.
-const badgeClasses = computed(() => { - const iconSizes = { - xs: "w-12px", - sm: "w-14px", - md: "w-16px", - lg: "w-18px", - xl: "w-20px", - "2xl": "w-24px", - } as const; +const ICON_SIZES = { + xs: "w-12px", + sm: "w-14px", + md: "w-16px", + lg: "w-18px", + xl: "w-20px", + "2xl": "w-24px", +} as const; + +const badgeClasses = computed(() => { return { badge: badgeVariants({ variant: props.variant, size: props.size }), - icon: `${iconSizes[props.size ?? "md"]} ${props.iconStyles}`, + icon: `${ICON_SIZES[props.size ?? "md"]} ${props.iconStyles}`, }; });unraid-ui/src/components/common/sheet/SheetContent.vue (1)
Line range hint
54-58: Enhance accessibility of the close button.The close button should have an aria-label for better screen reader support.
<DialogClose class="absolute right-4 top-4 rounded-sm opacity-70 ring-offset-background transition-opacity hover:opacity-100 focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:pointer-events-none data-[state=open]:bg-secondary" + aria-label="Close dialog" > <X class="w-4 h-4 text-muted-foreground" /> </DialogClose>web/components/HeaderOsVersion.ce.vue (1)
22-36: Add type annotation for the computed property.Adding a return type to the computed property improves type safety.
-const updateOsStatus = computed(() => { +const updateOsStatus = computed<{ + badge?: { + color: string; + icon: Component; + }; + href?: string; + click?: () => void; + text: string; + title?: string; +} | null>(() => {web/pages/index.vue (2)
39-50: Consider moving button variants to a constants file.The button variants array could be moved to a shared constants file to promote reusability across components and maintain consistency in the UI library.
+// src/constants/button.ts +export const BUTTON_VARIANTS = [ + 'fill', + 'black', + 'gray', + 'outline', + 'outline-black', + 'outline-white', + 'underline', + 'underline-hover-red', + 'white', + 'none', +] as const;
143-152: Improve button showcase layout.The button variants showcase could benefit from better spacing and organization for improved readability.
- <template v-for="variant in variants" :key="variant"> - <BrandButton - :variant="variant" - type="button" - size="14px" - :icon="ExclamationTriangleIcon" - >{{ variant }}</BrandButton - > - </template> + <div class="grid grid-cols-2 md:grid-cols-3 lg:grid-cols-4 gap-4"> + <template v-for="variant in variants" :key="variant"> + <BrandButton + :variant="variant" + type="button" + size="14px" + :icon="ExclamationTriangleIcon" + >{{ variant }}</BrandButton + > + </template> + </div>web/components/UpdateOs/Status.vue (1)
77-77: Improve click handler clarity.The ternary expression in the click handler could be clearer with parentheses and proper statement termination.
- props.showExternalDowngrade ? accountStore.downgradeOs() : accountStore.updateOs(); + return (props.showExternalDowngrade ? accountStore.downgradeOs : accountStore.updateOs)();🧰 Tools
🪛 GitHub Check: Build Web App
[failure] 77-77:
Expected an assignment or function call and instead saw an expressionunraid-ui/tailwind.config.ts (1)
Line range hint
1-31: Consider documenting the preset-based configuration approach.The move towards a preset-based configuration is a significant architectural change that supports the UI cleanup and migration effort. Consider adding documentation in the README about:
- The role of the
unraidPreset- How it relates to shadcn components (as mentioned in PR comments)
- Guidelines for maintaining consistent styling across components
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (2)
unraid-ui/package-lock.jsonis excluded by!**/package-lock.jsonweb/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (68)
.github/workflows/main.yml(2 hunks).vscode/settings.json(1 hunks).vscode/sftp-template.json(1 hunks)unraid-ui/.eslintrc.js(1 hunks)unraid-ui/.prettierrc.json(1 hunks)unraid-ui/.storybook/main.ts(2 hunks)unraid-ui/.storybook/tailwind.config.ts(1 hunks)unraid-ui/package.json(3 hunks)unraid-ui/src/components/brand/BrandButton.vue(1 hunks)unraid-ui/src/components/brand/BrandLoading.vue(1 hunks)unraid-ui/src/components/brand/BrandLoadingWhite.vue(1 hunks)unraid-ui/src/components/brand/BrandLogo.vue(1 hunks)unraid-ui/src/components/brand/BrandLogoConnect.vue(1 hunks)unraid-ui/src/components/brand/brand-button.variants.ts(1 hunks)unraid-ui/src/components/brand/index.ts(1 hunks)unraid-ui/src/components/common/badge/Badge.vue(1 hunks)unraid-ui/src/components/common/badge/badge.variants.ts(1 hunks)unraid-ui/src/components/common/badge/index.ts(1 hunks)unraid-ui/src/components/common/button/Button.vue(2 hunks)unraid-ui/src/components/common/button/button.variants.ts(1 hunks)unraid-ui/src/components/common/button/index.ts(1 hunks)unraid-ui/src/components/common/sheet/SheetContent.vue(1 hunks)unraid-ui/src/components/common/sheet/index.ts(1 hunks)unraid-ui/src/components/common/sheet/sheet.variants.ts(1 hunks)unraid-ui/src/components/form/lightswitch/index.ts(1 hunks)unraid-ui/src/components/form/switch/index.ts(1 hunks)unraid-ui/src/index.ts(6 hunks)unraid-ui/src/styles/index.css(1 hunks)unraid-ui/src/theme/preset.ts(1 hunks)unraid-ui/src/types/badge.ts(1 hunks)unraid-ui/src/types/button.ts(0 hunks)unraid-ui/stories/components/brand/BrandButton.stories.ts(1 hunks)unraid-ui/stories/components/brand/BrandLoading.stories.ts(1 hunks)unraid-ui/stories/components/brand/BrandLoadingWhite.stories.ts(1 hunks)unraid-ui/stories/components/brand/BrandLogo.stories.ts(1 hunks)unraid-ui/stories/components/brand/BrandLogoConnect.stories.ts(1 hunks)unraid-ui/stories/components/common/Badge.stories.ts(1 hunks)unraid-ui/stories/components/common/Button.stories.ts(1 hunks)unraid-ui/stories/components/common/DropdownMenu.stories.ts(1 hunks)unraid-ui/stories/components/common/Loading.stories.ts(1 hunks)unraid-ui/stories/components/common/ScrollArea.stories.ts(1 hunks)unraid-ui/stories/components/common/Sheet.stories.ts(1 hunks)unraid-ui/stories/components/common/Tabs.stories.ts(1 hunks)unraid-ui/stories/components/common/Tooltip.stories.ts(1 hunks)unraid-ui/stories/components/form/Input.stories.ts(1 hunks)unraid-ui/stories/components/form/Label.stories.ts(1 hunks)unraid-ui/stories/components/form/Lightswitch.stories.ts(1 hunks)unraid-ui/stories/components/form/Select.stories.ts(1 hunks)unraid-ui/stories/components/form/Switch.stories.ts(1 hunks)unraid-ui/stories/components/layout/CardWrapper.stories.ts(1 hunks)unraid-ui/stories/components/layout/PageContainer.stories.ts(1 hunks)unraid-ui/tailwind.config.ts(2 hunks)unraid-ui/tsconfig.app.json(3 hunks)unraid-ui/tsconfig.json(2 hunks)unraid-ui/tsconfig.test.json(1 hunks)unraid-ui/vite.config.ts(1 hunks)web/components/Auth.ce.vue(2 hunks)web/components/DowngradeOs.ce.vue(3 hunks)web/components/DownloadApiLogs.ce.vue(4 hunks)web/components/HeaderOsVersion.ce.vue(5 hunks)web/components/UpdateOs.ce.vue(4 hunks)web/components/UpdateOs/Downgrade.vue(5 hunks)web/components/UpdateOs/Status.vue(6 hunks)web/components/UserProfile/CallbackFeedback.vue(13 hunks)web/components/UserProfile/DropdownLaunchpad.vue(3 hunks)web/package.json(2 hunks)web/pages/index.vue(4 hunks)web/tailwind.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- unraid-ui/src/types/button.ts
🚧 Files skipped from review as they are similar to previous changes (53)
- unraid-ui/.prettierrc.json
- unraid-ui/src/styles/index.css
- unraid-ui/src/components/common/button/index.ts
- unraid-ui/src/components/common/button/button.variants.ts
- unraid-ui/src/components/common/badge/index.ts
- .vscode/sftp-template.json
- unraid-ui/.eslintrc.js
- unraid-ui/src/components/form/lightswitch/index.ts
- .vscode/settings.json
- unraid-ui/src/components/common/sheet/sheet.variants.ts
- unraid-ui/src/components/brand/BrandLoading.vue
- unraid-ui/stories/components/form/Label.stories.ts
- unraid-ui/stories/components/common/Sheet.stories.ts
- unraid-ui/src/components/brand/BrandLoadingWhite.vue
- unraid-ui/tsconfig.test.json
- unraid-ui/src/components/common/button/Button.vue
- unraid-ui/.storybook/main.ts
- web/components/UpdateOs.ce.vue
- unraid-ui/stories/components/form/Select.stories.ts
- unraid-ui/src/components/brand/brand-button.variants.ts
- unraid-ui/tsconfig.app.json
- unraid-ui/src/index.ts
- web/components/Auth.ce.vue
- unraid-ui/src/components/brand/BrandLogoConnect.vue
- unraid-ui/src/components/brand/BrandLogo.vue
- web/package.json
- unraid-ui/stories/components/brand/BrandButton.stories.ts
- unraid-ui/stories/components/common/DropdownMenu.stories.ts
- unraid-ui/stories/components/form/Input.stories.ts
- unraid-ui/stories/components/brand/BrandLogoConnect.stories.ts
- unraid-ui/src/components/brand/index.ts
- web/components/UserProfile/DropdownLaunchpad.vue
- unraid-ui/stories/components/common/Tabs.stories.ts
- unraid-ui/src/types/badge.ts
- unraid-ui/stories/components/layout/PageContainer.stories.ts
- unraid-ui/stories/components/brand/BrandLoading.stories.ts
- unraid-ui/stories/components/form/Lightswitch.stories.ts
- web/components/DownloadApiLogs.ce.vue
- unraid-ui/stories/components/common/Badge.stories.ts
- unraid-ui/tsconfig.json
- unraid-ui/stories/components/form/Switch.stories.ts
- unraid-ui/src/components/form/switch/index.ts
- unraid-ui/stories/components/brand/BrandLogo.stories.ts
- unraid-ui/src/components/common/badge/badge.variants.ts
- unraid-ui/stories/components/brand/BrandLoadingWhite.stories.ts
- web/components/DowngradeOs.ce.vue
- unraid-ui/stories/components/common/ScrollArea.stories.ts
- unraid-ui/src/components/common/sheet/index.ts
- unraid-ui/stories/components/common/Tooltip.stories.ts
- web/components/UserProfile/CallbackFeedback.vue
- unraid-ui/package.json
- web/components/UpdateOs/Downgrade.vue
- unraid-ui/.storybook/tailwind.config.ts
🧰 Additional context used
🪛 Biome (1.9.4)
unraid-ui/stories/components/common/Loading.stories.ts
[error] 3-3: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
unraid-ui/stories/components/layout/CardWrapper.stories.ts
[error] 28-28: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🪛 GitHub Check: Build Web App
web/components/UpdateOs/Status.vue
[failure] 77-77:
Expected an assignment or function call and instead saw an expression
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (32)
unraid-ui/stories/components/common/Button.stories.ts (1)
11-11: LGTM! Following Storybook 7.x best practicesThe type definition correctly leverages type inference from the meta object, which is the recommended approach in Storybook 7.x.
web/tailwind.config.ts (5)
2-4: LGTM! Well-organized imports with proper typing.The imports are structured correctly, and the addition of the base configuration from
@unraid/uialigns with the PR's objective of consolidating Tailwind configurations.
6-7: LGTM! Good use of TypeScript features and configuration inheritance.The use of the
satisfiesoperator withPartial<Config>provides better type safety while maintaining type inference. The preset configuration helps ensure consistency with the base UI library.Also applies to: 34-34
8-14: Add pattern for root app.vue file.Add
'./app.vue'to the content patterns to ensure the root app file is included in Tailwind's purge process.content: [ // Web components './components/**/*.ce.{js,vue,ts}', // Regular Vue components './components/**/*.{js,vue,ts}', './layouts/**/*.vue', './pages/**/*.vue', + './app.vue', ],
16-17: Remove deprecated JIT mode configuration.The
mode: 'jit'configuration option should be removed as it's deprecated. You're using a modern version of Tailwind CSS where JIT is the default and only compiler.- mode: 'jit', safelist: [],
29-33: LGTM! Clear separation of concerns in theme extensions.The empty theme extensions section with the comment clearly indicates that this is reserved for web-specific extensions only, maintaining a good separation of concerns between the base UI library and web-specific customizations.
unraid-ui/vite.config.ts (5)
8-9: LGTM! Good refactor to function-based configuration.The change to wrap the configuration in a function enables better flexibility and reusability.
62-64: Review production build optimization settings.Several settings might need adjustment for production:
minify: falsewill increase bundle sizesourcemap: truemight expose source codepreserveModules: truemight impact tree-shakingConsider using environment-specific settings for these options.
30-59: LGTM! Well-structured output configuration.The output configuration effectively handles different file types and maintains a clean dist structure. The conditional file naming for Tailwind config is particularly well thought out.
66-76: LGTM! Well-organized path aliases.The path aliases are well-structured, consistent, and follow best practices.
77-80: LGTM! Standard test configuration.The test configuration follows best practices with a lightweight DOM implementation and clear file patterns.
unraid-ui/src/theme/preset.ts (3)
1-5: LGTM! Well-structured configuration setup.The imports are properly typed and the configuration uses type safety with
satisfies Partial<Config>.
17-19: LGTM! Well-structured font stack.The font stack follows best practices by prioritizing system fonts for optimal performance and consistent rendering.
172-195: LGTM! Well-structured animation configuration.The keyframes and animations are well-defined, using CSS custom properties for dynamic values and providing smooth transitions.
unraid-ui/stories/components/layout/CardWrapper.stories.ts (3)
1-11: LGTM! Well-structured Storybook configuration.The imports and meta configuration follow Storybook best practices with proper type safety.
28-44: Rename the 'Error' story to avoid shadowing the global Error object.Consider using a more descriptive name like 'ErrorState' or 'ErrorVariant' to prevent potential confusion with the global Error object.
-export const Error: Story = { +export const ErrorState: Story = {🧰 Tools
🪛 Biome (1.9.4)
[error] 28-28: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
46-62: Verify prop types in the CardWrapper component.The story uses boolean props (
warning: true), but we should ensure these props are properly typed in the component definition.Let's verify the prop types in the CardWrapper component:
✅ Verification successful
Props are properly typed in CardWrapper component
The CardWrapper component has proper TypeScript definitions for all props, including the
warningprop which is correctly typed as an optional boolean with a default value offalse. The story's usage aligns with the component's type definitions.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the CardWrapper component definition ast-grep --pattern 'defineProps<{ $$$ warning?: boolean; error?: boolean; $$$ }>()'Length of output: 93
Script:
#!/bin/bash # Find the CardWrapper component file fd CardWrapper.vue # If found, display its contents focusing on props section fd CardWrapper.vue --exec cat {}Length of output: 1720
unraid-ui/stories/components/common/Loading.stories.ts (2)
3-3: Rename imported Error component to avoid shadowing.The imported Error component shadows the global Error object.
-import Error from "../../../src/components/common/loading/Error.vue"; +import LoadingError from "../../../src/components/common/loading/Error.vue";🧰 Tools
🪛 Biome (1.9.4)
[error] 3-3: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
45-51: Fix template syntax in LoadingError story.The template has an unclosed div and unnecessary empty lines.
template: ` - - <Error v-bind="args"> - <div class="text-center">Content when not loading or error</div> - </Error> - </div> - `, + <Error v-bind="args"> + <div class="text-center">Content when not loading or error</div> + </Error> + `,unraid-ui/src/components/brand/BrandButton.vue (2)
16-17: Replace any type with Component type for icon props.Using any type reduces type safety.
- icon?: any; - iconRight?: any; + icon?: Component; + iconRight?: Component;
53-63: Enhance accessibility and simplify click handling.Add aria-labels and simplify the click handler.
<component :is="href ? 'a' : 'button'" :disabled="disabled" :href="href" :rel="external ? 'noopener noreferrer' : ''" :target="external ? '_blank' : ''" :type="!href ? btnType : ''" :class="classes.button" :title="title" - @click="click ?? $emit('click')" + :aria-label="title || text" + :aria-label="external ? `${title || text} (opens in new tab)` : undefined" + @click="$emit('click')" >web/components/HeaderOsVersion.ce.vue (1)
8-9: Remove duplicate imports.These imports are already present.
-import { storeToRefs } from 'pinia'; -import { useI18n } from 'vue-i18n';web/pages/index.vue (2)
14-15: Consider using a responsive viewport meta tag.Setting a fixed viewport width of 1300px may cause usability issues on smaller screens and mobile devices. Consider using a responsive approach instead.
162-164: LGTM! Correct ordering of style imports.Good practice placing @unraid/ui globals first, ensuring proper CSS cascade and specificity.
web/components/UpdateOs/Status.vue (4)
11-11: LGTM! Consistent with UI migration strategy.Import of Badge and BrandButton from @unraid/ui aligns with the PR's objective of standardizing UI components.
44-45: LGTM! Proper use of storeToRefs for reactive state.Good practice using storeToRefs for destructuring reactive store state.
Line range hint
125-178: LGTM! Consistent Badge component implementation.Excellent migration to the new Badge component with proper variant usage, icons, and accessibility attributes.
Line range hint
184-210: LGTM! Well-structured BrandButton implementations.Excellent use of the BrandButton component with proper variant handling, dynamic text, and event binding.
unraid-ui/tailwind.config.ts (4)
1-5: Good use of presets for theme standardization!The migration to using a preset-based configuration is a good architectural decision that will help maintain consistency across the UI library.
6-10: Include additional content paths for comprehensive styling.The configuration is missing some critical paths that might contain Tailwind classes:
"./src/layouts/**/*.{js,vue,ts}""./src/pages/**/*.{js,vue,ts}"This was previously identified and remains relevant for ensuring all UI components are properly styled.
31-31: Excellent use of TypeScript'ssatisfiesoperator!The
satisfiesoperator provides better type safety than direct type assertions while maintaining type inference capabilities.
Line range hint
11-30: Verify completeness of safelisted classes.Let's ensure all necessary classes are included in the safelist by checking the codebase for potentially purged classes.
✅ Verification successful
Safelist configuration is complete and accurate.
All safelisted classes and their variants are actively used in the codebase, particularly in header components, user profile sections, and brand loading animations. The patterns correctly capture the required group-hover and group-focus variants for text and underline utilities.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for class names in the codebase that match the patterns # but might not be covered by the safelist # Check for text utility classes echo "Checking for text utility classes..." rg "class=.*text-(header-text-secondary|orange-dark)" \ --type vue --type ts --type js \ -g '!node_modules' # Check for underline utility classes echo "Checking for underline utility classes..." rg "class=.*(underline|no-underline)" \ --type vue --type ts --type js \ -g '!node_modules' # Check for mark classes echo "Checking for mark classes..." rg "class=.*unraid_mark_[0-9]" \ --type vue --type ts --type js \ -g '!node_modules'Length of output: 605
Script:
#!/bin/bash # Search without file type restrictions and include Vue class patterns echo "Checking text utility classes..." rg "(?:class|:class)=[\"\'{].*text-(header-text-secondary|orange-dark)" \ --type-add 'vue:*.vue' \ -g '!node_modules' \ -g '!dist' echo "Checking underline utility classes..." rg "(?:class|:class)=[\"\'{].*(underline|no-underline)" \ --type-add 'vue:*.vue' \ -g '!node_modules' \ -g '!dist' echo "Checking mark classes..." rg "(?:class|:class)=[\"\'{].*unraid_mark_[0-9]" \ --type-add 'vue:*.vue' \ -g '!node_modules' \ -g '!dist' # Also check for dynamic class bindings echo "Checking dynamic bindings..." rg "'(text-header-text-secondary|text-orange-dark|underline|no-underline|unraid_mark_[0-9])'\s*:" \ --type-add 'vue:*.vue' \ -g '!node_modules' \ -g '!dist'Length of output: 1260
Script:
#!/bin/bash # Search with simplified patterns echo "Checking text utility classes..." rg "text-header-text-secondary|text-orange-dark" \ --type-add 'vue:*.vue' \ -g '!node_modules' \ -g '!dist' \ -C 2 echo "Checking underline classes..." rg "class.*underline|:class.*underline" \ --type-add 'vue:*.vue' \ -g '!node_modules' \ -g '!dist' \ -C 2 echo "Checking mark classes..." rg "unraid_mark_[0-9]" \ --type-add 'vue:*.vue' \ -g '!node_modules' \ -g '!dist' \ -C 2Length of output: 29639
Here's an initial refactoring and cleanup effort for the UI components. I updated a few of the components in web and more will be migrated in followup PR's. This PR get's the build process updated, tailwind configs consolidated and moves this towards getting unraid-ui to act as the source of truth for Unraid apps.
This also solves the issue of having to import the tailwind declarations in each web component and also mostly resolves any tailwind tree shaking issues.
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes for Unraid UI Library
New Features
Component Improvements
Styling Updates
Developer Experience
Breaking Changes