feat: build out docker components #1427
Conversation
WalkthroughThis update refactors the Unraid UI component library by replacing external UI library dependencies with local UI primitives for Accordion, Dialog, DropdownMenu, and Select components. It introduces explicit TypeScript interfaces, consolidates exports, updates Storybook stories, and synchronizes usage across the codebase. Package dependencies are also updated to the latest Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant UI_Primitive as Local UI Primitive
participant User
User->>App: Triggers UI action (e.g., opens dialog, selects item)
App->>UI_Primitive: Passes props, events, and data
UI_Primitive-->>App: Emits explicit events (e.g., update:modelValue, select)
App-->>User: Updates UI based on event/selection
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learningsweb/components/Notifications/Sidebar.vue (5)⏰ Context from checks skipped due to timeout of 90000ms (4)
🔇 Additional comments (2)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (3)
web/components/ApiKey/ApiKeyCreate.vue (1)
185-185: Remove unnecessary whitespace.Line 185 appears to contain only whitespace.
- +unraid-ui/src/components/common/accordion/Accordion.vue (1)
31-49: Consider clarifying the dual usage pattern.While the component correctly handles both slot composition and props-driven usage, having both patterns available simultaneously might confuse developers about the intended usage. Consider adding documentation or comments about when to use each approach.
unraid-ui/src/components/common/dialog/Dialog.vue (1)
64-94: Consider extracting complex conditional logic for better maintainability.While the template is functional, the nested conditional rendering and dynamic component usage might benefit from helper computed properties or template decomposition to improve readability.
For example, consider extracting the content component selection:
const ContentComponent = computed(() => scrollable ? DialogScrollContent : DialogContent );And potentially breaking the footer logic into a separate component if it grows more complex.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
CLAUDE.md(3 hunks)unraid-ui/components.json(1 hunks)unraid-ui/src/components/common/accordion/Accordion.vue(1 hunks)unraid-ui/src/components/common/accordion/index.ts(1 hunks)unraid-ui/src/components/common/dialog/Dialog.vue(1 hunks)unraid-ui/src/components/common/dialog/DialogClose.vue(0 hunks)unraid-ui/src/components/common/dialog/index.ts(1 hunks)unraid-ui/src/components/ui/accordion/Accordion.vue(1 hunks)unraid-ui/src/components/ui/accordion/AccordionItem.vue(1 hunks)unraid-ui/src/components/ui/accordion/AccordionTrigger.vue(1 hunks)unraid-ui/src/components/ui/accordion/index.ts(1 hunks)unraid-ui/src/components/ui/dialog/Dialog.vue(1 hunks)unraid-ui/src/components/ui/dialog/DialogClose.vue(1 hunks)unraid-ui/src/components/ui/dialog/DialogContent.vue(1 hunks)unraid-ui/src/components/ui/dialog/DialogScrollContent.vue(2 hunks)unraid-ui/src/components/ui/dialog/index.ts(1 hunks)unraid-ui/stories/components/common/Accordion.stories.ts(1 hunks)unraid-ui/stories/components/common/Dialog.stories.ts(1 hunks)web/components/ApiKey/ApiKeyCreate.vue(2 hunks)
💤 Files with no reviewable changes (1)
- unraid-ui/src/components/common/dialog/DialogClose.vue
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Build Web App
- GitHub Check: Build API
- GitHub Check: Test API
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (41)
CLAUDE.md (7)
18-18: Consistent blank line before code block improves readability.
The inserted blank line cleanly separates the heading from the code example.
27-27: Add blank line before testing commands block.
This enhances visual separation of the “Testing & Code Quality” section.
34-34: Insert blank line before API development commands.
Maintains consistent formatting across sections.
42-42: Blank line before deployment instructions is helpful.
Keeps the “Deployment” section distinct.
50-50: Add blank line before architecture notes heading.
Improves separation between command sections and architecture overview.
53-53: Insert blank line before key patterns list.
Ensures consistent spacing in the “Key Patterns” subsection.
59-59: Add blank line before authentication section.
Enhances clarity by distinguishing high-level notes from security details.unraid-ui/src/components/ui/accordion/Accordion.vue (1)
1-19: LGTM! Clean wrapper component implementation.The component correctly wraps
AccordionRootwith proper TypeScript typing and prop/emit forwarding. The implementation follows Vue 3 best practices for creating wrapper components.unraid-ui/components.json (1)
13-13: Alias update supports the component restructuring.The broader alias scope from
@/components/commonto@/componentsproperly supports the new component organization withui/subdirectories.unraid-ui/src/components/ui/accordion/AccordionItem.vue (1)
15-15: Good styling consistency improvement.Adding the fixed
border-bclass ensures consistent bottom border styling across all accordion items while still allowing custom classes via props.unraid-ui/src/components/ui/dialog/DialogContent.vue (1)
25-25: Let’s retry searching without relying on ripgrep’s built-in Vue type:#!/bin/bash # Search for DialogPortal usage in all .vue files rg -A3 -B1 "DialogPortal" --glob "*.vue" # Search for any remaining useTeleport calls in .vue files rg -A3 -B1 "useTeleport" --glob "*.vue" # Search for raw <teleport> tags to see default teleport usage rg -A1 -B1 "<teleport" --glob "*.vue"unraid-ui/src/components/ui/accordion/AccordionTrigger.vue (1)
19-19: Styling simplification looks good.The updated classes remove visual clutter (borders, rounded corners) while adding useful hover feedback and maintaining all functional styling (flex layout, icon rotation).
unraid-ui/src/components/ui/accordion/index.ts (1)
1-4: Clean barrel export pattern.The accordion component exports follow a clear and consistent pattern.
unraid-ui/src/components/ui/dialog/Dialog.vue (1)
1-14: Well-structured wrapper component.The Dialog component properly forwards props and emits using TypeScript types and the useForwardPropsEmits composable. Clean implementation following established patterns.
unraid-ui/src/components/ui/dialog/DialogClose.vue (1)
1-11: Verify if emits should be forwarded.The DialogClose component only forwards props, unlike Dialog.vue which forwards both props and emits. Ensure DialogClose from reka-ui doesn't have emits that should be forwarded for consistency.
#!/bin/bash # Check if DialogClose has emits in reka-ui documentation or types rg -A 10 -B 5 "DialogCloseEmits\|DialogClose.*emit" --type tsunraid-ui/src/components/common/dialog/index.ts (1)
1-4: Good simplification aligning with refactor.The export simplification properly delegates subcomponent exports to the new ui/dialog module while maintaining the main Dialog export and type.
unraid-ui/src/components/ui/dialog/DialogScrollContent.vue (1)
16-16: ```shell
#!/bin/bashDisplay Dialog.vue around the content component to see passed props
sed -n '1,200p' unraid-ui/src/components/common/dialog/Dialog.vue
rg -n "component :is" -A 5 -B 5 unraid-ui/src/components/common/dialog/Dialog.vue</details> <details> <summary>unraid-ui/src/components/ui/dialog/index.ts (1)</summary> `1-9`: **Clean barrel export structure.** The centralized export pattern improves import ergonomics and maintains consistent naming conventions across all dialog components. </details> <details> <summary>unraid-ui/src/components/common/accordion/index.ts (2)</summary> `2-2`: **Good addition of TypeScript type exports.** Exporting the interfaces improves type safety and developer experience when using the accordion component. --- `3-3`: **Verify the new UI accordion components exist and are functional.** The export source changed from local `.vue` files to `@/components/ui/accordion`. Ensure all referenced components are properly implemented in the new location. ```shell #!/bin/bash # Description: Verify that the accordion UI components exist and exports match echo "Checking if accordion UI components exist..." # Check if the accordion UI directory and components exist fd "accordion" --type d unraid-ui/src/components/ui/ # Check if individual accordion components exist fd -e vue "Accordion" unraid-ui/src/components/ui/accordion/ # Check the exports in the UI accordion index file if [ -f "unraid-ui/src/components/ui/accordion/index.ts" ]; then echo "Contents of UI accordion index file:" cat unraid-ui/src/components/ui/accordion/index.ts else echo "UI accordion index file not found!" fiweb/components/ApiKey/ApiKeyCreate.vue (2)
199-214: Excellent dialog API improvements.The refactor to use
v-modeland explicit props significantly improves the component's usability:
v-modelis more idiomatic than separate:open/@update:openbindings- Explicit props for title, buttons, and loading states enhance clarity
- Moving form submission to
@primary-clickevent provides better separation of concerns
215-314: Simplified dialog content structure improves maintainability.Removing the nested dialog components (
DialogHeader,DialogTitle, etc.) in favor of the new dialog API makes the template much cleaner and easier to understand.unraid-ui/src/components/common/accordion/Accordion.vue (2)
4-17: Well-designed TypeScript interfaces.The
AccordionItemDataandAccordionPropsinterfaces provide excellent type safety and clear API contracts. The optional properties and sensible defaults enhance usability.
26-50: Flexible template design supports multiple usage patterns.The template elegantly supports both direct slot composition and props-driven configuration with scoped slots for customization. This provides excellent developer experience.
unraid-ui/src/components/common/dialog/Dialog.vue (3)
15-27: Comprehensive and well-designed TypeScript interface.The
DialogPropsinterface covers all necessary configuration options with appropriate optional properties and clear naming conventions.
43-54: Proper controlled component implementation.The emit definitions and handler functions follow Vue 3 best practices for controlled components, providing clear event boundaries and proper state management.
58-95: Feature-rich template with excellent flexibility.The template handles multiple usage patterns beautifully with conditional rendering for triggers, headers, and footers. The scoped slot system provides great customization options.
unraid-ui/stories/components/common/Dialog.stories.ts (6)
1-13: Excellent Storybook setup with proper TypeScript integration.The file structure follows best practices with clean imports, proper typing, and good use of the
satisfiesoperator for type safety.
15-47: Well-structured main story with comprehensive prop coverage.The story effectively demonstrates the Dialog component's core functionality with proper Vue 3 reactive state management and event handling patterns.
49-66: Good coverage of different dialog configurations.These stories effectively demonstrate simpler dialog variations, showing the flexibility of the component's prop-based configuration.
68-90: Excellent demonstration of slot-based customization.The story properly showcases custom trigger and content slots, demonstrating the component's flexibility for advanced use cases.
92-129: Comprehensive custom footer implementation.The story effectively demonstrates advanced footer customization with multiple action buttons and proper event handling separation.
131-181: Excellent coverage of edge cases and advanced patterns.These stories effectively demonstrate important use cases including footerless dialogs and programmatic control, providing complete documentation coverage.
unraid-ui/stories/components/common/Accordion.stories.ts (8)
1-25: Well-structured Storybook configuration with comprehensive controls.The file setup properly imports all accordion components and provides good argTypes configuration for interactive documentation.
28-64: Clean basic usage demonstration with realistic sample data.The story effectively showcases the items prop-based approach with good sample content and proper args forwarding.
67-119: Excellent demonstration of slot-based customization.The story properly showcases scoped slots for trigger and content customization, providing practical examples of advanced accordion usage.
122-183: Comprehensive demonstration of direct component composition.The story effectively showcases manual composition using individual accordion components, providing an important alternative to the items prop approach.
186-221: Clear demonstration of multiple mode with default values.The story effectively showcases the multiple accordion type with proper default value configuration and explanatory content.
224-265: Advanced pattern demonstration with clear documentation.The story effectively shows the mixed usage pattern combining items prop and direct composition, with helpful comments explaining the rendering order.
268-330: Sophisticated dynamic component pattern demonstration.The story showcases an advanced real-world pattern using dynamic component rendering with proper component mapping and scoped slot usage.
333-369: Important accessibility demonstration with disabled items.The story properly showcases disabled item functionality, covering an important accessibility and UX consideration with clear state differentiation.
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
unraid-ui/stories/components/common/Loading.stories.ts (1)
45-50: Fix unmatched closing tag in template.
The closing</div>on line 50 has no matching opening<div>, resulting in invalid markup. Remove the extra</div>or add the corresponding opening<div>wrapper to restore proper structure.
🧹 Nitpick comments (3)
unraid-ui/src/components/common/dialog/Dialog.vue (2)
65-65: Consider improving dynamic component readability.The ternary operator for dynamic component selection works but could be more readable.
Consider extracting this to a computed property:
+const ContentComponent = computed(() => + scrollable ? DialogScrollContent : DialogContent +);Then use in template:
- <component :is="scrollable ? DialogScrollContent : DialogContent"> + <component :is="ContentComponent">
83-94: Primary button implementation could be more robust.The primary button logic is functional but the loading state handling could be improved for better UX.
Consider this enhancement for better loading state management:
<Button v-if="primaryButtonText" variant="primary" :disabled="primaryButtonDisabled || primaryButtonLoading" + :loading="primaryButtonLoading" @click="handlePrimaryClick" > - <span v-if="primaryButtonLoading && primaryButtonLoadingText"> - {{ primaryButtonLoadingText }} - </span> - <span v-else>{{ primaryButtonText }}</span> + {{ primaryButtonLoading && primaryButtonLoadingText + ? primaryButtonLoadingText + : primaryButtonText }} </Button>unraid-ui/stories/components/common/DropdownMenu.stories.ts (1)
24-45: Props-based story is much cleaner.The new approach is significantly more maintainable than the previous manual composition. The declarative API using
triggeranditemsprops is intuitive.Consider adding a second story to demonstrate more complex scenarios:
+export const WithIcons: Story = { + args: { + trigger: 'Actions', + items: [ + { label: 'Edit', icon: 'edit' }, + { label: 'Delete', icon: 'trash', variant: 'destructive' }, + { type: 'separator' }, + { label: 'Share', icon: 'share' } + ], + }, +};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
unraid-ui/src/components.ts(2 hunks)unraid-ui/src/components/common/accordion/index.ts(1 hunks)unraid-ui/src/components/common/dialog/Dialog.vue(1 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenu.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuArrow.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuCheckboxItem.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuContent.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuGroup.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuItem.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuLabel.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuRadioGroup.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuRadioItem.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuSeparator.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuShortcut.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuSub.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuSubContent.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuSubTrigger.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuTrigger.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/index.ts(0 hunks)unraid-ui/src/index.ts(1 hunks)unraid-ui/stories/components/common/Accordion.stories.ts(1 hunks)unraid-ui/stories/components/common/Dialog.stories.ts(1 hunks)unraid-ui/stories/components/common/DropdownMenu.stories.ts(2 hunks)unraid-ui/stories/components/common/Loading.stories.ts(3 hunks)unraid-ui/stories/components/common/Sheet.stories.ts(1 hunks)
💤 Files with no reviewable changes (16)
- unraid-ui/src/components/common/dropdown-menu/DropdownMenu.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuArrow.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuGroup.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuSeparator.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuTrigger.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuItem.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuShortcut.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuLabel.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuRadioItem.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuSubContent.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuContent.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuCheckboxItem.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuSub.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuSubTrigger.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuRadioGroup.vue
- unraid-ui/src/components/common/dropdown-menu/index.ts
✅ Files skipped from review due to trivial changes (2)
- unraid-ui/stories/components/common/Sheet.stories.ts
- unraid-ui/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- unraid-ui/src/components/common/accordion/index.ts
- unraid-ui/stories/components/common/Dialog.stories.ts
- unraid-ui/stories/components/common/Accordion.stories.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)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (13)
unraid-ui/src/components/common/dialog/Dialog.vue (5)
2-13: Import structure looks clean and well-organized.The transition from external
reka-uito local UI components provides better control and customization. The imports are properly structured and follow naming conventions.
15-27: Well-defined TypeScript interface with comprehensive props.The
DialogPropsinterface covers all necessary dialog functionality with appropriate optional properties. The typing is clear and self-documenting.
29-41: Props destructuring follows Vue 3 best practices.Good use of default values and proper destructuring. The defaults are sensible and improve developer experience.
43-46: Emit definitions are properly typed for v-model pattern.The emit definitions correctly implement the v-model pattern with
update:modelValueand include the primary action event.
48-54: Event handlers are simple and focused.The handler functions are appropriately minimal, focusing solely on event emission without side effects.
unraid-ui/src/components.ts (2)
5-5: Verify the updated dropdown export path exists.The export path changed from
dropdown-menutodropdown, reflecting the component consolidation mentioned in the summary.#!/bin/bash # Verify the new dropdown export path exists and contains expected exports fd -t f "dropdown" unraid-ui/src/components/common/ --exec ls -la {} # Check if the new dropdown module exports what's expected cat unraid-ui/src/components/common/dropdown/index.ts 2>/dev/null || echo "Index file not found"
24-25: UI primitives export addition looks good.Adding the UI primitives export with clear documentation aligns with the component refactoring initiative to provide lower-level building blocks for composition.
#!/bin/bash # Verify the UI primitives directory exists and contains expected components fd -t d "ui" unraid-ui/src/components/ --exec ls -la {} # Check what's being exported from the UI module cat unraid-ui/src/components/ui/index.ts 2>/dev/null || echo "UI index file not found"unraid-ui/stories/components/common/DropdownMenu.stories.ts (2)
1-1: Import simplification looks good.The consolidated import reflects the component refactoring from multiple subcomponents to a single, more user-friendly DropdownMenu component.
9-16: Excellent addition of Storybook controls.The
alignandsidecontrols will make it much easier to test different dropdown positioning scenarios in Storybook.unraid-ui/stories/components/common/Loading.stories.ts (4)
1-4: Consistent import formatting.
Switching all import statements to single quotes aligns with the project’s style conventions.
20-23: Remove unused props in story render.
Dropping theargsparameter andv-bindforLoadingBarsince it has no configurable props keeps the story clean.
28-30: Remove unused props in story render.
Dropping theargsparameter andv-bindforLoadingSpinnersince it has no configurable props keeps the story clean.
38-38: Consistent string delimiting.
Switching the emptyclassprop value to single quotes aligns with the repository’s quotation style.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
unraid-ui/src/components/common/dropdown/DropdownMenu.vue(1 hunks)unraid-ui/src/components/common/dropdown/index.ts(1 hunks)unraid-ui/src/components/ui/dropdown/DropdownMenu.vue(1 hunks)unraid-ui/src/components/ui/dropdown/DropdownMenuCheckboxItem.vue(2 hunks)unraid-ui/src/components/ui/dropdown/DropdownMenuContent.vue(1 hunks)unraid-ui/src/components/ui/dropdown/DropdownMenuItem.vue(1 hunks)unraid-ui/src/components/ui/dropdown/DropdownMenuLabel.vue(1 hunks)unraid-ui/src/components/ui/dropdown/DropdownMenuRadioGroup.vue(1 hunks)unraid-ui/src/components/ui/dropdown/DropdownMenuRadioItem.vue(2 hunks)unraid-ui/src/components/ui/dropdown/DropdownMenuSeparator.vue(1 hunks)unraid-ui/src/components/ui/dropdown/DropdownMenuSub.vue(1 hunks)unraid-ui/src/components/ui/dropdown/DropdownMenuSubContent.vue(1 hunks)unraid-ui/src/components/ui/dropdown/DropdownMenuSubTrigger.vue(1 hunks)unraid-ui/src/components/ui/dropdown/DropdownMenuTrigger.vue(1 hunks)unraid-ui/src/components/ui/dropdown/index.ts(1 hunks)unraid-ui/src/components/ui/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- unraid-ui/src/components/ui/dropdown/index.ts
- unraid-ui/src/components/ui/dropdown/DropdownMenuTrigger.vue
- unraid-ui/src/components/ui/dropdown/DropdownMenuLabel.vue
- unraid-ui/src/components/common/dropdown/index.ts
- unraid-ui/src/components/ui/dropdown/DropdownMenuSubTrigger.vue
- unraid-ui/src/components/ui/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (14)
unraid-ui/src/components/ui/dropdown/DropdownMenu.vue (1)
12-12: LGTM - Clean simplification relying on type inference.The removal of explicit generic parameters is good as TypeScript can infer the types from the
propsandemitsarguments. This makes the code cleaner while maintaining type safety.#!/bin/bash # Verify that TypeScript can properly infer types without explicit generics # by checking if there are any type errors in dropdown components fd -e vue . unraid-ui/src/components/ui/dropdown/ --exec echo "Checking: {}" \; --exec grep -l "useForwardPropsEmits" {} \;unraid-ui/src/components/ui/dropdown/DropdownMenuRadioGroup.vue (1)
12-12: Consistent refactoring - good.This follows the same pattern as other dropdown components, removing explicit generic parameters to rely on type inference. The consistency across the codebase is appreciated.
unraid-ui/src/components/ui/dropdown/DropdownMenuSub.vue (1)
12-12: Consistent with the refactoring pattern.Same improvement as the other dropdown components - relying on type inference instead of explicit generics.
unraid-ui/src/components/ui/dropdown/DropdownMenuCheckboxItem.vue (3)
3-3: Good addition of reactive utility.Adding
reactiveOmitfrom@vueuse/coreis a clean approach for omitting properties from reactive objects.
12-12: Appropriate import update.The import change from
computedtoHTMLAttributesaligns with the new approach usingreactiveOmitinstead of manual computed properties.
17-17: Cleaner approach using reactiveOmit.Replacing the manual computed property with
reactiveOmit(props, 'class')is more declarative and leverages a well-tested utility function. This improves code maintainability.unraid-ui/src/components/ui/dropdown/DropdownMenuSubContent.vue (1)
3-3: Consistent refactoring with reactiveOmit.The same clean refactoring pattern as
DropdownMenuCheckboxItem.vue- replacing manual computed properties with thereactiveOmitutility. This consistency across components is good architectural practice.Also applies to: 10-10, 15-15
unraid-ui/src/components/ui/dropdown/DropdownMenuSeparator.vue (1)
3-3: Clean refactoring to VueUse utilities.The replacement of manual computed property with
reactiveOmitimproves code clarity and follows modern Vue patterns.Also applies to: 13-13
unraid-ui/src/components/ui/dropdown/DropdownMenuRadioItem.vue (1)
3-3: Consistent refactoring across dropdown components.Good use of
reactiveOmitfollowing the same pattern applied across other dropdown components.Also applies to: 18-18
unraid-ui/src/components/ui/dropdown/DropdownMenuItem.vue (1)
3-3: Maintains consistency with the refactoring pattern.The use of
reactiveOmitis consistent with the other dropdown components in this refactor.Also applies to: 11-11
unraid-ui/src/components/ui/dropdown/DropdownMenuContent.vue (1)
1-41: Well-structured UI primitive component.The new DropdownMenuContent component follows Vue 3 best practices with proper TypeScript integration, consistent use of
reactiveOmit, and comprehensive styling for dropdown animations and positioning.unraid-ui/src/components/common/dropdown/DropdownMenu.vue (3)
12-25: Well-designed TypeScript interfaces.The
DropdownMenuItemDataandDropdownMenuPropsinterfaces provide clear contracts for the component's API with good type safety.
46-76: Flexible component design with dual API support.The component nicely supports both declarative prop-based configuration and compositional slot-based usage, providing good developer experience flexibility.
1-1: Inconsistent PR title and content.The PR is titled "feat: build out docker components" but contains UI dropdown component refactoring. Ensure the PR title accurately reflects the changes.
Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
web/components/UserProfile/DropdownTrigger.vue (1)
43-43: Consider dynamic title behavior.The title now always returns "Open Dropdown" regardless of the actual dropdown state. While this simplification aligns with the external state management approach, consider whether this provides adequate accessibility information to users.
unraid-ui/src/components/common/dropdown-menu/DropdownMenu.vue (1)
61-73: Comprehensive item type rendering.The template properly handles all item types (separator, label, item) with appropriate components and maintains disabled state and click handling. The key generation could be improved for better performance.
Consider using a more predictable key generation:
- <template v-for="item in props.items" :key="item.label || Math.random()"> + <template v-for="(item, index) in props.items" :key="`${item.label || item.type}-${index}`">unraid-ui/stories/components/common/DropdownMenu.stories.ts (1)
221-223: Consider using design system styles instead of inline CSS.The custom trigger button uses inline styles. Consider using design system classes or CSS-in-JS for better consistency and maintainability.
- <button style="padding: 8px 16px; background: #3b82f6; color: white; border: none; border-radius: 4px; cursor: pointer;"> + <button class="btn btn-primary"> Custom Button Trigger </button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
unraid-ui/src/components/common/accordion/Accordion.vue(1 hunks)unraid-ui/src/components/common/dialog/Dialog.vue(1 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenu.vue(1 hunks)unraid-ui/src/components/common/dropdown-menu/index.ts(1 hunks)unraid-ui/src/components/ui/accordion/AccordionRoot.vue(1 hunks)unraid-ui/src/components/ui/accordion/index.ts(1 hunks)unraid-ui/src/components/ui/dialog/DialogRoot.vue(1 hunks)unraid-ui/src/components/ui/dialog/index.ts(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuCheckboxItem.vue(2 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuContent.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuItem.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuLabel.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioGroup.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioItem.vue(2 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRoot.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSeparator.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSub.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubContent.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubTrigger.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuTrigger.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/index.ts(1 hunks)unraid-ui/src/components/ui/index.ts(1 hunks)unraid-ui/src/index.ts(1 hunks)unraid-ui/stories/components/common/DropdownMenu.stories.ts(2 hunks)web/__test__/components/UserProfile.test.ts(1 hunks)web/__test__/mocks/ui-components.ts(1 hunks)web/components/UserProfile.ce.vue(2 hunks)web/components/UserProfile/DropdownMenu.vue(0 hunks)web/components/UserProfile/DropdownTrigger.vue(1 hunks)
💤 Files with no reviewable changes (1)
- web/components/UserProfile/DropdownMenu.vue
✅ Files skipped from review due to trivial changes (9)
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioGroup.vue
- web/test/components/UserProfile.test.ts
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRoot.vue
- web/test/mocks/ui-components.ts
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubContent.vue
- unraid-ui/src/components/ui/dialog/DialogRoot.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuContent.vue
- unraid-ui/src/components/ui/accordion/AccordionRoot.vue
- unraid-ui/src/components/ui/dropdown-menu/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- unraid-ui/src/index.ts
- unraid-ui/src/components/ui/dialog/index.ts
- unraid-ui/src/components/common/dialog/Dialog.vue
🧰 Additional context used
📓 Path-based instructions (2)
`web/**/*`: Always run pnpm codegen for GraphQL code generation in the web directory. All GraphQL under web/ must follow this naming convention (.query.ts for queries, .mutation.ts...
web/**/*: Always run pnpm codegen for GraphQL code generation in the web directory.
All GraphQL under web/ must follow this naming convention (.query.ts for queries, .mutation.ts for mutations).
web/components/UserProfile.ce.vueweb/components/UserProfile/DropdownTrigger.vue
`web/**/*`: Always run `pnpm codegen` for GraphQL code generation in the web directory All GraphQL files under web/ must follow the naming convention: queries in .query.ts files an...
web/**/*: Always runpnpm codegenfor GraphQL code generation in the web directory
All GraphQL files under web/ must follow the naming convention: queries in .query.ts files and mutations in .mutation.ts files
web/components/UserProfile.ce.vueweb/components/UserProfile/DropdownTrigger.vue
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (29)
unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSub.vue (1)
12-12: LGTM: Simplified TypeScript inferenceThe removal of explicit generic type parameters leverages TypeScript's type inference, making the code cleaner while maintaining type safety.
unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSeparator.vue (1)
3-3: LGTM: Improved prop handling with reactiveOmitReplacing the manual computed property with
reactiveOmitfrom@vueuse/corereduces boilerplate and leverages a well-maintained utility function for reactive prop exclusion.Also applies to: 5-5, 13-13
unraid-ui/src/components/ui/dropdown-menu/DropdownMenuItem.vue (1)
3-3: LGTM: Consistent refactoring patternThe adoption of
reactiveOmitfollows the same pattern as other dropdown menu components, improving consistency and reducing manual prop destructuring.Also applies to: 5-5, 11-11
unraid-ui/src/components/ui/index.ts (2)
1-4: LGTM: Centralized UI primitive exportsThe centralized export pattern improves import ergonomics and module organization.
1-4: PR title mismatch with contentThe PR is titled "feat: build out docker components" but the changes are Vue.js UI components for dropdown menus, accordions, and dialogs. Consider updating the PR title to accurately reflect the UI component refactoring work.
Likely an incorrect or invalid review comment.
unraid-ui/src/components/ui/dropdown-menu/DropdownMenuTrigger.vue (2)
6-6: LGTM: Simplified TypeScript inferenceRemoving explicit generic type parameters leverages TypeScript's inference capabilities effectively.
10-13: LGTM: Enhanced cursor stylingThe addition of cursor pointer styles and data-state-aware cursor behavior improves user experience by providing clear visual feedback for interactive elements.
unraid-ui/src/components/ui/dropdown-menu/DropdownMenuCheckboxItem.vue (1)
3-3: LGTM! Clean refactoring to use VueUse utilities.The replacement of manual computed property with
reactiveOmitimproves code readability and leverages VueUse's optimized reactive utilities.Also applies to: 17-17
unraid-ui/src/components/ui/dropdown-menu/DropdownMenuLabel.vue (1)
3-3: Consistent refactoring pattern applied correctly.Good to see the same
reactiveOmitpattern being applied consistently across all dropdown menu components.Also applies to: 11-11
unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioItem.vue (1)
3-3: Refactoring pattern consistently applied.The
reactiveOmitrefactoring maintains consistency with the other dropdown menu components and improves prop handling.Also applies to: 18-18
unraid-ui/src/components/ui/accordion/index.ts (1)
1-4: Clean barrel export for accordion UI primitives.The module structure follows standard practices for organizing and re-exporting UI components.
unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubTrigger.vue (2)
3-3: Refactoring pattern completed across all dropdown components.This completes the consistent application of
reactiveOmitacross all dropdown menu UI primitives, improving code maintainability.Also applies to: 10-10
1-29: PR title doesn't match the actual changes.The PR title mentions "build out docker components" but the changes are actually UI component refactoring for dropdown menus and accordion primitives. Consider updating the PR title to reflect the actual work being done.
Likely an incorrect or invalid review comment.
web/components/UserProfile.ce.vue (2)
7-7: Clean migration to the new dropdown API.The import change from custom component to
@unraid/uialigns with the broader refactoring to use UI primitives.
140-153: ```shell
#!/bin/bashSearch for any remaining usage of the old UpcDropdownMenu component across .vue, .ts, and .js files
rg "UpcDropdownMenu" -n --glob ".vue" --glob ".ts" --glob "*.js" -A3
</details> <details> <summary>web/components/UserProfile/DropdownTrigger.vue (1)</summary> `49-49`: **Minor styling adjustment.** The removal of `focus:opacity-75` class simplifies the styling. Ensure this doesn't negatively impact the focus indicators for accessibility. </details> <details> <summary>unraid-ui/src/components/common/dropdown-menu/index.ts (1)</summary> `1-2`: **Excellent API simplification.** The reduction from multiple subcomponent exports to just the main `DropdownMenu` component and its types creates a cleaner, more maintainable public API. The internal complexity is now properly encapsulated in UI primitives. </details> <details> <summary>unraid-ui/src/components/common/accordion/Accordion.vue (3)</summary> `9-22`: **Well-structured TypeScript interfaces.** The `AccordionItemData` and `AccordionProps` interfaces provide clear type safety and good API documentation. The optional properties and sensible structure make the component flexible and easy to use. --- `24-27`: **Good default prop values.** The defaults for `type: 'single'` and `collapsible: true` provide sensible behavior out of the box while remaining configurable. --- `37-59`: **Flexible dual usage pattern.** The template elegantly supports both slot-based composition (line 38) and props-driven usage (lines 41-59) with scoped slots for customization. This provides excellent developer experience for different use cases. </details> <details> <summary>unraid-ui/src/components/common/dropdown-menu/DropdownMenu.vue (3)</summary> `12-25`: **Excellent TypeScript interface design.** The interfaces clearly define the dropdown menu's data structure and props. The `DropdownMenuItemData` type union ('item' | 'label' | 'separator') provides good type safety for different menu item types. --- `33-42`: **Proper event handling implementation.** The `handleItemClick` function correctly chains the item's `onClick` callback with the component's `select` event emission, providing both local and parent component integration points. --- `47-54`: **Smart trigger slot handling.** The conditional rendering of default trigger button vs. custom trigger slot provides excellent fallback behavior while maintaining flexibility for custom implementations. </details> <details> <summary>unraid-ui/stories/components/common/DropdownMenu.stories.ts (6)</summary> `9-16`: **Well-structured argTypes configuration.** The control configurations for `align` and `side` props provide proper Storybook controls that match the expected enum values for dropdown positioning. --- `25-45`: **Good foundation story with proper args usage.** The BasicUsage story effectively demonstrates the core functionality and serves as a solid foundation for the component documentation. --- `48-124`: **Excellent variety of realistic use cases.** These stories (UserAccountMenu, FileOperationsMenu, ContextMenu, SettingsMenu) provide comprehensive coverage of real-world dropdown menu patterns, including proper use of separators, labels, and disabled states. --- `234-236`: **Smart approach for demonstrating scroll behavior.** Using `Array.from` to programmatically generate menu items is an elegant way to test and demonstrate the component's handling of long lists without cluttering the story definition. --- `1-246`: **Comprehensive refactor that significantly improves component documentation.** This refactor transforms the story file from basic examples to a comprehensive showcase of the DropdownMenu component's capabilities. The 11 different stories cover all major use cases and API features, making this an excellent reference for developers. --- `127-147`: ```shell #!/bin/bash # Inspect the UI component to see how disabled items are handled file="unraid-ui/src/components/ui/dropdown-menu/DropdownMenuItem.vue" echo "===== First 200 lines of $file =====" sed -n '1,200p' "$file" echo echo "===== Disabled prop and click handler in $file =====" rg -n "defineProps" -C3 "$file" rg -n "disabled" -C3 "$file" rg -n "@click" -C3 "$file"
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
unraid-ui/src/components/ui/select/SelectScrollUpButton.vue (1)
3-10: Systematic refactoring completed.Excellent completion of the consistent refactoring pattern across all select components. This systematic approach improves code maintainability significantly.
Consider applying this same refactoring pattern to other UI component families (accordion, dialog, dropdown-menu) if they still use manual computed prop destructuring for similar functionality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
unraid-ui/src/components/common/dropdown-menu/DropdownMenu.vue(1 hunks)unraid-ui/src/components/form/select/Select.vue(1 hunks)unraid-ui/src/components/form/select/index.ts(1 hunks)unraid-ui/src/components/ui/index.ts(1 hunks)unraid-ui/src/components/ui/select/SelectContent.vue(2 hunks)unraid-ui/src/components/ui/select/SelectGroup.vue(1 hunks)unraid-ui/src/components/ui/select/SelectItem.vue(2 hunks)unraid-ui/src/components/ui/select/SelectRoot.vue(1 hunks)unraid-ui/src/components/ui/select/SelectScrollDownButton.vue(1 hunks)unraid-ui/src/components/ui/select/SelectScrollUpButton.vue(1 hunks)unraid-ui/src/components/ui/select/SelectSeparator.vue(1 hunks)unraid-ui/src/components/ui/select/SelectTrigger.vue(1 hunks)unraid-ui/src/components/ui/select/index.ts(1 hunks)unraid-ui/src/forms/Select.vue(3 hunks)unraid-ui/stories/components/form/Select.stories.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- unraid-ui/src/components/ui/select/SelectRoot.vue
- unraid-ui/src/components/ui/select/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- unraid-ui/src/components/ui/index.ts
- unraid-ui/src/components/common/dropdown-menu/DropdownMenu.vue
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (17)
unraid-ui/src/components/ui/select/SelectGroup.vue (1)
3-9: Let's retry the search by explicitly targeting.vuefiles:#!/bin/bash # Search for manual computed prop destructuring in all .vue files find . -type f -name '*.vue' -print0 | xargs -0 rg -n -C2 "computed\s*\(\s*\(\s*props" # Search for reactiveOmit usage in all .vue files find . -type f -name '*.vue' -print0 | xargs -0 rg -n "reactiveOmit"unraid-ui/src/components/ui/select/SelectSeparator.vue (1)
3-9: Consistent refactoring pattern applied.Good to see the same improvement pattern applied consistently across select components.
unraid-ui/src/components/ui/select/SelectItem.vue (1)
3-16: Consistent refactoring continues.The systematic application of
reactiveOmitacross select components maintains consistency and improves maintainability.unraid-ui/src/components/ui/select/SelectTrigger.vue (1)
3-10: Systematic refactoring continues.The consistent application of the refactoring pattern across components demonstrates good systematic approach to code improvement.
unraid-ui/src/components/ui/select/SelectScrollDownButton.vue (1)
3-10: LGTM! Good refactoring to use standard utility.The replacement of manual computed property logic with
reactiveOmitfrom@vueuse/coreimproves code consistency and leverages a well-tested utility. This pattern aligns with the broader refactoring effort across select components.unraid-ui/src/components/ui/select/SelectContent.vue (2)
3-3: LGTM! Consistent refactoring pattern applied.The adoption of
reactiveOmitutility follows the same pattern as other select components, improving code consistency and maintainability.Also applies to: 12-12, 24-24
30-30: Teleport logic removal simplifies component usage.The removal of teleport-related props (
forceMount,to) fromSelectPortalaligns with the broader simplification effort across dialog and select components.unraid-ui/src/forms/Select.vue (2)
7-10: Import path updated to use UI primitives.The change from form-specific select components to UI primitives (
@/components/ui/select) reflects the new modular architecture. The import ofSelectRootinstead ofSelectaligns with the component usage below.
44-50: I need to inspect howdefinePropsis declared for SelectRoot to confirm prop compatibility. Let’s pull in those lines:#!/bin/bash # Show defineProps usage in SelectRoot.vue rg -n "defineProps" -A5 unraid-ui/src/components/ui/select/SelectRoot.vue # Look for the SelectRootProps/type definitions in the repo rg -n "SelectRootProps" -A5 .unraid-ui/src/components/form/select/index.ts (1)
2-2: Clean export consolidation improves API design.Simplifying exports to only include the main
Selectcomponent and its types (SelectItemType,SelectProps) follows good module design principles by hiding internal implementation details and providing a cleaner public API.unraid-ui/stories/components/form/Select.stories.ts (6)
1-3: Simplified imports support new component architecture.The updated imports reflect the new structure where only the main
Selectcomponent is imported, withreffrom Vue for reactive state management in stories.
8-37: Excellent documentation enhancement.The comprehensive meta configuration with detailed component description and argTypes significantly improves the developer experience by clearly documenting all available props and their purposes.
44-63: Well-structured story demonstrates basic usage.The
SimpleArraystory effectively demonstrates basic Select usage with v-model binding and provides helpful debugging output showing selected value and items.
65-123: Comprehensive coverage of Select features.The additional stories (
ObjectArray,GroupedItems,WithDisabledItems,WithSeparators) provide excellent coverage of the Select component's advanced features, making it easy for developers to understand and implement different use cases.
125-173: Advanced usage patterns well demonstrated.The
ControlledValueandMultipleSelectionstories showcase important usage patterns for controlled components and multi-select functionality, with clear reactive state management examples.
175-202: Custom slots demonstrate extensibility.The
CustomSlotsstory effectively shows how to customize item rendering, providing a practical example of the component's flexibility for custom UI requirements.unraid-ui/src/components/form/select/Select.vue (1)
1-196: Well-implemented Select component with comprehensive featuresThe refactor successfully transforms the component into a feature-rich select with excellent TypeScript support, proper composition patterns, and flexible item configurations.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
unraid-ui/src/components/form/select/Select.vue (2)
80-80: Fix fallback logic for custom key properties.The current fallback logic could cause unexpected behavior when custom keys have falsy values. If
item[props.labelKey]oritem[props.valueKey]exists but is falsy (e.g., empty string, 0), it will incorrectly fall back to default properties.Apply this fix to check for property existence rather than truthiness:
- return String(item[props.labelKey] || item.label || item.value); + return String(props.labelKey in item ? item[props.labelKey] : (item.label ?? item.value));- const value = item[props.valueKey] || item.value; + const value = props.valueKey in item ? item[props.valueKey] : item.value;Also applies to: 90-90
189-189: Handle null values explicitly.Defaulting to empty string when
getItemValuereturns null could cause issues with value matching.Consider filtering out non-selectable items earlier or handling null values explicitly:
- :value="getItemValue(item) || ''" + :value="getItemValue(item)!"Since type guards ensure only selectable items reach this point, the non-null assertion is safe.
🧹 Nitpick comments (4)
unraid-ui/stories/components/form/Select.stories.ts (2)
44-276: Comprehensive story coverage with potential for DRY optimization.The stories provide excellent coverage of Select component functionality across different scenarios. However, there's significant code duplication in the render functions.
Consider extracting a common render factory:
+const createSelectStory = (customTemplate?: string) => ({ + render: (args) => ({ + components: { Select }, + setup() { + const value = ref(args.defaultValue || null); + return { args, value }; + }, + template: customTemplate || ` + <div class="space-y-4"> + <Select + v-model="value" + v-bind="args" + /> + <p class="text-sm text-muted-foreground">Selected: {{ value }}</p> + </div> + `, + }), +}); export const SimpleArray: Story = { - render: (args) => ({ - components: { Select }, - setup() { - const value = ref(null); - return { args, value }; - }, - template: `...`, - }), + ...createSelectStory(), args: { ... }, };
247-247: Inconsistent default value initialization.Most stories initialize with
ref(null)but MultipleSelection usesref(['apple', 'orange'])and ControlledValue usesref('banana').Consider using a consistent pattern with
args.defaultValue:- const value = ref(['apple', 'orange']); + const value = ref(args.defaultValue || []);Then add to args:
args: { + defaultValue: ['apple', 'orange'], placeholder: 'Select fruits', // ... },unraid-ui/src/components/form/select/Select.vue (2)
99-101: Improve type guard reliability.The current type guard assumes that if the first item is an array, all items are arrays, which may not be reliable.
-function isGroupedItems(items: SelectItemType[] | SelectItemType[][]): items is SelectItemType[][] { - return Array.isArray(items) && items.length > 0 && Array.isArray(items[0]); -} +function isGroupedItems(items: SelectItemType[] | SelectItemType[][]): items is SelectItemType[][] { + return Array.isArray(items) && items.length > 0 && items.every(item => Array.isArray(item)); +}
16-16: Clarify AcceptableValue type usage.The
AcceptableValuetype includesRecord<string, unknown>but this doesn't seem to be used in the component logic.Consider simplifying if object values aren't supported:
-type AcceptableValue = SelectValueType | SelectValueType[] | Record<string, unknown> | null; +type AcceptableValue = SelectValueType | SelectValueType[] | null;Or add logic to handle object values if they are intended to be supported.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
unraid-ui/src/components/common/dropdown-menu/DropdownMenu.vue(1 hunks)unraid-ui/src/components/form/select/Select.vue(1 hunks)unraid-ui/stories/components/form/Select.stories.ts(1 hunks)web/components/ApiKey/ApiKeyCreate.vue(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web/components/ApiKey/ApiKeyCreate.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenu.vue
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
unraid-ui/src/components/form/select/Select.vue (1)
152-154: Removeanytype for better type safety.The function parameter should use specific typing instead of
any.-function handleUpdateModelValue(value: AcceptableValue) { +function handleUpdateModelValue(value: SelectValueType | SelectValueType[]) { emit('update:modelValue', value); }Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
unraid-ui/src/components/form/select/Select.vue (1)
99-101: Improve grouped items validation for robustness.The current implementation only checks if the first element is an array, which could cause issues with mixed-type arrays.
Consider validating all elements:
function isGroupedItems(items: SelectItemType[] | SelectItemType[][]): items is SelectItemType[][] { - return Array.isArray(items) && items.length > 0 && Array.isArray(items[0]); + return Array.isArray(items) && items.length > 0 && items.every(item => Array.isArray(item)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
unraid-ui/src/components/form/select/Select.vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (6)
unraid-ui/src/components/form/select/Select.vue (6)
2-53: Well-structured type system and imports.The refactor to local UI primitives with comprehensive TypeScript interfaces provides excellent type safety and flexibility. The type definitions clearly separate different item types and provide proper prop typing.
66-76: Robust type guard implementation.The type guard functions properly handle null safety and type narrowing, enabling accurate discrimination between different item types.
78-97: Previous fallback logic issues have been properly addressed.The utility functions now correctly use property existence checks (
props.labelKey in item) rather than truthiness, preventing incorrect fallbacks when custom keys have falsy values.
103-150: Excellent data transformation and display logic.The computed properties efficiently handle item normalization, filtering, and multiple selection display with appropriate truncation for large selections.
158-201: Well-structured template with proper null value handling.The template efficiently renders different item types with appropriate conditional logic. The non-null assertion at line 189 is safe due to prior filtering, addressing previous concerns about null value handling.
152-154: Type-safe event handling resolves previous concerns.The typed event handler function properly addresses the previous issue about using
anytypes for better type safety.
mdatelle
force-pushed
the
feat/build-out-docker-components
branch
from
84ee840 to
ede3c35
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
unraid-ui/stories/components/common/Loading.stories.ts (1)
3-3: The Error import still shadows the global Error object.This issue was flagged in a previous review but remains unaddressed. Rename the import to avoid confusion.
unraid-ui/src/components/common/dialog/Dialog.vue (1)
58-98: Accessibility attributes still missing.The dialog template lacks explicit ARIA attributes for screen readers and keyboard navigation, which was identified in a previous review.
The underlying UI primitives may handle some accessibility features, but explicit ARIA attributes should still be added:
aria-labelledbyconnecting to the dialog titlearia-describedbyconnecting to the description- Proper focus management for dialog interactions
Consider adding these attributes to the
DialogRootorDialogContentcomponents to ensure full accessibility compliance.
🧹 Nitpick comments (2)
unraid-ui/stories/components/form/Select.stories.ts (1)
44-349: Consider reducing story template duplication.While the stories are comprehensive, there's significant duplication in the render functions and templates. Consider creating a reusable story renderer helper to reduce maintenance overhead.
Example approach:
+const createSelectStory = (customTemplate?: string) => ({ + components: { Select }, + setup() { + const value = ref(null); + return { args, value }; + }, + template: customTemplate || ` + <div class="space-y-4"> + <Select + v-model="value" + :items="args.items" + :placeholder="args.placeholder" + :valueKey="args.valueKey" + :labelKey="args.labelKey" + :multiple="args.multiple" + :disabled="args.disabled" + /> + <p class="text-sm text-muted-foreground">Selected: {{ value }}</p> + </div> + ` +});unraid-ui/src/components/form/select/Select.vue (1)
103-150: Consider breaking down complex computed properties.While the computed properties work correctly, the component has grown quite complex with many responsibilities. Consider extracting some of this logic into composables for better maintainability.
Example approach:
// composables/useSelectItems.ts export function useSelectItems(props: SelectProps) { const itemGroups = computed(() => { // Extract grouping logic }); const renderableItems = computed(() => { // Extract filtering logic }); // Return organized utilities return { itemGroups, renderableItems, /* ... */ }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (60)
CLAUDE.md(3 hunks)unraid-ui/components.json(1 hunks)unraid-ui/src/components/common/accordion/Accordion.vue(1 hunks)unraid-ui/src/components/common/accordion/index.ts(1 hunks)unraid-ui/src/components/common/dialog/Dialog.vue(1 hunks)unraid-ui/src/components/common/dialog/DialogClose.vue(0 hunks)unraid-ui/src/components/common/dialog/index.ts(1 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenu.vue(1 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuArrow.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuContent.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/index.ts(1 hunks)unraid-ui/src/components/form/select/Select.vue(1 hunks)unraid-ui/src/components/form/select/index.ts(1 hunks)unraid-ui/src/components/ui/accordion/AccordionItem.vue(1 hunks)unraid-ui/src/components/ui/accordion/AccordionRoot.vue(1 hunks)unraid-ui/src/components/ui/accordion/AccordionTrigger.vue(1 hunks)unraid-ui/src/components/ui/accordion/index.ts(1 hunks)unraid-ui/src/components/ui/dialog/DialogClose.vue(1 hunks)unraid-ui/src/components/ui/dialog/DialogContent.vue(1 hunks)unraid-ui/src/components/ui/dialog/DialogRoot.vue(1 hunks)unraid-ui/src/components/ui/dialog/DialogScrollContent.vue(2 hunks)unraid-ui/src/components/ui/dialog/index.ts(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuCheckboxItem.vue(2 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuContent.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuItem.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuLabel.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioGroup.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioItem.vue(2 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRoot.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSeparator.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSub.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubContent.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubTrigger.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuTrigger.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/index.ts(1 hunks)unraid-ui/src/components/ui/index.ts(1 hunks)unraid-ui/src/components/ui/select/SelectContent.vue(2 hunks)unraid-ui/src/components/ui/select/SelectGroup.vue(1 hunks)unraid-ui/src/components/ui/select/SelectItem.vue(2 hunks)unraid-ui/src/components/ui/select/SelectRoot.vue(1 hunks)unraid-ui/src/components/ui/select/SelectScrollDownButton.vue(1 hunks)unraid-ui/src/components/ui/select/SelectScrollUpButton.vue(1 hunks)unraid-ui/src/components/ui/select/SelectSeparator.vue(1 hunks)unraid-ui/src/components/ui/select/SelectTrigger.vue(1 hunks)unraid-ui/src/components/ui/select/index.ts(1 hunks)unraid-ui/src/forms/Select.vue(3 hunks)unraid-ui/src/index.ts(1 hunks)unraid-ui/stories/components/common/Accordion.stories.ts(1 hunks)unraid-ui/stories/components/common/Dialog.stories.ts(1 hunks)unraid-ui/stories/components/common/DropdownMenu.stories.ts(2 hunks)unraid-ui/stories/components/common/Loading.stories.ts(3 hunks)unraid-ui/stories/components/common/Sheet.stories.ts(1 hunks)unraid-ui/stories/components/form/Select.stories.ts(1 hunks)web/__test__/components/UserProfile.test.ts(1 hunks)web/__test__/mocks/ui-components.ts(1 hunks)web/components/ApiKey/ApiKeyCreate.vue(2 hunks)web/components/UserProfile.ce.vue(2 hunks)web/components/UserProfile/DropdownMenu.vue(0 hunks)web/components/UserProfile/DropdownTrigger.vue(1 hunks)web/helpers/globals.d.ts(1 hunks)
💤 Files with no reviewable changes (4)
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuArrow.vue
- unraid-ui/src/components/common/dialog/DialogClose.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuContent.vue
- web/components/UserProfile/DropdownMenu.vue
✅ Files skipped from review due to trivial changes (2)
- web/helpers/globals.d.ts
- unraid-ui/src/components/ui/dialog/index.ts
🚧 Files skipped from review as they are similar to previous changes (49)
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSub.vue
- unraid-ui/components.json
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioGroup.vue
- unraid-ui/stories/components/common/Sheet.stories.ts
- unraid-ui/src/components/ui/select/SelectTrigger.vue
- unraid-ui/src/index.ts
- unraid-ui/src/components/ui/dialog/DialogContent.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubContent.vue
- unraid-ui/src/components/ui/select/SelectItem.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuItem.vue
- web/test/components/UserProfile.test.ts
- unraid-ui/src/components/ui/select/SelectGroup.vue
- unraid-ui/src/components/ui/index.ts
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSeparator.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuLabel.vue
- unraid-ui/src/components/ui/accordion/AccordionItem.vue
- unraid-ui/src/components/ui/select/SelectSeparator.vue
- unraid-ui/src/components/ui/accordion/AccordionTrigger.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioItem.vue
- web/components/UserProfile.ce.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubTrigger.vue
- web/components/UserProfile/DropdownTrigger.vue
- unraid-ui/src/components/ui/select/SelectScrollUpButton.vue
- unraid-ui/src/components/common/dialog/index.ts
- unraid-ui/src/components/ui/accordion/index.ts
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuCheckboxItem.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuTrigger.vue
- unraid-ui/src/forms/Select.vue
- CLAUDE.md
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRoot.vue
- unraid-ui/src/components/ui/select/SelectContent.vue
- unraid-ui/src/components/ui/dialog/DialogScrollContent.vue
- unraid-ui/src/components/ui/select/SelectScrollDownButton.vue
- unraid-ui/src/components/common/accordion/index.ts
- unraid-ui/src/components/ui/dialog/DialogRoot.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuContent.vue
- unraid-ui/src/components/ui/select/SelectRoot.vue
- unraid-ui/src/components/ui/accordion/AccordionRoot.vue
- unraid-ui/src/components/ui/dialog/DialogClose.vue
- unraid-ui/src/components/form/select/index.ts
- unraid-ui/src/components/ui/select/index.ts
- web/test/mocks/ui-components.ts
- unraid-ui/src/components/common/accordion/Accordion.vue
- unraid-ui/src/components/common/dropdown-menu/index.ts
- unraid-ui/src/components/common/dropdown-menu/DropdownMenu.vue
- unraid-ui/stories/components/common/Dialog.stories.ts
- web/components/ApiKey/ApiKeyCreate.vue
- unraid-ui/src/components/ui/dropdown-menu/index.ts
- unraid-ui/stories/components/common/Accordion.stories.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)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build Web App
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (25)
unraid-ui/stories/components/common/Loading.stories.ts (3)
1-2: Good stylistic consistency with single quotes.The consistent use of single quotes across import statements and properties improves code consistency.
Also applies to: 7-7
20-20: Good cleanup removing unused parameters.Removing the unused
argsparameters from render functions that don't use them improves code clarity.Also applies to: 28-28
22-22: Consistent quote usage in template strings.The conversion to single quotes maintains consistency with the overall codebase style.
Also applies to: 30-30, 38-38
unraid-ui/src/components/common/dialog/Dialog.vue (5)
2-13: Clean import structure with local UI primitives.The refactoring to use local UI dialog primitives provides better control and consistency across the application.
15-27: Well-defined TypeScript interface.The
DialogPropsinterface is comprehensive and covers all the necessary configuration options for the dialog component.
29-46: Proper Vue 3 composition API usage.The props destructuring with defaults and typed emits follow Vue 3 best practices correctly.
48-54: Clean event handling implementation.The event handlers properly manage the v-model pattern and primary button interactions.
65-65: Clever dynamic component usage.The conditional component switching between
DialogScrollContentandDialogContentbased on thescrollableprop is an elegant solution for handling different content types.unraid-ui/stories/components/common/DropdownMenu.stories.ts (13)
1-1: Import statement looks good.The import correctly references the DropdownMenu component from the expected path.
9-16: ArgTypes configuration is well-structured.The select controls for
alignandsideprops provide good interactive documentation for users exploring the component.
24-45: BasicUsage story demonstrates the component effectively.The story shows the declarative API well with props-based configuration and proper template usage.
48-64: UserAccountMenu story showcases realistic usage patterns.Good example of using labels, separators, and disabled items to create a typical user account menu.
67-84: FileOperationsMenu provides practical context.The story demonstrates common file operations with appropriate grouping and disabled states.
86-104: ContextMenu story shows browser-like interactions.Good example of context menu patterns with realistic disabled states and menu structure.
107-124: SettingsMenu demonstrates effective grouping.The use of label types for grouping menu sections provides a clear example of organizing complex menus.
127-147: WithClickHandlers story shows event handling.The onClick handlers demonstrate proper event handling, including the disabled item behavior.
149-173: AlignmentVariations provides visual comparison.The side-by-side layout effectively demonstrates the different alignment options.
175-204: SidePositions story shows positioning options.The centered layout with gaps provides clear visual demonstration of the different side positioning options.
206-228: CustomTrigger demonstrates slot usage.Good example of using the trigger slot to provide custom button styling and content.
230-238: LongMenuWithScroll tests scrolling behavior.The dynamically generated array demonstrates how the component handles many items.
240-246: EmptyState story covers edge case.Good practice to include an empty state story to demonstrate component behavior with no items.
unraid-ui/stories/components/form/Select.stories.ts (1)
8-37: Excellent comprehensive documentation and story setup.The detailed argTypes and component documentation provide clear guidance for developers using this component. The stories cover all major use cases effectively.
unraid-ui/src/components/form/select/Select.vue (3)
14-53: Well-designed type system with comprehensive interfaces.The type definitions properly handle the complex item structures and provide good developer experience with clear interfaces for different item types.
66-97: Robust type guards and utility functions.The type guards properly distinguish between different item types, and the utility functions handle edge cases well. The fallback logic correctly uses property existence checks rather than truthiness.
158-201: Clean template structure with proper slot usage.The template effectively uses the UI primitives and provides flexible slot customization. The conditional rendering logic is clear and handles all item types appropriately.
mdatelle
force-pushed
the
feat/build-out-docker-components
branch
2 times, most recently
from
709480a to
344fb72
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
unraid-ui/stories/components/common/Loading.stories.ts (1)
3-3: Rename the Error import to avoid shadowing the global Error object.The import named
Errorshadows the built-in JavaScript Error constructor, which can cause confusion. Consider renaming it toErrorComponent.
🧹 Nitpick comments (2)
unraid-ui/stories/components/common/DropdownMenu.stories.ts (1)
127-147: Consider demonstrating the component's select event.While the onClick handlers on individual items are useful, the stories don't show how to listen to the component's
selectevent mentioned in the refactor. Consider adding a story that demonstrates event handling at the component level.// Add a new story showing component-level event handling +export const WithSelectEvent: Story = { + render: () => ({ + components: { DropdownMenu }, + setup() { + const handleSelect = (item) => { + console.log('Selected:', item); + }; + return { handleSelect }; + }, + template: ` + <DropdownMenu + trigger="Event Demo" + :items="[{ label: 'Option 1' }, { label: 'Option 2' }]" + @select="handleSelect" + /> + `, + }), +};unraid-ui/stories/components/form/Select.stories.ts (1)
44-349: Reduce code duplication across storiesAll stories follow nearly identical patterns with the same setup() function and template structure. This creates maintenance overhead and violates the DRY principle.
Consider extracting a common render function:
+const createSelectStory = (defaultValue: any = null) => (args: any) => ({ + components: { Select }, + setup() { + const value = ref(defaultValue); + return { args, value }; + }, + template: ` + <div class="space-y-4"> + <Select + v-model="value" + :items="args.items" + :placeholder="args.placeholder" + :disabled="args.disabled" + :multiple="args.multiple" + :valueKey="args.valueKey" + :labelKey="args.labelKey" + > + <template v-if="args.customSlot" #item="{ item }"> + <div class="flex items-center gap-2"> + <span class="w-3 h-3 rounded-full" :style="{ backgroundColor: item.color }"></span> + {{ item.label }} + </div> + </template> + </Select> + <p class="text-sm text-muted-foreground">Selected: {{ Array.isArray(value) ? value.join(', ') : value }}</p> + </div> + `, +}); export const SimpleArray: Story = { - render: (args) => ({ - components: { Select }, - setup() { - const value = ref(null); - return { args, value }; - }, - template: `...` - }), + render: createSelectStory(), args: { placeholder: 'Select a fruit', items: ['Apple', 'Banana', 'Orange', 'Grape', 'Mango'], }, };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (60)
CLAUDE.md(3 hunks)unraid-ui/components.json(1 hunks)unraid-ui/src/components/common/accordion/Accordion.vue(1 hunks)unraid-ui/src/components/common/accordion/index.ts(1 hunks)unraid-ui/src/components/common/dialog/Dialog.vue(1 hunks)unraid-ui/src/components/common/dialog/DialogClose.vue(0 hunks)unraid-ui/src/components/common/dialog/index.ts(1 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenu.vue(1 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuArrow.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuContent.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/index.ts(1 hunks)unraid-ui/src/components/form/select/Select.vue(1 hunks)unraid-ui/src/components/form/select/index.ts(1 hunks)unraid-ui/src/components/ui/accordion/AccordionItem.vue(1 hunks)unraid-ui/src/components/ui/accordion/AccordionRoot.vue(1 hunks)unraid-ui/src/components/ui/accordion/AccordionTrigger.vue(1 hunks)unraid-ui/src/components/ui/accordion/index.ts(1 hunks)unraid-ui/src/components/ui/dialog/DialogClose.vue(1 hunks)unraid-ui/src/components/ui/dialog/DialogContent.vue(1 hunks)unraid-ui/src/components/ui/dialog/DialogRoot.vue(1 hunks)unraid-ui/src/components/ui/dialog/DialogScrollContent.vue(2 hunks)unraid-ui/src/components/ui/dialog/index.ts(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuCheckboxItem.vue(2 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuContent.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuItem.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuLabel.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioGroup.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioItem.vue(2 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRoot.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSeparator.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSub.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubContent.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubTrigger.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuTrigger.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/index.ts(1 hunks)unraid-ui/src/components/ui/index.ts(1 hunks)unraid-ui/src/components/ui/select/SelectContent.vue(2 hunks)unraid-ui/src/components/ui/select/SelectGroup.vue(1 hunks)unraid-ui/src/components/ui/select/SelectItem.vue(2 hunks)unraid-ui/src/components/ui/select/SelectRoot.vue(1 hunks)unraid-ui/src/components/ui/select/SelectScrollDownButton.vue(1 hunks)unraid-ui/src/components/ui/select/SelectScrollUpButton.vue(1 hunks)unraid-ui/src/components/ui/select/SelectSeparator.vue(1 hunks)unraid-ui/src/components/ui/select/SelectTrigger.vue(1 hunks)unraid-ui/src/components/ui/select/index.ts(1 hunks)unraid-ui/src/forms/Select.vue(3 hunks)unraid-ui/src/index.ts(1 hunks)unraid-ui/stories/components/common/Accordion.stories.ts(1 hunks)unraid-ui/stories/components/common/Dialog.stories.ts(1 hunks)unraid-ui/stories/components/common/DropdownMenu.stories.ts(2 hunks)unraid-ui/stories/components/common/Loading.stories.ts(3 hunks)unraid-ui/stories/components/common/Sheet.stories.ts(1 hunks)unraid-ui/stories/components/form/Select.stories.ts(1 hunks)web/__test__/components/UserProfile.test.ts(1 hunks)web/__test__/mocks/ui-components.ts(1 hunks)web/components/ApiKey/ApiKeyCreate.vue(2 hunks)web/components/UserProfile.ce.vue(2 hunks)web/components/UserProfile/DropdownMenu.vue(0 hunks)web/components/UserProfile/DropdownTrigger.vue(1 hunks)web/helpers/globals.d.ts(1 hunks)
💤 Files with no reviewable changes (4)
- unraid-ui/src/components/common/dialog/DialogClose.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuArrow.vue
- web/components/UserProfile/DropdownMenu.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuContent.vue
✅ Files skipped from review due to trivial changes (3)
- unraid-ui/src/components/ui/select/SelectRoot.vue
- unraid-ui/src/components/ui/dialog/index.ts
- unraid-ui/src/components/ui/accordion/AccordionRoot.vue
🚧 Files skipped from review as they are similar to previous changes (49)
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSub.vue
- unraid-ui/src/components/ui/accordion/AccordionItem.vue
- unraid-ui/src/components/ui/accordion/AccordionTrigger.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubContent.vue
- unraid-ui/src/components/ui/dialog/DialogContent.vue
- unraid-ui/components.json
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioItem.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSeparator.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioGroup.vue
- unraid-ui/stories/components/common/Sheet.stories.ts
- unraid-ui/src/components/ui/select/SelectSeparator.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuTrigger.vue
- unraid-ui/src/components/ui/select/SelectGroup.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuLabel.vue
- unraid-ui/src/components/ui/select/SelectScrollDownButton.vue
- unraid-ui/src/index.ts
- unraid-ui/src/components/ui/select/SelectTrigger.vue
- web/test/components/UserProfile.test.ts
- unraid-ui/src/components/ui/select/SelectScrollUpButton.vue
- unraid-ui/src/components/ui/index.ts
- unraid-ui/src/components/ui/dialog/DialogRoot.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuItem.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRoot.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubTrigger.vue
- unraid-ui/src/components/ui/select/SelectItem.vue
- web/components/UserProfile.ce.vue
- web/helpers/globals.d.ts
- unraid-ui/src/components/ui/accordion/index.ts
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuCheckboxItem.vue
- web/components/UserProfile/DropdownTrigger.vue
- unraid-ui/src/components/ui/dialog/DialogClose.vue
- CLAUDE.md
- web/test/mocks/ui-components.ts
- unraid-ui/src/components/common/accordion/index.ts
- unraid-ui/src/components/ui/dialog/DialogScrollContent.vue
- unraid-ui/src/components/ui/select/SelectContent.vue
- unraid-ui/src/components/common/dialog/index.ts
- unraid-ui/src/forms/Select.vue
- unraid-ui/src/components/form/select/index.ts
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuContent.vue
- unraid-ui/src/components/ui/select/index.ts
- unraid-ui/src/components/common/dropdown-menu/index.ts
- web/components/ApiKey/ApiKeyCreate.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenu.vue
- unraid-ui/stories/components/common/Dialog.stories.ts
- unraid-ui/src/components/ui/dropdown-menu/index.ts
- unraid-ui/src/components/common/accordion/Accordion.vue
- unraid-ui/src/components/common/dialog/Dialog.vue
- unraid-ui/stories/components/common/Accordion.stories.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: zackspear
PR: unraid/api#1143
File: web/pages/webComponents.vue:28-29
Timestamp: 2025-02-18T18:38:24.981Z
Learning: In Vue components, when there's a repeating pattern of heading + content + divider, prefer using a wrapper component that takes the heading as a prop and content via slots, rather than duplicating the structure. This improves maintainability and reduces code duplication.
Learnt from: mdatelle
PR: unraid/api#1122
File: web/components/UserProfile/DropdownLaunchpad.vue:38-42
Timestamp: 2025-02-06T17:24:58.784Z
Learning: In the DropdownLaunchpad.vue component, the current implementation of conditional icon rendering using `h(BrandLoading, { variant: 'white' })` is a temporary fix to maintain build stability, with a proper refactoring being handled in a separate branch.
Learnt from: elibosley
PR: unraid/api#1408
File: web/components/ApiKey/PermissionCounter.vue:6-6
Timestamp: 2025-05-23T21:59:29.632Z
Learning: This codebase uses ESM (ECMAScript Modules) and requires .js extensions in import statements, even when importing from TypeScript files, as the imports refer to the compiled JavaScript output.
unraid-ui/stories/components/form/Select.stories.ts (5)
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-23T12:38:32.437Z
Learning: Do not rely on Nuxt's auto-imports in test files; explicitly import Vue reactivity utilities (`computed`, `ref`, `watchEffect`) in store files to avoid missing dependencies.
Learnt from: zackspear
PR: unraid/api#0
File: :0-0
Timestamp: 2025-03-27T23:33:13.215Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files would break the build by causing all web components to share a singular Pinia store instance. Each web component needs its own Pinia store instance to maintain proper isolation and encapsulation.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:38:09.623Z
Learning: Ensure Vue reactivity imports (computed, ref, watchEffect) are present in store files.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:38:09.623Z
Learning: Stub complex child components that aren't the focus of the test, and mock external dependencies and services in Vue component tests.
Learnt from: zackspear
PR: unraid/api#1143
File: web/pages/webComponents.vue:28-29
Timestamp: 2025-02-18T18:38:24.981Z
Learning: In Vue components, when there's a repeating pattern of heading + content + divider, prefer using a wrapper component that takes the heading as a prop and content via slots, rather than duplicating the structure. This improves maintainability and reduces code duplication.
unraid-ui/src/components/form/select/Select.vue (7)
Learnt from: zackspear
PR: unraid/api#1143
File: web/pages/webComponents.vue:28-29
Timestamp: 2025-02-18T18:38:24.981Z
Learning: In Vue components, when there's a repeating pattern of heading + content + divider, prefer using a wrapper component that takes the heading as a prop and content via slots, rather than duplicating the structure. This improves maintainability and reduces code duplication.
Learnt from: pujitm
PR: unraid/api#1211
File: plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Connect.page:0-0
Timestamp: 2025-03-14T19:14:10.408Z
Learning: *.ce.vue components in `web/components` get built as custom elements and prefixed with `unraid-`, making them available as HTML custom elements at runtime.
Learnt from: pujitm
PR: unraid/api#1211
File: plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Connect.page:0-0
Timestamp: 2025-03-14T19:14:10.408Z
Learning: *.ce.vue components in `web/components` get built as custom elements and prefixed with `unraid-`, making them available as HTML custom elements at runtime.
Learnt from: mdatelle
PR: unraid/api#1122
File: web/components/UserProfile/DropdownLaunchpad.vue:38-42
Timestamp: 2025-02-06T17:24:58.784Z
Learning: In the DropdownLaunchpad.vue component, the current implementation of conditional icon rendering using `h(BrandLoading, { variant: 'white' })` is a temporary fix to maintain build stability, with a proper refactoring being handled in a separate branch.
Learnt from: pujitm
PR: unraid/api#1219
File: api/src/unraid-api/main.ts:18-25
Timestamp: 2025-03-07T19:29:12.540Z
Learning: When handling type mismatches between dependencies in TypeScript (such as between NestJS and Fastify), prefer using `@ts-expect-error` with a detailed explanation comment instead of `as any` type assertions. This better documents the reason for the type bypass and makes the issue more visible for future maintenance.
Learnt from: pujitm
PR: unraid/api#1211
File: web/codegen.ts:14-14
Timestamp: 2025-03-12T13:35:43.900Z
Learning: The JSON scalar type in web/codegen.ts was temporarily changed from 'string' to 'any' for compatibility with JsonForms integration. This change facilitates the implementation of the Connect settings web component.
Learnt from: elibosley
PR: unraid/api#1181
File: web/store/theme.ts:0-0
Timestamp: 2025-02-24T14:51:21.328Z
Learning: In the Unraid API project's theme system, exact TypeScript type definitions are preferred over index signatures for theme variables to ensure better type safety.
unraid-ui/stories/components/common/Loading.stories.ts (14)
Learnt from: elibosley
PR: unraid/api#974
File: web/components/Loading/Error.vue:1-3
Timestamp: 2024-12-06T17:38:40.999Z
Learning: In Nuxt.js projects, components are automatically imported, so explicit import statements for components like `LoadingSpinner` in `web/components/Loading/Error.vue` are not necessary.
Learnt from: pujitm
PR: unraid/api#974
File: web/components/Loading/Error.vue:50-50
Timestamp: 2024-12-06T17:34:16.133Z
Learning: In this project, the `Button` component from `~/components/shadcn/Button.vue` is autoloaded and does not need to be imported manually in components like `web/components/Loading/Error.vue`.
Learnt from: mdatelle
PR: unraid/api#1122
File: web/components/UserProfile/DropdownLaunchpad.vue:38-42
Timestamp: 2025-02-06T17:24:58.784Z
Learning: In the DropdownLaunchpad.vue component, the current implementation of conditional icon rendering using `h(BrandLoading, { variant: 'white' })` is a temporary fix to maintain build stability, with a proper refactoring being handled in a separate branch.
Learnt from: elibosley
PR: unraid/api#1308
File: unraid-ui/src/components/common/loading/Error.vue:2-2
Timestamp: 2025-04-02T21:21:29.168Z
Learning: Components in the unraid-ui folder require explicit imports and are not autoloaded, unlike other parts of the project that may use Nuxt.js autoloading features.
Learnt from: mdatelle
PR: unraid/api#1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
Learnt from: elibosley
PR: unraid/api#972
File: web/components/ColorSwitcher.ce.vue:1-2
Timestamp: 2024-12-17T13:55:42.068Z
Learning: In this Nuxt.js project, components used in templates are automatically imported by Nuxt, so explicit import statements for components are unnecessary.
Learnt from: pujitm
PR: unraid/api#1075
File: unraid-ui/src/register.ts:15-34
Timestamp: 2025-01-30T19:38:02.478Z
Learning: In the web components registration process for unraid-ui, use a soft-fail approach (logging warnings/errors) instead of throwing errors, to ensure other components can still register successfully even if one component fails.
Learnt from: elibosley
PR: unraid/api#1408
File: web/components/ApiKey/PermissionCounter.vue:6-6
Timestamp: 2025-05-23T21:59:29.632Z
Learning: This codebase uses ESM (ECMAScript Modules) and requires .js extensions in import statements, even when importing from TypeScript files, as the imports refer to the compiled JavaScript output.
Learnt from: pujitm
PR: unraid/api#1219
File: api/src/unraid-api/main.ts:18-25
Timestamp: 2025-03-07T19:29:12.540Z
Learning: When handling type mismatches between dependencies in TypeScript (such as between NestJS and Fastify), prefer using `@ts-expect-error` with a detailed explanation comment instead of `as any` type assertions. This better documents the reason for the type bypass and makes the issue more visible for future maintenance.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-23T12:38:32.437Z
Learning: Do not rely on Nuxt's auto-imports in test files; explicitly import Vue reactivity utilities (`computed`, `ref`, `watchEffect`) in store files to avoid missing dependencies.
Learnt from: pujitm
PR: unraid/api#975
File: web/components/Notifications/TabList.vue:1-24
Timestamp: 2024-12-09T15:47:29.325Z
Learning: In our Nuxt setup using Vue.js, components defined within the codebase are autoloaded and do not require explicit import statements in the script section. For example, components like `TabsList` and `TabsTrigger` used in `web/components/Notifications/TabList.vue` are automatically available without imports.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-23T12:38:09.623Z
Learning: Stub complex child components that aren't the focus of the test, and mock external dependencies and services in Vue component tests.
Learnt from: pujitm
PR: unraid/api#1417
File: web/components/ConnectSettings/ConnectSettings.ce.vue:11-18
Timestamp: 2025-06-13T17:14:21.739Z
Learning: The project’s build tooling auto-imports common Vue/Pinia helpers such as `storeToRefs`, so explicit import statements for them are not required.
Learnt from: zackspear
PR: unraid/api#1143
File: web/pages/webComponents.vue:28-29
Timestamp: 2025-02-18T18:38:24.981Z
Learning: In Vue components, when there's a repeating pattern of heading + content + divider, prefer using a wrapper component that takes the heading as a prop and content via slots, rather than duplicating the structure. This improves maintainability and reduces code duplication.
🪛 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)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Build API
- GitHub Check: Build Web App
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (14)
unraid-ui/stories/components/common/Loading.stories.ts (2)
20-22: Good cleanup of unused parameters.Removing the unused
argsparameter andv-bind="args"directive simplifies the render function since the LoadingBar story doesn't use any arguments.
28-30: Good cleanup of unused parameters.Similar to LoadingBar, removing the unused
argsparameter andv-bind="args"directive appropriately simplifies the LoadingSpinner story.unraid-ui/stories/components/common/DropdownMenu.stories.ts (7)
1-1: Import looks clean and properly structured.The import follows the expected path structure for the refactored component.
9-16: ArgTypes configuration is well-defined.The select controls for
alignandsideprops provide good Storybook interactivity with all the expected positioning options.
25-45: BasicUsage story demonstrates the declarative API well.The story effectively shows the simplified usage pattern with the
itemsprop structure including separators.
48-64: UserAccountMenu story shows proper item type usage.Good demonstration of
type: 'label'andtype: 'separator'along with disabled states. This helps developers understand the item structure.
207-228: CustomTrigger story effectively demonstrates slot usage.The template slot syntax is correctly implemented and shows how developers can provide custom trigger elements.
231-246: Edge case stories provide good coverage.Both
LongMenuWithScrollandEmptyStatestories test important scenarios that developers will encounter. The dynamic array generation for the scroll test is particularly useful.
150-204: Alignment and positioning stories are comprehensive.These stories effectively demonstrate all the available
alignandsideprop values with clear visual examples. The layout styling helps showcase the positioning differences.unraid-ui/src/components/form/select/Select.vue (5)
2-11: Well-structured import organizationThe imports are clearly organized and all necessary UI components are properly imported from the local UI library.
14-53: Comprehensive type system designThe type definitions provide excellent type safety with proper union types for different item structures (primitive values, structured items, labels, separators) and a flexible props interface supporting various use cases.
66-97: Robust type guards and utility functionsThe type guard functions properly differentiate between item types, and the utility functions correctly handle value/label extraction with proper fallback logic using property existence checks rather than truthiness.
103-150: Efficient computed properties for item processingThe computed properties efficiently process items into groups, flatten arrays, filter renderable items, and handle multiple selection display. The logic correctly handles both flat and grouped item arrays.
158-201: Clean template with proper slot compositionThe template effectively composes UI primitives with proper slot usage, conditional rendering, and correct prop binding. The use of non-null assertion on
getItemValue(item)!is appropriate since type guards ensure only selectable items reach this point.
| if (item.onClick) { | ||
| item.onClick(); | ||
| } |
There was a problem hiding this comment.
nifty one liner for this: item.onClick?.();
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining
| return delegated; | ||
| }); | ||
| const delegatedProps = reactiveOmit(props, 'class'); |
There was a problem hiding this comment.
did not know about this api, this is sick!
| <SelectPortal :force-mount="forceMount" :to="teleportTarget"> | ||
| <SelectPortal> |
| <button | ||
| class="group text-18px border-0 relative flex flex-row justify-end items-center h-full gap-x-8px opacity-100 hover:opacity-75 focus:opacity-75 transition-opacity text-header-text-primary" | ||
| class="group text-18px border-0 relative flex flex-row justify-end items-center h-full gap-x-8px opacity-100 hover:opacity-75 transition-opacity text-header-text-primary" |
There was a problem hiding this comment.
i'm 99% sure this is the right button (button vs Button), but just making sure
There was a problem hiding this comment.
I think this should be an uppercase component sourced from our own button in this UI lib?
There was a problem hiding this comment.
This component was already built like this I just removed that focus. This is for the user account dropdown so doesn't need to be the unraid-ui Button.
There was a problem hiding this comment.
button vs Button is something we may continue having confusion on...and maybe we should consider a solution that will help us avoid this.
I think this should be an uppercase component sourced from our own button in this UI lib?
Negative. It's the trigger for the UPC Dropdown.
This is a semantically used regular HTML <button> element and should stay as such. Aka essentially an unstyled / wrapper element.
|
|
||
| <template> | ||
| <SelectPortal :force-mount="forceMount" :to="teleportTarget"> | ||
| <SelectPortal> |
There was a problem hiding this comment.
The removal of the portal here is 100% going to cause a regression.
There was a problem hiding this comment.
This should probably be moved to the actual component.
|
|
||
| <template> | ||
| <DialogPortal :to="teleportTarget"> | ||
| <DialogPortal> |
There was a problem hiding this comment.
The removal of the teleport target here is also a regression.
|
Build is failing also:
|
| </SelectTrigger> | ||
| <!-- The content includes the selectable options --> | ||
|
|
||
| <SelectContent :to="teleportTarget"> |
There was a problem hiding this comment.
This has the teleport still, which it definitely should!
| </UpcDropdownMenu> | ||
| <template #content> | ||
| <div class="w-[350px]"> | ||
| <UpcDropdownContent :t="t" /> |
I reinstalled the latest from shadcn for these new components so just need to add it back. |
|
|
||
| <template> | ||
| <DialogPortal :to="teleportTarget ?? to"> | ||
| <DialogPortal> |
There was a problem hiding this comment.
Make sure the teleportTarget removal is not a regression
There was a problem hiding this comment.
See my comment above some of these shadcn components were reinstalled to latest versions so just need to add it back if the teleport ends up still being needed. I'd like to move it to the custom component if still needed instead of the shadcn primitives.
web/components/UserProfile.ce.vue
Outdated
| </template> | ||
| </UpcDropdownMenu> | ||
| <template #content> | ||
| <div class="w-[350px]"> |
There was a problem hiding this comment.
w-[350px] will need a refactor to work better responsively.
Please try this out on a 7.2.0-beta.x.x test release for a better idea of what you'll need to do.
Probably will need a min-width for "non-mobile" sizes in addition to a max width.
I don't know if something like max-w-[350px] sm:min-w-[350px] or something would do the trick.
| <button | ||
| class="group text-18px border-0 relative flex flex-row justify-end items-center h-full gap-x-8px opacity-100 hover:opacity-75 focus:opacity-75 transition-opacity text-header-text-primary" | ||
| class="group text-18px border-0 relative flex flex-row justify-end items-center h-full gap-x-8px opacity-100 hover:opacity-75 transition-opacity text-header-text-primary" |
There was a problem hiding this comment.
button vs Button is something we may continue having confusion on...and maybe we should consider a solution that will help us avoid this.
I think this should be an uppercase component sourced from our own button in this UI lib?
Negative. It's the trigger for the UPC Dropdown.
This is a semantically used regular HTML <button> element and should stay as such. Aka essentially an unstyled / wrapper element.
mdatelle
force-pushed
the
feat/build-out-docker-components
branch
from
7f118ec to
328995d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
unraid-ui/stories/components/common/Loading.stories.ts (1)
3-3: Avoid shadowing the globalErrorobject.Importing the component as
Errorshadows the built-in Error constructor, which can be confusing. Consider renaming the import to something likeErrorComponentand update references insubcomponentsand the story.
🧹 Nitpick comments (4)
unraid-ui/src/components/form/select/Select.vue (4)
16-16: Consider narrowing the AcceptableValue type.The
Record<string, unknown>in AcceptableValue seems overly permissive for a select component's model value. This could allow any object to be passed as a value, potentially causing runtime issues.Consider restricting this to more specific types:
-type AcceptableValue = SelectValueType | SelectValueType[] | Record<string, unknown> | null; +type AcceptableValue = SelectValueType | SelectValueType[] | null;
44-44: Clarify the grouped items structure.The
SelectItemType[][]type for grouped items could benefit from better documentation or a more explicit type alias to clarify the expected structure.Consider adding a type alias for clarity:
+type SelectItemGroup = SelectItemType[]; + export interface SelectProps { modelValue?: AcceptableValue; - items?: SelectItemType[] | SelectItemType[][]; + items?: SelectItemType[] | SelectItemGroup[];
132-150: Optimize multiple value display computation.The
multipleValueDisplaycomputed performs expensive operations on every re-render. Consider memoizing the item lookup for better performance.const multipleValueDisplay = computed(() => { if (!isMultipleSelection.value || !Array.isArray(props.modelValue)) return ''; const values = props.modelValue as SelectValueType[]; + // Create a lookup map for better performance + const itemLookup = new Map( + renderableItems.value.map(item => [getItemValue(item), item]) + ); + const displayLabels = values.map((value) => { - const item = renderableItems.value.find((item) => { - const itemValue = getItemValue(item); - return itemValue === value; - }); + const item = itemLookup.get(value); return item ? getItemLabel(item) : String(value); });
145-149: Extract magic numbers to constants.The hardcoded values
2for display limit should be configurable or at least extracted as constants.+const MULTIPLE_DISPLAY_LIMIT = 2; + const multipleValueDisplay = computed(() => { // ... existing code ... - if (displayLabels.length <= 2) { + if (displayLabels.length <= MULTIPLE_DISPLAY_LIMIT) { return displayLabels.join(', '); } else { - return `${displayLabels[0]}, ${displayLabels[1]} +${displayLabels.length - 2} more`; + return `${displayLabels[0]}, ${displayLabels[1]} +${displayLabels.length - MULTIPLE_DISPLAY_LIMIT} more`; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (64)
CLAUDE.md(3 hunks)api/package.json(1 hunks)packages/unraid-api-plugin-connect/package.json(2 hunks)packages/unraid-shared/package.json(2 hunks)unraid-ui/components.json(1 hunks)unraid-ui/package.json(1 hunks)unraid-ui/src/components/common/accordion/Accordion.vue(1 hunks)unraid-ui/src/components/common/accordion/index.ts(1 hunks)unraid-ui/src/components/common/dialog/Dialog.vue(1 hunks)unraid-ui/src/components/common/dialog/DialogClose.vue(0 hunks)unraid-ui/src/components/common/dialog/index.ts(1 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenu.vue(1 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuArrow.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/index.ts(1 hunks)unraid-ui/src/components/form/select/Select.vue(1 hunks)unraid-ui/src/components/form/select/index.ts(1 hunks)unraid-ui/src/components/ui/accordion/AccordionItem.vue(1 hunks)unraid-ui/src/components/ui/accordion/AccordionRoot.vue(1 hunks)unraid-ui/src/components/ui/accordion/AccordionTrigger.vue(1 hunks)unraid-ui/src/components/ui/accordion/index.ts(1 hunks)unraid-ui/src/components/ui/dialog/DialogClose.vue(1 hunks)unraid-ui/src/components/ui/dialog/DialogContent.vue(1 hunks)unraid-ui/src/components/ui/dialog/DialogRoot.vue(1 hunks)unraid-ui/src/components/ui/dialog/DialogScrollContent.vue(1 hunks)unraid-ui/src/components/ui/dialog/index.ts(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuCheckboxItem.vue(2 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuContent.vue(2 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuItem.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuLabel.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioGroup.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioItem.vue(2 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRoot.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSeparator.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSub.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubContent.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubTrigger.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuTrigger.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/index.ts(1 hunks)unraid-ui/src/components/ui/index.ts(1 hunks)unraid-ui/src/components/ui/select/SelectContent.vue(2 hunks)unraid-ui/src/components/ui/select/SelectGroup.vue(1 hunks)unraid-ui/src/components/ui/select/SelectItem.vue(2 hunks)unraid-ui/src/components/ui/select/SelectRoot.vue(1 hunks)unraid-ui/src/components/ui/select/SelectScrollDownButton.vue(1 hunks)unraid-ui/src/components/ui/select/SelectScrollUpButton.vue(1 hunks)unraid-ui/src/components/ui/select/SelectSeparator.vue(1 hunks)unraid-ui/src/components/ui/select/SelectTrigger.vue(1 hunks)unraid-ui/src/components/ui/select/index.ts(1 hunks)unraid-ui/src/forms/Select.vue(3 hunks)unraid-ui/src/index.ts(1 hunks)unraid-ui/stories/components/common/Accordion.stories.ts(1 hunks)unraid-ui/stories/components/common/Dialog.stories.ts(1 hunks)unraid-ui/stories/components/common/DropdownMenu.stories.ts(2 hunks)unraid-ui/stories/components/common/Loading.stories.ts(3 hunks)unraid-ui/stories/components/common/Sheet.stories.ts(3 hunks)unraid-ui/stories/components/form/Select.stories.ts(1 hunks)web/__test__/components/UserProfile.test.ts(1 hunks)web/__test__/mocks/ui-components.ts(1 hunks)web/components/ApiKey/ApiKeyCreate.vue(2 hunks)web/components/UserProfile.ce.vue(2 hunks)web/components/UserProfile/DropdownMenu.vue(0 hunks)web/components/UserProfile/DropdownTrigger.vue(1 hunks)web/helpers/globals.d.ts(1 hunks)web/package.json(1 hunks)
💤 Files with no reviewable changes (3)
- unraid-ui/src/components/common/dialog/DialogClose.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuArrow.vue
- web/components/UserProfile/DropdownMenu.vue
✅ Files skipped from review due to trivial changes (3)
- api/package.json
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuTrigger.vue
- unraid-ui/package.json
🚧 Files skipped from review as they are similar to previous changes (54)
- packages/unraid-api-plugin-connect/package.json
- unraid-ui/src/components/ui/accordion/AccordionItem.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSub.vue
- web/test/components/UserProfile.test.ts
- unraid-ui/src/forms/Select.vue
- unraid-ui/src/components/ui/dialog/DialogContent.vue
- unraid-ui/components.json
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioItem.vue
- unraid-ui/src/components/ui/index.ts
- unraid-ui/src/components/ui/dialog/DialogRoot.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubContent.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuCheckboxItem.vue
- packages/unraid-shared/package.json
- unraid-ui/src/components/ui/dialog/DialogScrollContent.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuLabel.vue
- unraid-ui/src/components/ui/select/SelectTrigger.vue
- unraid-ui/src/components/ui/dialog/DialogClose.vue
- unraid-ui/src/components/ui/accordion/AccordionTrigger.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioGroup.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuItem.vue
- CLAUDE.md
- unraid-ui/src/index.ts
- unraid-ui/src/components/ui/select/SelectSeparator.vue
- web/package.json
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubTrigger.vue
- unraid-ui/src/components/ui/select/SelectGroup.vue
- unraid-ui/src/components/ui/select/SelectScrollUpButton.vue
- unraid-ui/src/components/ui/select/SelectContent.vue
- web/helpers/globals.d.ts
- unraid-ui/src/components/ui/select/SelectItem.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuContent.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSeparator.vue
- unraid-ui/src/components/ui/accordion/index.ts
- unraid-ui/src/components/ui/select/SelectScrollDownButton.vue
- web/components/UserProfile/DropdownTrigger.vue
- web/components/UserProfile.ce.vue
- unraid-ui/src/components/ui/select/index.ts
- unraid-ui/src/components/common/dialog/index.ts
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRoot.vue
- web/test/mocks/ui-components.ts
- unraid-ui/src/components/common/accordion/index.ts
- unraid-ui/src/components/ui/select/SelectRoot.vue
- unraid-ui/src/components/ui/accordion/AccordionRoot.vue
- unraid-ui/stories/components/common/Sheet.stories.ts
- unraid-ui/src/components/ui/dialog/index.ts
- unraid-ui/src/components/form/select/index.ts
- web/components/ApiKey/ApiKeyCreate.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenu.vue
- unraid-ui/src/components/common/dropdown-menu/index.ts
- unraid-ui/stories/components/common/Dialog.stories.ts
- unraid-ui/src/components/common/accordion/Accordion.vue
- unraid-ui/src/components/ui/dropdown-menu/index.ts
- unraid-ui/src/components/common/dialog/Dialog.vue
- unraid-ui/stories/components/form/Select.stories.ts
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: TypeScript imports use .js extensions for ESM compatibility
**/*.{ts,tsx}: TypeScript imports use .js extensions for ESM compatibility
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
unraid-ui/stories/components/common/Accordion.stories.tsunraid-ui/stories/components/common/DropdownMenu.stories.tsunraid-ui/stories/components/common/Loading.stories.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: mdatelle
PR: unraid/api#1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
Learnt from: elibosley
PR: unraid/api#1211
File: unraid-ui/src/components/form/number/NumberFieldInput.vue:1-21
Timestamp: 2025-03-13T16:17:21.897Z
Learning: The unraid-ui is a UI library being published externally, where wrapping third-party components (like those from reka-ui) is an intentional design choice to maintain a consistent interface, standardize styling, and control the exposed API.
Learnt from: pujitm
PR: unraid/api#1211
File: unraid-ui/package.json:43-43
Timestamp: 2025-03-14T19:18:23.324Z
Learning: The @internationalized/number package is a dependency of reka-ui, which is used for number formatting in the Unraid UI components.
Learnt from: elibosley
PR: unraid/api#1308
File: unraid-ui/src/components/common/loading/Error.vue:2-2
Timestamp: 2025-04-02T21:21:29.168Z
Learning: Components in the unraid-ui folder require explicit imports and are not autoloaded, unlike other parts of the project that may use Nuxt.js autoloading features.
Learnt from: elibosley
PR: unraid/api#1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:24-27
Timestamp: 2025-01-31T22:01:22.708Z
Learning: The Unraid UI uses a modern notification system with a custom `unraid-toaster` component replacing the legacy jGrowl notifications. The system is backed by a comprehensive GraphQL API with real-time subscription support for notification updates.
Learnt from: elibosley
PR: unraid/api#1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch removes the old jGrowl notification system and is complemented by the unraid-toaster component implementation. The new system is added through the DefaultPageLayout modification which inserts the toaster component with proper position configuration based on user preferences.
Learnt from: elibosley
PR: unraid/api#1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch is used to remove the old jGrowl notification system from Unraid pages, as notifications are handled by a new system implemented on a different page.
Learnt from: elibosley
PR: unraid/api#1181
File: web/store/theme.ts:0-0
Timestamp: 2025-02-24T14:51:21.328Z
Learning: In the Unraid API project's theme system, exact TypeScript type definitions are preferred over index signatures for theme variables to ensure better type safety.
Learnt from: zackspear
PR: unraid/api#1079
File: web/scripts/deploy-dev.sh:51-54
Timestamp: 2025-01-29T00:59:26.633Z
Learning: For the Unraid web components deployment process, JS file validation isn't required in auth-request.php updates since the files come from the controlled build pipeline where we are the source.
Learnt from: pujitm
PR: unraid/api#1075
File: unraid-ui/src/register.ts:15-34
Timestamp: 2025-01-30T19:38:02.478Z
Learning: In the web components registration process for unraid-ui, use a soft-fail approach (logging warnings/errors) instead of throwing errors, to ensure other components can still register successfully even if one component fails.
unraid-ui/stories/components/common/Accordion.stories.ts (1)
Learnt from: zackspear
PR: unraid/api#1143
File: web/pages/webComponents.vue:28-29
Timestamp: 2025-02-18T18:38:24.981Z
Learning: In Vue components, when there's a repeating pattern of heading + content + divider, prefer using a wrapper component that takes the heading as a prop and content via slots, rather than duplicating the structure. This improves maintainability and reduces code duplication.
unraid-ui/src/components/form/select/Select.vue (7)
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Applies to web/store/**/*.{ts,js} : Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Learnt from: zackspear
PR: unraid/api#1143
File: web/pages/webComponents.vue:28-29
Timestamp: 2025-02-18T18:38:24.981Z
Learning: In Vue components, when there's a repeating pattern of heading + content + divider, prefer using a wrapper component that takes the heading as a prop and content via slots, rather than duplicating the structure. This improves maintainability and reduces code duplication.
Learnt from: mdatelle
PR: unraid/api#1122
File: web/components/UserProfile/DropdownLaunchpad.vue:38-42
Timestamp: 2025-02-06T17:24:58.784Z
Learning: In the DropdownLaunchpad.vue component, the current implementation of conditional icon rendering using `h(BrandLoading, { variant: 'white' })` is a temporary fix to maintain build stability, with a proper refactoring being handled in a separate branch.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/*.test.ts : Check for expected prop handling and event emissions in component tests
Learnt from: pujitm
PR: unraid/api#1219
File: api/src/unraid-api/main.ts:18-25
Timestamp: 2025-03-07T19:29:12.540Z
Learning: When handling type mismatches between dependencies in TypeScript (such as between NestJS and Fastify), prefer using `@ts-expect-error` with a detailed explanation comment instead of `as any` type assertions. This better documents the reason for the type bypass and makes the issue more visible for future maintenance.
Learnt from: pujitm
PR: unraid/api#1211
File: web/codegen.ts:14-14
Timestamp: 2025-03-12T13:35:43.900Z
Learning: The JSON scalar type in web/codegen.ts was temporarily changed from 'string' to 'any' for compatibility with JsonForms integration. This change facilitates the implementation of the Connect settings web component.
Learnt from: elibosley
PR: unraid/api#1181
File: web/store/theme.ts:0-0
Timestamp: 2025-02-24T14:51:21.328Z
Learning: In the Unraid API project's theme system, exact TypeScript type definitions are preferred over index signatures for theme variables to ensure better type safety.
unraid-ui/stories/components/common/Loading.stories.ts (18)
Learnt from: elibosley
PR: unraid/api#974
File: web/components/Loading/Error.vue:1-3
Timestamp: 2024-12-06T17:38:40.999Z
Learning: In Nuxt.js projects, components are automatically imported, so explicit import statements for components like `LoadingSpinner` in `web/components/Loading/Error.vue` are not necessary.
Learnt from: pujitm
PR: unraid/api#974
File: web/components/Loading/Error.vue:50-50
Timestamp: 2024-12-06T17:34:16.133Z
Learning: In this project, the `Button` component from `~/components/shadcn/Button.vue` is autoloaded and does not need to be imported manually in components like `web/components/Loading/Error.vue`.
Learnt from: mdatelle
PR: unraid/api#1122
File: web/components/UserProfile/DropdownLaunchpad.vue:38-42
Timestamp: 2025-02-06T17:24:58.784Z
Learning: In the DropdownLaunchpad.vue component, the current implementation of conditional icon rendering using `h(BrandLoading, { variant: 'white' })` is a temporary fix to maintain build stability, with a proper refactoring being handled in a separate branch.
Learnt from: elibosley
PR: unraid/api#1308
File: unraid-ui/src/components/common/loading/Error.vue:2-2
Timestamp: 2025-04-02T21:21:29.168Z
Learning: Components in the unraid-ui folder require explicit imports and are not autoloaded, unlike other parts of the project that may use Nuxt.js autoloading features.
Learnt from: mdatelle
PR: unraid/api#1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Applies to web/store/**/*.{ts,js} : Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Learnt from: elibosley
PR: unraid/api#972
File: web/components/ColorSwitcher.ce.vue:1-2
Timestamp: 2024-12-17T13:55:42.068Z
Learning: In this Nuxt.js project, components used in templates are automatically imported by Nuxt, so explicit import statements for components are unnecessary.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/__test__/store/**/*.ts : Verify state changes by updating the store in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/__test__/store/**/*.ts : Verify proper error handling in store tests
Learnt from: pujitm
PR: unraid/api#1075
File: unraid-ui/src/register.ts:15-34
Timestamp: 2025-01-30T19:38:02.478Z
Learning: In the web components registration process for unraid-ui, use a soft-fail approach (logging warnings/errors) instead of throwing errors, to ensure other components can still register successfully even if one component fails.
Learnt from: elibosley
PR: unraid/api#1408
File: web/components/ApiKey/PermissionCounter.vue:6-6
Timestamp: 2025-05-23T21:59:29.632Z
Learning: This codebase uses ESM (ECMAScript Modules) and requires .js extensions in import statements, even when importing from TypeScript files, as the imports refer to the compiled JavaScript output.
Learnt from: pujitm
PR: unraid/api#1219
File: api/src/unraid-api/main.ts:18-25
Timestamp: 2025-03-07T19:29:12.540Z
Learning: When handling type mismatches between dependencies in TypeScript (such as between NestJS and Fastify), prefer using `@ts-expect-error` with a detailed explanation comment instead of `as any` type assertions. This better documents the reason for the type bypass and makes the issue more visible for future maintenance.
Learnt from: pujitm
PR: unraid/api#1075
File: web/components/Notifications/Sidebar.vue:41-58
Timestamp: 2025-01-31T14:42:06.752Z
Learning: When handling errors in code that uses toast notifications, avoid using toast in the error handler itself since toast functionality might be the source of the error. Use console.error or other reliable logging mechanisms instead.
Learnt from: pujitm
PR: unraid/api#975
File: web/components/Notifications/TabList.vue:1-24
Timestamp: 2024-12-09T15:47:29.325Z
Learning: In our Nuxt setup using Vue.js, components defined within the codebase are autoloaded and do not require explicit import statements in the script section. For example, components like `TabsList` and `TabsTrigger` used in `web/components/Notifications/TabList.vue` are automatically available without imports.
Learnt from: pujitm
PR: unraid/api#1417
File: web/components/ConnectSettings/ConnectSettings.ce.vue:11-18
Timestamp: 2025-06-13T17:14:21.739Z
Learning: The project’s build tooling auto-imports common Vue/Pinia helpers such as `storeToRefs`, so explicit import statements for them are not required.
Learnt from: zackspear
PR: unraid/api#1143
File: web/pages/webComponents.vue:28-29
Timestamp: 2025-02-18T18:38:24.981Z
Learning: In Vue components, when there's a repeating pattern of heading + content + divider, prefer using a wrapper component that takes the heading as a prop and content via slots, rather than duplicating the structure. This improves maintainability and reduces code duplication.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/*.test.ts : Use `mount` from Vue Test Utils for component testing
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/*.test.ts : Stub complex child components that aren't the focus of the test
🪛 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)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Cloudflare Pages
- GitHub Check: Build API
- GitHub Check: Build Web App
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (22)
unraid-ui/stories/components/common/Loading.stories.ts (3)
1-4: LGTM on quote consistency.The conversion to single quotes aligns with the project's coding standards.
20-23: Good cleanup of unused parameters.Removing the unused
argsparameter andv-bind="args"directive simplifies the render function while maintaining functionality.
28-31: Good cleanup of unused parameters.Removing the unused
argsparameter andv-bind="args"directive simplifies the render function while maintaining functionality.unraid-ui/stories/components/common/DropdownMenu.stories.ts (6)
1-18: Excellent story configuration.The meta configuration properly defines the component, controls, and options for testing different dropdown menu variations.
24-45: Good basic usage example.The BasicUsage story demonstrates the component's core functionality with a clear, simple example that shows the essential props and structure.
47-124: Comprehensive usage scenarios.The UserAccountMenu, FileOperationsMenu, ContextMenu, and SettingsMenu stories provide excellent real-world examples that demonstrate the component's versatility and practical applications.
126-147: Great demonstration of click handlers.The WithClickHandlers story effectively shows how to handle item clicks and properly demonstrates disabled state behavior.
149-204: Excellent positioning demonstrations.The AlignmentVariations and SidePositions stories thoroughly test the component's positioning capabilities, which is crucial for dropdown menu functionality.
206-246: Good custom trigger example.The CustomTrigger story demonstrates slot usage effectively, showing how developers can provide custom trigger elements.
unraid-ui/stories/components/common/Accordion.stories.ts (8)
1-21: Well-structured story configuration.The meta configuration properly defines the component with appropriate controls for testing different accordion modes and behaviors.
23-60: Excellent basic usage example.The BasicUsage story provides a clear demonstration of the component's core functionality with well-structured sample data.
62-115: Great demonstration of slot customization.The ItemsWithCustomSlots story effectively shows how to customize both trigger and content areas, demonstrating the component's flexibility.
117-179: Excellent direct composition pattern.The DirectComposition story demonstrates manual accordion construction using subcomponents, showing the component's versatility and providing a clear example of direct usage.
181-217: Good multiple mode demonstration.The MultipleMode story effectively shows how multiple accordion sections can be open simultaneously and demonstrates default value handling.
219-261: Excellent mixed usage pattern.The MixedUsage story demonstrates how direct composition and props-driven items can be combined, showing the component's flexibility.
263-326: Impressive dynamic component demonstration.The DynamicComponents story showcases advanced usage with different Vue components rendered dynamically based on item properties, demonstrating the component's extensibility.
328-365: Good disabled state coverage.The DisabledItems story properly demonstrates how disabled items behave, which is important for accessibility and user experience.
unraid-ui/src/components/form/select/Select.vue (5)
128-130: Simplify multiple selection logic.The
isMultipleSelectioncomputed property has redundant checks that could be simplified.const isMultipleSelection = computed(() => { - return props.multiple && Array.isArray(props.modelValue) && props.modelValue.length > 0; + return props.multiple && Array.isArray(props.modelValue) && props.modelValue.length > 0; });Actually, the current logic is correct. The redundant check I initially saw doesn't exist - the logic properly checks for multiple mode, array type, and non-empty array.
189-189: Non-null assertion is safe here.The non-null assertion on
getItemValue(item)!is appropriately used since the template filters ensure only items with values reach this point.
158-201: Well-structured template with good slot flexibility.The template provides excellent flexibility with multiple slot options (
default,content-top,content-bottom,item) while maintaining clean structure and proper component composition.
179-198: Verify Select.vue performance with nested loopsNo existing virtualization or pagination logic was detected in unraid-ui/src/components/form/select/Select.vue. The nested v-for over groupedOrderedItems could lead to slow renders on large datasets. Please test this component with realistic (large) item sets and consider:
- Introducing list virtualization/windowing (e.g., vue-virtual-scroller)
- Adding pagination or “load more” patterns
Location to review:
• unraid-ui/src/components/form/select/Select.vue (around lines 179–198)
99-101: Add null safety to grouped items check.The
isGroupedItemsfunction doesn't handle the case whereitemsmight be empty or contain null/undefined elements.function isGroupedItems(items: SelectItemType[] | SelectItemType[][]): items is SelectItemType[][] { - return Array.isArray(items) && items.length > 0 && Array.isArray(items[0]); + return Array.isArray(items) && items.length > 0 && items[0] != null && Array.isArray(items[0]); }Likely an incorrect or invalid review comment.
…eate to use new Dialog component
mdatelle
force-pushed
the
feat/build-out-docker-components
branch
from
6dbc799 to
b704b4a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
unraid-ui/stories/components/common/Loading.stories.ts (1)
3-3: Rename the Error import to avoid shadowing the global Error object.The import named
Errorshadows the global JavaScriptErrorconstructor, which can cause confusion and potential issues.Apply this diff to fix the shadowing issue:
-import Error from '../../../src/components/common/loading/Error.vue'; +import ErrorComponent from '../../../src/components/common/loading/Error.vue';Then update the references in the file:
- subcomponents: { Bar, Spinner, Error }, + subcomponents: { Bar, Spinner, ErrorComponent },-type ErrorStory = StoryObj<typeof Error>; +type ErrorStory = StoryObj<typeof ErrorComponent>;- components: { Error }, + components: { ErrorComponent },- <Error v-bind="args"> + <ErrorComponent v-bind="args">- </Error> + </ErrorComponent>
🧹 Nitpick comments (3)
unraid-ui/src/components/form/select/Select.vue (3)
113-115: Consider performance optimization for large datasets.The renderableItems computed property filters the entire flattened array on every access, which could be inefficient for large datasets.
-const renderableItems = computed(() => { - return flatItems.value.filter((item) => !isLabelItem(item) && !isSeparatorItem(item)); -}); +const renderableItems = computed(() => { + const cache = new Map(); + return flatItems.value.filter((item) => { + if (cache.has(item)) return cache.get(item); + const isRenderable = !isLabelItem(item) && !isSeparatorItem(item); + cache.set(item, isRenderable); + return isRenderable; + }); +});
117-126: Optimize the grouped items computation.The nested mapping operations could be expensive for large datasets and frequent updates.
Consider memoizing the computation or using a more efficient data structure:
-const groupedOrderedItems = computed(() => { - return itemGroups.value.map((group, groupIndex) => ({ - groupIndex, - items: group.map((item, index) => ({ - item, - index, - type: isLabelItem(item) ? 'label' : isSeparatorItem(item) ? 'separator' : 'item', - })), - })); -}); +const groupedOrderedItems = computed(() => { + const result = []; + for (let groupIndex = 0; groupIndex < itemGroups.value.length; groupIndex++) { + const group = itemGroups.value[groupIndex]; + const items = []; + for (let index = 0; index < group.length; index++) { + const item = group[index]; + items.push({ + item, + index, + type: isLabelItem(item) ? 'label' : isSeparatorItem(item) ? 'separator' : 'item', + }); + } + result.push({ groupIndex, items }); + } + return result; +});
179-198: Consider component complexity and maintainability.The template has nested v-for loops with multiple conditional renderings, which increases complexity and could impact performance with large datasets.
Consider extracting the item rendering logic into a separate component:
<!-- SelectItemRenderer.vue --> <template> <SelectLabel v-if="type === 'label'"> {{ getItemLabel(item) }} </SelectLabel> <SelectSeparator v-if="type === 'separator'" /> <SelectItem v-if="type === 'item'" :value="getItemValue(item)!" :disabled="isStructuredItem(item) ? item.disabled : false" :class="isStructuredItem(item) ? item.class : undefined" > <slot name="item" :item="item" :index="index"> {{ getItemLabel(item) }} </slot> </SelectItem> </template>Then use it in the main template:
- <template v-for="{ item, index, type } in items" :key="index"> - <SelectLabel v-if="type === 'label'"> - {{ getItemLabel(item) }} - </SelectLabel> - - <SelectSeparator v-if="type === 'separator'" /> - - <SelectItem - v-if="type === 'item'" - :value="getItemValue(item)!" - :disabled="isStructuredItem(item) ? item.disabled : false" - :class="isStructuredItem(item) ? item.class : undefined" - > - <slot name="item" :item="item" :index="index"> - {{ getItemLabel(item) }} - </slot> - </SelectItem> - </template> + <SelectItemRenderer + v-for="{ item, index, type } in items" + :key="index" + :item="item" + :index="index" + :type="type" + > + <template #item="{ item, index }"> + <slot name="item" :item="item" :index="index"> + {{ getItemLabel(item) }} + </slot> + </template> + </SelectItemRenderer>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (64)
CLAUDE.md(3 hunks)api/package.json(2 hunks)packages/unraid-api-plugin-connect/package.json(4 hunks)packages/unraid-shared/package.json(2 hunks)unraid-ui/components.json(1 hunks)unraid-ui/package.json(1 hunks)unraid-ui/src/components/common/accordion/Accordion.vue(1 hunks)unraid-ui/src/components/common/accordion/index.ts(1 hunks)unraid-ui/src/components/common/dialog/Dialog.vue(1 hunks)unraid-ui/src/components/common/dialog/DialogClose.vue(0 hunks)unraid-ui/src/components/common/dialog/index.ts(1 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenu.vue(1 hunks)unraid-ui/src/components/common/dropdown-menu/DropdownMenuArrow.vue(0 hunks)unraid-ui/src/components/common/dropdown-menu/index.ts(1 hunks)unraid-ui/src/components/form/select/Select.vue(1 hunks)unraid-ui/src/components/form/select/index.ts(1 hunks)unraid-ui/src/components/ui/accordion/AccordionItem.vue(1 hunks)unraid-ui/src/components/ui/accordion/AccordionRoot.vue(1 hunks)unraid-ui/src/components/ui/accordion/AccordionTrigger.vue(1 hunks)unraid-ui/src/components/ui/accordion/index.ts(1 hunks)unraid-ui/src/components/ui/dialog/DialogClose.vue(1 hunks)unraid-ui/src/components/ui/dialog/DialogContent.vue(1 hunks)unraid-ui/src/components/ui/dialog/DialogRoot.vue(1 hunks)unraid-ui/src/components/ui/dialog/DialogScrollContent.vue(1 hunks)unraid-ui/src/components/ui/dialog/index.ts(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuCheckboxItem.vue(2 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuContent.vue(2 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuItem.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuLabel.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioGroup.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioItem.vue(2 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRoot.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSeparator.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSub.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubContent.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubTrigger.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/DropdownMenuTrigger.vue(1 hunks)unraid-ui/src/components/ui/dropdown-menu/index.ts(1 hunks)unraid-ui/src/components/ui/index.ts(1 hunks)unraid-ui/src/components/ui/select/SelectContent.vue(2 hunks)unraid-ui/src/components/ui/select/SelectGroup.vue(1 hunks)unraid-ui/src/components/ui/select/SelectItem.vue(2 hunks)unraid-ui/src/components/ui/select/SelectRoot.vue(1 hunks)unraid-ui/src/components/ui/select/SelectScrollDownButton.vue(1 hunks)unraid-ui/src/components/ui/select/SelectScrollUpButton.vue(1 hunks)unraid-ui/src/components/ui/select/SelectSeparator.vue(1 hunks)unraid-ui/src/components/ui/select/SelectTrigger.vue(1 hunks)unraid-ui/src/components/ui/select/index.ts(1 hunks)unraid-ui/src/forms/Select.vue(3 hunks)unraid-ui/src/index.ts(1 hunks)unraid-ui/stories/components/common/Accordion.stories.ts(1 hunks)unraid-ui/stories/components/common/Dialog.stories.ts(1 hunks)unraid-ui/stories/components/common/DropdownMenu.stories.ts(2 hunks)unraid-ui/stories/components/common/Loading.stories.ts(3 hunks)unraid-ui/stories/components/common/Sheet.stories.ts(3 hunks)unraid-ui/stories/components/form/Select.stories.ts(1 hunks)web/__test__/components/UserProfile.test.ts(1 hunks)web/__test__/mocks/ui-components.ts(1 hunks)web/components/ApiKey/ApiKeyCreate.vue(2 hunks)web/components/UserProfile.ce.vue(2 hunks)web/components/UserProfile/DropdownMenu.vue(0 hunks)web/components/UserProfile/DropdownTrigger.vue(1 hunks)web/helpers/globals.d.ts(1 hunks)web/package.json(2 hunks)
💤 Files with no reviewable changes (3)
- unraid-ui/src/components/common/dialog/DialogClose.vue
- unraid-ui/src/components/common/dropdown-menu/DropdownMenuArrow.vue
- web/components/UserProfile/DropdownMenu.vue
✅ Files skipped from review due to trivial changes (2)
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuTrigger.vue
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (55)
- unraid-ui/components.json
- unraid-ui/src/components/ui/dialog/DialogContent.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuItem.vue
- unraid-ui/package.json
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioGroup.vue
- unraid-ui/src/components/ui/accordion/AccordionTrigger.vue
- unraid-ui/src/components/ui/accordion/AccordionItem.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSub.vue
- packages/unraid-shared/package.json
- unraid-ui/src/index.ts
- unraid-ui/src/components/ui/dialog/DialogScrollContent.vue
- unraid-ui/src/components/ui/index.ts
- unraid-ui/src/components/ui/select/SelectTrigger.vue
- web/test/components/UserProfile.test.ts
- unraid-ui/src/components/ui/select/SelectGroup.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRadioItem.vue
- web/helpers/globals.d.ts
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuCheckboxItem.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubContent.vue
- unraid-ui/src/components/ui/select/SelectScrollUpButton.vue
- unraid-ui/src/components/ui/select/SelectSeparator.vue
- unraid-ui/src/components/ui/select/SelectScrollDownButton.vue
- web/package.json
- unraid-ui/src/components/ui/accordion/index.ts
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSeparator.vue
- packages/unraid-api-plugin-connect/package.json
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuLabel.vue
- unraid-ui/src/components/common/dialog/index.ts
- unraid-ui/src/components/ui/dialog/DialogClose.vue
- web/components/UserProfile.ce.vue
- unraid-ui/src/forms/Select.vue
- unraid-ui/src/components/common/accordion/index.ts
- web/test/mocks/ui-components.ts
- unraid-ui/src/components/ui/select/SelectItem.vue
- unraid-ui/src/components/ui/dialog/DialogRoot.vue
- web/components/UserProfile/DropdownTrigger.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuContent.vue
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuRoot.vue
- unraid-ui/src/components/ui/accordion/AccordionRoot.vue
- unraid-ui/stories/components/common/Sheet.stories.ts
- unraid-ui/src/components/ui/select/index.ts
- unraid-ui/src/components/common/dropdown-menu/index.ts
- unraid-ui/src/components/form/select/index.ts
- unraid-ui/src/components/ui/select/SelectContent.vue
- unraid-ui/src/components/ui/select/SelectRoot.vue
- unraid-ui/src/components/ui/dialog/index.ts
- unraid-ui/src/components/common/dropdown-menu/DropdownMenu.vue
- unraid-ui/src/components/ui/dropdown-menu/index.ts
- unraid-ui/stories/components/form/Select.stories.ts
- unraid-ui/src/components/common/dialog/Dialog.vue
- api/package.json
- unraid-ui/src/components/common/accordion/Accordion.vue
- unraid-ui/stories/components/common/Dialog.stories.ts
- unraid-ui/src/components/ui/dropdown-menu/DropdownMenuSubTrigger.vue
- web/components/ApiKey/ApiKeyCreate.vue
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.{ts,tsx}`: TypeScript imports use .js extensions for ESM compatibility
**/*.{ts,tsx}: TypeScript imports use .js extensions for ESM compatibility
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
unraid-ui/stories/components/common/Accordion.stories.tsunraid-ui/stories/components/common/DropdownMenu.stories.tsunraid-ui/stories/components/common/Loading.stories.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: mdatelle
PR: unraid/api#1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
Learnt from: elibosley
PR: unraid/api#1211
File: unraid-ui/src/components/form/number/NumberFieldInput.vue:1-21
Timestamp: 2025-03-13T16:17:21.897Z
Learning: The unraid-ui is a UI library being published externally, where wrapping third-party components (like those from reka-ui) is an intentional design choice to maintain a consistent interface, standardize styling, and control the exposed API.
Learnt from: pujitm
PR: unraid/api#1211
File: unraid-ui/package.json:43-43
Timestamp: 2025-03-14T19:18:23.324Z
Learning: The @internationalized/number package is a dependency of reka-ui, which is used for number formatting in the Unraid UI components.
Learnt from: elibosley
PR: unraid/api#1308
File: unraid-ui/src/components/common/loading/Error.vue:2-2
Timestamp: 2025-04-02T21:21:29.168Z
Learning: Components in the unraid-ui folder require explicit imports and are not autoloaded, unlike other parts of the project that may use Nuxt.js autoloading features.
Learnt from: elibosley
PR: unraid/api#1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:24-27
Timestamp: 2025-01-31T22:01:22.708Z
Learning: The Unraid UI uses a modern notification system with a custom `unraid-toaster` component replacing the legacy jGrowl notifications. The system is backed by a comprehensive GraphQL API with real-time subscription support for notification updates.
Learnt from: elibosley
PR: unraid/api#1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch removes the old jGrowl notification system and is complemented by the unraid-toaster component implementation. The new system is added through the DefaultPageLayout modification which inserts the toaster component with proper position configuration based on user preferences.
Learnt from: elibosley
PR: unraid/api#1101
File: api/src/unraid-api/unraid-file-modifier/modifications/patches/default-page-layout.patch:6-20
Timestamp: 2025-01-31T22:01:41.842Z
Learning: The default-page-layout.patch is used to remove the old jGrowl notification system from Unraid pages, as notifications are handled by a new system implemented on a different page.
Learnt from: elibosley
PR: unraid/api#1181
File: web/store/theme.ts:0-0
Timestamp: 2025-02-24T14:51:21.328Z
Learning: In the Unraid API project's theme system, exact TypeScript type definitions are preferred over index signatures for theme variables to ensure better type safety.
Learnt from: zackspear
PR: unraid/api#1079
File: web/scripts/deploy-dev.sh:51-54
Timestamp: 2025-01-29T00:59:26.633Z
Learning: For the Unraid web components deployment process, JS file validation isn't required in auth-request.php updates since the files come from the controlled build pipeline where we are the source.
Learnt from: pujitm
PR: unraid/api#1075
File: unraid-ui/src/register.ts:15-34
Timestamp: 2025-01-30T19:38:02.478Z
Learning: In the web components registration process for unraid-ui, use a soft-fail approach (logging warnings/errors) instead of throwing errors, to ensure other components can still register successfully even if one component fails.
unraid-ui/stories/components/common/Accordion.stories.ts (1)
Learnt from: zackspear
PR: unraid/api#1143
File: web/pages/webComponents.vue:28-29
Timestamp: 2025-02-18T18:38:24.981Z
Learning: In Vue components, when there's a repeating pattern of heading + content + divider, prefer using a wrapper component that takes the heading as a prop and content via slots, rather than duplicating the structure. This improves maintainability and reduces code duplication.
unraid-ui/src/components/form/select/Select.vue (9)
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Applies to web/store/**/*.{ts,js} : Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Learnt from: zackspear
PR: unraid/api#1143
File: web/pages/webComponents.vue:28-29
Timestamp: 2025-02-18T18:38:24.981Z
Learning: In Vue components, when there's a repeating pattern of heading + content + divider, prefer using a wrapper component that takes the heading as a prop and content via slots, rather than duplicating the structure. This improves maintainability and reduces code duplication.
Learnt from: pujitm
PR: unraid/api#1211
File: plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Connect.page:0-0
Timestamp: 2025-03-14T19:14:10.408Z
Learning: *.ce.vue components in `web/components` get built as custom elements and prefixed with `unraid-`, making them available as HTML custom elements at runtime.
Learnt from: pujitm
PR: unraid/api#1211
File: plugin/source/dynamix.unraid.net/usr/local/emhttp/plugins/dynamix.my.servers/Connect.page:0-0
Timestamp: 2025-03-14T19:14:10.408Z
Learning: *.ce.vue components in `web/components` get built as custom elements and prefixed with `unraid-`, making them available as HTML custom elements at runtime.
Learnt from: mdatelle
PR: unraid/api#1122
File: web/components/UserProfile/DropdownLaunchpad.vue:38-42
Timestamp: 2025-02-06T17:24:58.784Z
Learning: In the DropdownLaunchpad.vue component, the current implementation of conditional icon rendering using `h(BrandLoading, { variant: 'white' })` is a temporary fix to maintain build stability, with a proper refactoring being handled in a separate branch.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/*.test.ts : Check for expected prop handling and event emissions in component tests
Learnt from: pujitm
PR: unraid/api#1219
File: api/src/unraid-api/main.ts:18-25
Timestamp: 2025-03-07T19:29:12.540Z
Learning: When handling type mismatches between dependencies in TypeScript (such as between NestJS and Fastify), prefer using `@ts-expect-error` with a detailed explanation comment instead of `as any` type assertions. This better documents the reason for the type bypass and makes the issue more visible for future maintenance.
Learnt from: pujitm
PR: unraid/api#1211
File: web/codegen.ts:14-14
Timestamp: 2025-03-12T13:35:43.900Z
Learning: The JSON scalar type in web/codegen.ts was temporarily changed from 'string' to 'any' for compatibility with JsonForms integration. This change facilitates the implementation of the Connect settings web component.
Learnt from: elibosley
PR: unraid/api#1181
File: web/store/theme.ts:0-0
Timestamp: 2025-02-24T14:51:21.328Z
Learning: In the Unraid API project's theme system, exact TypeScript type definitions are preferred over index signatures for theme variables to ensure better type safety.
unraid-ui/stories/components/common/Loading.stories.ts (18)
Learnt from: elibosley
PR: unraid/api#974
File: web/components/Loading/Error.vue:1-3
Timestamp: 2024-12-06T17:38:40.999Z
Learning: In Nuxt.js projects, components are automatically imported, so explicit import statements for components like `LoadingSpinner` in `web/components/Loading/Error.vue` are not necessary.
Learnt from: pujitm
PR: unraid/api#974
File: web/components/Loading/Error.vue:50-50
Timestamp: 2024-12-06T17:34:16.133Z
Learning: In this project, the `Button` component from `~/components/shadcn/Button.vue` is autoloaded and does not need to be imported manually in components like `web/components/Loading/Error.vue`.
Learnt from: mdatelle
PR: unraid/api#1122
File: web/components/UserProfile/DropdownLaunchpad.vue:38-42
Timestamp: 2025-02-06T17:24:58.784Z
Learning: In the DropdownLaunchpad.vue component, the current implementation of conditional icon rendering using `h(BrandLoading, { variant: 'white' })` is a temporary fix to maintain build stability, with a proper refactoring being handled in a separate branch.
Learnt from: elibosley
PR: unraid/api#1308
File: unraid-ui/src/components/common/loading/Error.vue:2-2
Timestamp: 2025-04-02T21:21:29.168Z
Learning: Components in the unraid-ui folder require explicit imports and are not autoloaded, unlike other parts of the project that may use Nuxt.js autoloading features.
Learnt from: mdatelle
PR: unraid/api#1106
File: unraid-ui/src/components/index.ts:2-2
Timestamp: 2025-02-04T17:21:39.710Z
Learning: The unraid-ui package is undergoing a major refactoring process, and breaking changes are expected during this transition period.
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Applies to web/store/**/*.{ts,js} : Ensure Vue reactivity imports are added to store files (computed, ref, watchEffect)
Learnt from: elibosley
PR: unraid/api#972
File: web/components/ColorSwitcher.ce.vue:1-2
Timestamp: 2024-12-17T13:55:42.068Z
Learning: In this Nuxt.js project, components used in templates are automatically imported by Nuxt, so explicit import statements for components are unnecessary.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/__test__/store/**/*.ts : Verify state changes by updating the store in store tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/__test__/store/**/*.ts : Verify proper error handling in store tests
Learnt from: pujitm
PR: unraid/api#1075
File: unraid-ui/src/register.ts:15-34
Timestamp: 2025-01-30T19:38:02.478Z
Learning: In the web components registration process for unraid-ui, use a soft-fail approach (logging warnings/errors) instead of throwing errors, to ensure other components can still register successfully even if one component fails.
Learnt from: elibosley
PR: unraid/api#1408
File: web/components/ApiKey/PermissionCounter.vue:6-6
Timestamp: 2025-05-23T21:59:29.632Z
Learning: This codebase uses ESM (ECMAScript Modules) and requires .js extensions in import statements, even when importing from TypeScript files, as the imports refer to the compiled JavaScript output.
Learnt from: pujitm
PR: unraid/api#1219
File: api/src/unraid-api/main.ts:18-25
Timestamp: 2025-03-07T19:29:12.540Z
Learning: When handling type mismatches between dependencies in TypeScript (such as between NestJS and Fastify), prefer using `@ts-expect-error` with a detailed explanation comment instead of `as any` type assertions. This better documents the reason for the type bypass and makes the issue more visible for future maintenance.
Learnt from: pujitm
PR: unraid/api#1075
File: web/components/Notifications/Sidebar.vue:41-58
Timestamp: 2025-01-31T14:42:06.752Z
Learning: When handling errors in code that uses toast notifications, avoid using toast in the error handler itself since toast functionality might be the source of the error. Use console.error or other reliable logging mechanisms instead.
Learnt from: pujitm
PR: unraid/api#975
File: web/components/Notifications/TabList.vue:1-24
Timestamp: 2024-12-09T15:47:29.325Z
Learning: In our Nuxt setup using Vue.js, components defined within the codebase are autoloaded and do not require explicit import statements in the script section. For example, components like `TabsList` and `TabsTrigger` used in `web/components/Notifications/TabList.vue` are automatically available without imports.
Learnt from: pujitm
PR: unraid/api#1417
File: web/components/ConnectSettings/ConnectSettings.ce.vue:11-18
Timestamp: 2025-06-13T17:14:21.739Z
Learning: The project’s build tooling auto-imports common Vue/Pinia helpers such as `storeToRefs`, so explicit import statements for them are not required.
Learnt from: zackspear
PR: unraid/api#1143
File: web/pages/webComponents.vue:28-29
Timestamp: 2025-02-18T18:38:24.981Z
Learning: In Vue components, when there's a repeating pattern of heading + content + divider, prefer using a wrapper component that takes the heading as a prop and content via slots, rather than duplicating the structure. This improves maintainability and reduces code duplication.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/*.test.ts : Use `mount` from Vue Test Utils for component testing
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/*.test.ts : Stub complex child components that aren't the focus of the test
🪛 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)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Build API
- GitHub Check: Test API
- GitHub Check: Build Web App
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
unraid-ui/stories/components/common/DropdownMenu.stories.ts (1)
1-246: Excellent refactoring to demonstrate the new simplified DropdownMenu API.The comprehensive story coverage effectively showcases the component's capabilities, from basic prop-driven usage to advanced patterns like custom triggers and click handlers. The variety of examples will serve as excellent documentation for users of this component.
unraid-ui/stories/components/common/Accordion.stories.ts (1)
1-365: Comprehensive accordion story coverage provides excellent documentation.The variety of usage patterns - from basic prop-driven usage to advanced dynamic component rendering - effectively demonstrates the accordion component's flexibility and capabilities. The mix of direct composition and prop-based approaches shows users multiple ways to leverage the component.
unraid-ui/stories/components/common/Loading.stories.ts (1)
1-53: Stylistic improvements maintain code consistency.The quote style changes and removal of unused parameters improve code consistency across the project.
unraid-ui/src/components/form/select/Select.vue (2)
2-11: Imports align with the UI library refactor.The imports from
@/components/ui/selectare consistent with the broader refactor to replace external dependencies with local UI primitives.
36-40: Well-designed type system for flexible item handling.The union type
SelectItemTypeeffectively supports various item formats (primitives, structured items, labels, separators) while maintaining type safety.
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
🤖 I have created a release *beep* *boop* --- ## [4.9.0](v4.8.0...v4.9.0) (2025-07-08) ### Features * add graphql resource for API plugins ([#1420](#1420)) ([642a220](642a220)) * add management page for API keys ([#1408](#1408)) ([0788756](0788756)) * add rclone ([#1362](#1362)) ([5517e75](5517e75)) * API key management ([#1407](#1407)) ([d37dc3b](d37dc3b)) * api plugin management via CLI ([#1416](#1416)) ([3dcbfbe](3dcbfbe)) * build out docker components ([#1427](#1427)) ([711cc9a](711cc9a)) * docker and info resolver issues ([#1423](#1423)) ([9901039](9901039)) * fix shading in UPC to be less severe ([#1438](#1438)) ([b7c2407](b7c2407)) * info resolver cleanup ([#1425](#1425)) ([1b279bb](1b279bb)) * initial codeql setup ([#1390](#1390)) ([2ade7eb](2ade7eb)) * initialize claude code in codebse ([#1418](#1418)) ([b6c4ee6](b6c4ee6)) * move api key fetching to use api key service ([#1439](#1439)) ([86bea56](86bea56)) * move to cron v4 ([#1428](#1428)) ([b8035c2](b8035c2)) * move to iframe for changelog ([#1388](#1388)) ([fcd6fbc](fcd6fbc)) * native slackware package ([#1381](#1381)) ([4f63b4c](4f63b4c)) * send active unraid theme to docs ([#1400](#1400)) ([f71943b](f71943b)) * slightly better watch mode ([#1398](#1398)) ([881f1e0](881f1e0)) * upgrade nuxt-custom-elements ([#1461](#1461)) ([345e83b](345e83b)) * use bigint instead of long ([#1403](#1403)) ([574d572](574d572)) ### Bug Fixes * activation indicator removed ([5edfd82](5edfd82)) * alignment of settings on ManagementAccess settings page ([#1421](#1421)) ([70c790f](70c790f)) * allow rclone to fail to initialize ([#1453](#1453)) ([7c6f02a](7c6f02a)) * always download 7.1 versioned files for patching ([edc0d15](edc0d15)) * api `pnpm type-check` ([#1442](#1442)) ([3122bdb](3122bdb)) * **api:** connect config `email` validation ([#1454](#1454)) ([b9a1b9b](b9a1b9b)) * backport unraid/webgui[#2269](https://github.com/unraid/api/issues/2269) rc.nginx update ([#1436](#1436)) ([a7ef06e](a7ef06e)) * bigint ([e54d27a](e54d27a)) * config migration from `myservers.cfg` ([#1440](#1440)) ([c4c9984](c4c9984)) * **connect:** fatal race-condition in websocket disposal ([#1462](#1462)) ([0ec0de9](0ec0de9)) * **connect:** mothership connection ([#1464](#1464)) ([7be8bc8](7be8bc8)) * console hidden ([9b85e00](9b85e00)) * debounce is too long ([#1426](#1426)) ([f12d231](f12d231)) * delete legacy connect keys and ensure description ([22fe91c](22fe91c)) * **deps:** pin dependencies ([#1465](#1465)) ([ba75a40](ba75a40)) * **deps:** pin dependencies ([#1470](#1470)) ([412b329](412b329)) * **deps:** storybook v9 ([#1476](#1476)) ([45bb49b](45bb49b)) * **deps:** update all non-major dependencies ([#1366](#1366)) ([291ee47](291ee47)) * **deps:** update all non-major dependencies ([#1379](#1379)) ([8f70326](8f70326)) * **deps:** update all non-major dependencies ([#1389](#1389)) ([cb43f95](cb43f95)) * **deps:** update all non-major dependencies ([#1399](#1399)) ([68df344](68df344)) * **deps:** update dependency @types/diff to v8 ([#1393](#1393)) ([00da27d](00da27d)) * **deps:** update dependency cache-manager to v7 ([#1413](#1413)) ([9492c2a](9492c2a)) * **deps:** update dependency commander to v14 ([#1394](#1394)) ([106ea09](106ea09)) * **deps:** update dependency diff to v8 ([#1386](#1386)) ([e580f64](e580f64)) * **deps:** update dependency dotenv to v17 ([#1474](#1474)) ([d613bfa](d613bfa)) * **deps:** update dependency lucide-vue-next to ^0.509.0 ([#1383](#1383)) ([469333a](469333a)) * **deps:** update dependency marked to v16 ([#1444](#1444)) ([453a5b2](453a5b2)) * **deps:** update dependency shadcn-vue to v2 ([#1302](#1302)) ([26ecf77](26ecf77)) * **deps:** update dependency vue-sonner to v2 ([#1401](#1401)) ([53ca414](53ca414)) * disable file changes on Unraid 7.2 ([#1382](#1382)) ([02de89d](02de89d)) * do not start API with doinst.sh ([7d88b33](7d88b33)) * do not uninstall fully on 7.2 ([#1484](#1484)) ([2263881](2263881)) * drop console with terser ([a87d455](a87d455)) * error logs from `cloud` query when connect is not installed ([#1450](#1450)) ([719f460](719f460)) * flash backup integration with Unraid Connect config ([#1448](#1448)) ([038c582](038c582)) * header padding regression ([#1477](#1477)) ([e791cc6](e791cc6)) * incorrect state merging in redux store ([#1437](#1437)) ([17b7428](17b7428)) * lanip copy button not present ([#1459](#1459)) ([a280786](a280786)) * move to bigint scalar ([b625227](b625227)) * node_modules dir removed on plugin update ([#1406](#1406)) ([7b005cb](7b005cb)) * omit Connect actions in UPC when plugin is not installed ([#1417](#1417)) ([8c8a527](8c8a527)) * parsing of `ssoEnabled` in state.php ([#1455](#1455)) ([f542c8e](f542c8e)) * pin ranges ([#1460](#1460)) ([f88400e](f88400e)) * pr plugin promotion workflow ([#1456](#1456)) ([13bd9bb](13bd9bb)) * proper fallback if missing paths config modules ([7067e9e](7067e9e)) * rc.unraid-api now cleans up older dependencies ([#1404](#1404)) ([83076bb](83076bb)) * remote access lifecycle during boot & shutdown ([#1422](#1422)) ([7bc583b](7bc583b)) * sign out correctly on error ([#1452](#1452)) ([d08fc94](d08fc94)) * simplify usb listing ([#1402](#1402)) ([5355115](5355115)) * theme issues when sent from graph ([#1424](#1424)) ([75ad838](75ad838)) * **ui:** notifications positioning regression ([#1445](#1445)) ([f73e5e0](f73e5e0)) * use some instead of every for connect detection ([9ce2fee](9ce2fee)) ### Reverts * revert package.json dependency updates from commit 711cc9a for api and packages/* ([94420e4](94420e4)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Style
Chores
@jsonformsdependencies across all packages to version^3.6.0.