refactor: update modals and color picker #1494
Conversation
WalkthroughThis set of changes refactors several components and tests to streamline usage of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI_Component
participant Dialog
participant Store
User->>UI_Component: Triggers action (e.g., open modal, select theme)
UI_Component->>Dialog: Sets modelValue / opens dialog
Dialog-->>User: Renders modal/dialog with updated content
User->>UI_Component: Interacts (e.g., selects value)
UI_Component->>Store: Updates state (e.g., theme, activation)
Store-->>UI_Component: Provides updated reactive data
Estimated code review effort2 (~15 minutes) Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
✨ Finishing Touches
🪧 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 (1)
web/pages/welcome.vue (1)
2-2: Remove unnecessary explicit import.Based on the project's auto-import configuration, this explicit import is not required. The build tooling auto-imports common Vue/Pinia helpers like
storeToRefs.-import { storeToRefs } from 'pinia';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
unraid-ui/src/components/brand/BrandButton.vue(1 hunks)unraid-ui/src/components/common/dialog/Dialog.vue(4 hunks)unraid-ui/src/components/form/select/Select.vue(1 hunks)unraid-ui/src/components/ui/dialog/DialogContent.vue(2 hunks)unraid-ui/src/components/ui/dialog/DialogScrollContent.vue(2 hunks)web/__test__/components/Activation/WelcomeModal.test.ts(4 hunks)web/components/Activation/ActivationModal.vue(2 hunks)web/components/Activation/WelcomeModal.ce.vue(2 hunks)web/pages/welcome.vue(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
`web/__test__/**/*`: Tests are located under web/__test__, run with pnpm test Us...
web/__test__/**/*: Tests are located under web/test, run with pnpm test
Use mount from Vue Test Utils for component testing in web/test
Stub complex child components that aren't the focus of the test in web/test
Mock external dependencies and services in web/test
Test component behavior and output, not implementation details in web/test
Use createTestingPinia() for mocking stores in components in web/test
Find elements with semantic queries like find('button') rather than data-test IDs in web/test
Use await nextTick() for DOM updates in web/test
Always await async operations before making assertions in web/test
Use createPinia() and setActivePinia when testing Store files
Only use createTestingPinia if you specifically need its testing features
Let stores initialize with their natural default state in store tests
Don't mock the store being tested in store tests
Place all mock declarations at the top level in store tests
Use factory functions for module mocks to avoid hoisting issues in store tests
Clear mocks between tests to ensure isolation in store tests
📄 Source: CodeRabbit Inference Engine (CLAUDE.md)
List of files the instruction was applied to:
web/__test__/components/Activation/WelcomeModal.test.ts
`**/*.{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:
web/__test__/components/Activation/WelcomeModal.test.ts
`**/*.test.ts`: Use `mount` from Vue Test Utils for component testing Stub compl...
**/*.test.ts: Usemountfrom Vue Test Utils for component testing
Stub complex child components that aren't the focus of the test
Mock external dependencies and services in tests
Verify that the expected elements are rendered in component tests
Test component interactions (clicks, inputs, etc.) in tests
Check for expected prop handling and event emissions in component tests
UsecreateTestingPinia()for mocking stores in component tests
Use semantic queries likefind('button')orfind('[data-test="id"]')but prefer not to use data test IDs
Find components withfindComponent(ComponentName)in tests
UsefindAllto check for multiple elements in component tests
Assert on rendered text content withwrapper.text()
Assert on element attributes withelement.attributes()
Verify element existence withexpect(element.exists()).toBe(true)
Check component state through rendered output in tests
Trigger events withawait element.trigger('click')in component tests
Set input values withawait input.setValue('value')in component tests
Test emitted events withwrapper.emitted()in component tests
Mock external services and API calls in tests
Usevi.mock()for module-level mocks in tests
Specify return values for component methods withvi.spyOn()in tests
Reset mocks between tests withvi.clearAllMocks()
Useawait nextTick()for DOM updates in async tests
UseflushPromises()for more complex promise chains in async tests
Always await async operations before making assertions in tests
Don't rely on Nuxt auto-imports in tests
Place all mock declarations at the top level in test files
Use factory functions for module mocks to avoid hoisting issues in test files
Don't mix mock declarations and module mocks incorrectly in test files
Avoid relying on Nuxt's auto-imports in test environment
Clear mocks between tests to ensure isolation
Remember thatvi.mock()calls are hoisted
📄 Source: CodeRabbit Inference Engine (.cursor/rules/web-testing-rules.mdc)
List of files the instruction was applied to:
web/__test__/components/Activation/WelcomeModal.test.ts
🧠 Learnings (10)
📓 Common learnings
Learnt from: mdatelle
PR: unraid/api#1183
File: web/components/Modal.vue:120-120
Timestamp: 2025-02-21T18:59:47.977Z
Learning: In the Modal.vue component, the modal's width is controlled through the `maxWidth` prop (defaults to 'sm:max-w-lg'). The parent containers with `w-screen` and `w-full` classes ensure proper width behavior, making additional width classes on the inner modal content div redundant.
Learnt from: mdatelle
PR: unraid/api#1183
File: web/components/Modal.vue:110-110
Timestamp: 2025-02-21T19:02:09.947Z
Learning: In Vue modal components using @headlessui/vue's TransitionChild, explicit width control (w-full) may be necessary on the TransitionChild component to maintain proper width during scale transitions, even when parent elements have width classes.
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.
web/pages/welcome.vue (12)
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: 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: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Ensure Vue reactivity imports are added to original store files as they may be missing because Nuxt auto import was turned on
Learnt from: zackspear
PR: unraid/api#0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files ensures that all web components share a single Pinia store instance, which is the desired behavior. Without this initialization, each web component would have its own isolated store, breaking the intended architecture.
Learnt from: zackspear
PR: unraid/api#0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. The `setActivePinia(createPinia())` call at the module level in store files is intentional and ensures all web components share a single Pinia store instance, which is the desired behavior. This shared state approach is critical for the application's architecture to function correctly.
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 function correctly.
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: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-06-30T14:52:37.535Z
Learning: Applies to **/__test__/store/**/*.ts : Use `createPinia()` instead of `createTestingPinia()` for most 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 : When testing Store files use `createPinia` and `setActivePinia`
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 : Do not mock the store being tested in the test file; use `createPinia()`
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 `createTestingPinia()` for mocking stores in component 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 : Only use `createTestingPinia` if you specifically need its testing features in store tests
unraid-ui/src/components/brand/BrandButton.vue (2)
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#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`.
unraid-ui/src/components/ui/dialog/DialogScrollContent.vue (4)
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#1183
File: web/components/Modal.vue:120-120
Timestamp: 2025-02-21T18:59:47.977Z
Learning: In the Modal.vue component, the modal's width is controlled through the `maxWidth` prop (defaults to 'sm:max-w-lg'). The parent containers with `w-screen` and `w-full` classes ensure proper width behavior, making additional width classes on the inner modal content div redundant.
Learnt from: mdatelle
PR: unraid/api#1183
File: web/components/Modal.vue:110-110
Timestamp: 2025-02-21T19:02:09.947Z
Learning: In Vue modal components using @headlessui/vue's TransitionChild, explicit width control (w-full) may be necessary on the TransitionChild component to maintain proper width during scale transitions, even when parent elements have width classes.
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.
web/components/Activation/WelcomeModal.ce.vue (14)
Learnt from: elibosley
PR: unraid/api#1369
File: web/components/Activation/WelcomeModal.ce.vue:40-40
Timestamp: 2025-04-25T13:29:07.868Z
Learning: The `#confirmPassword` selector in the WelcomeModal.ce.vue component is intentionally looking for an element that exists in the Unraid OS login page (.login.php), not within the component itself. This is used as part of a font-size adjustment workaround to handle different CSS between login and authenticated pages.
Learnt from: mdatelle
PR: unraid/api#1183
File: web/components/Modal.vue:120-120
Timestamp: 2025-02-21T18:59:47.977Z
Learning: In the Modal.vue component, the modal's width is controlled through the `maxWidth` prop (defaults to 'sm:max-w-lg'). The parent containers with `w-screen` and `w-full` classes ensure proper width behavior, making additional width classes on the inner modal content div redundant.
Learnt from: zackspear
PR: unraid/api#0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. The `setActivePinia(createPinia())` call at the module level in store files is intentional and ensures all web components share a single Pinia store instance, which is the desired behavior. This shared state approach is critical for the application's architecture to function correctly.
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: 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: 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 function correctly.
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: 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: zackspear
PR: unraid/api#0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files ensures that all web components share a single Pinia store instance, which is the desired behavior. Without this initialization, each web component would have its own isolated store, breaking the intended architecture.
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: 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: 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: 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.
unraid-ui/src/components/ui/dialog/DialogContent.vue (3)
Learnt from: mdatelle
PR: unraid/api#1183
File: web/components/Modal.vue:120-120
Timestamp: 2025-02-21T18:59:47.977Z
Learning: In the Modal.vue component, the modal's width is controlled through the `maxWidth` prop (defaults to 'sm:max-w-lg'). The parent containers with `w-screen` and `w-full` classes ensure proper width behavior, making additional width classes on the inner modal content div redundant.
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: 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/common/dialog/Dialog.vue (3)
Learnt from: mdatelle
PR: unraid/api#1183
File: web/components/Modal.vue:120-120
Timestamp: 2025-02-21T18:59:47.977Z
Learning: In the Modal.vue component, the modal's width is controlled through the `maxWidth` prop (defaults to 'sm:max-w-lg'). The parent containers with `w-screen` and `w-full` classes ensure proper width behavior, making additional width classes on the inner modal content div redundant.
Learnt from: mdatelle
PR: unraid/api#1183
File: web/components/Modal.vue:110-110
Timestamp: 2025-02-21T19:02:09.947Z
Learning: In Vue modal components using @headlessui/vue's TransitionChild, explicit width control (w-full) may be necessary on the TransitionChild component to maintain proper width during scale transitions, even when parent elements have width classes.
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.
web/components/Activation/ActivationModal.vue (14)
Learnt from: mdatelle
PR: unraid/api#1183
File: web/components/Modal.vue:120-120
Timestamp: 2025-02-21T18:59:47.977Z
Learning: In the Modal.vue component, the modal's width is controlled through the `maxWidth` prop (defaults to 'sm:max-w-lg'). The parent containers with `w-screen` and `w-full` classes ensure proper width behavior, making additional width classes on the inner modal content div redundant.
Learnt from: mdatelle
PR: unraid/api#1183
File: web/components/Modal.vue:110-110
Timestamp: 2025-02-21T19:02:09.947Z
Learning: In Vue modal components using @headlessui/vue's TransitionChild, explicit width control (w-full) may be necessary on the TransitionChild component to maintain proper width during scale transitions, even when parent elements have width classes.
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#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: elibosley
PR: unraid/api#1369
File: web/components/Activation/WelcomeModal.ce.vue:40-40
Timestamp: 2025-04-25T13:29:07.868Z
Learning: The `#confirmPassword` selector in the WelcomeModal.ce.vue component is intentionally looking for an element that exists in the Unraid OS login page (.login.php), not within the component itself. This is used as part of a font-size adjustment workaround to handle different CSS between login and authenticated pages.
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: zackspear
PR: unraid/api#0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files ensures that all web components share a single Pinia store instance, which is the desired behavior. Without this initialization, each web component would have its own isolated store, breaking the intended architecture.
Learnt from: zackspear
PR: unraid/api#0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. The `setActivePinia(createPinia())` call at the module level in store files is intentional and ensures all web components share a single Pinia store instance, which is the desired behavior. This shared state approach is critical for the application's architecture to function correctly.
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 function correctly.
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: 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: 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 : When testing Store files use `createPinia` and `setActivePinia`
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 : Use `createPinia()` instead of `createTestingPinia()` for most store tests
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.
unraid-ui/src/components/form/select/Select.vue (3)
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: 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.
web/__test__/components/Activation/WelcomeModal.test.ts (26)
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 : Test component interactions (clicks, inputs, etc.) in 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.ts : Check for expected prop handling and event emissions in component 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.ts : Verify that the expected elements are rendered in component tests
Learnt from: elibosley
PR: unraid/api#1369
File: web/components/Activation/WelcomeModal.ce.vue:40-40
Timestamp: 2025-04-25T13:29:07.868Z
Learning: The `#confirmPassword` selector in the WelcomeModal.ce.vue component is intentionally looking for an element that exists in the Unraid OS login page (.login.php), not within the component itself. This is used as part of a font-size adjustment workaround to handle different CSS between login and authenticated pages.
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 : Mock external dependencies appropriately 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.ts : Check component state through rendered output in 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.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 : Trigger events with `await element.trigger('click')` in component 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.ts : Stub complex child components that aren't the focus of the test
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 after actions 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 web/test/mocks/**/*.ts : Frequently used mocks are stored under `web/test/mocks`
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 `vi.mock()` for module-level mocks in 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.ts : Remember that `vi.mock()` calls are hoisted
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 : Test getter dependencies are properly mocked 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.ts : Use factory functions for module mocks to avoid hoisting issues in test files
Learnt from: zackspear
PR: unraid/api#0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. The `setActivePinia(createPinia())` call at the module level in store files is intentional and ensures all web components share a single Pinia store instance, which is the desired behavior. This shared state approach is critical for the application's architecture to function correctly.
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 : Mock external dependencies and services in 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.ts : Use `createTestingPinia()` for mocking stores in component 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.ts : Assert on rendered text content with `wrapper.text()`
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 : Verify element existence with `expect(element.exists()).toBe(true)`
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 : Test action side effects if not stubbed in store tests
Learnt from: CR
PR: unraid/api#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-30T14:50:43.946Z
Learning: Applies to web/__test__/**/* : Use await nextTick() for DOM updates in web/__test__
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 `await nextTick()` for DOM updates in async 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.ts : Use semantic queries like `find('button')` or `find('[data-test="id"]')` but prefer not to use data test IDs
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 : Test emitted events with `wrapper.emitted()` in component tests
Learnt from: mdatelle
PR: unraid/api#1183
File: web/components/Modal.vue:120-120
Timestamp: 2025-02-21T18:59:47.977Z
Learning: In the Modal.vue component, the modal's width is controlled through the `maxWidth` prop (defaults to 'sm:max-w-lg'). The parent containers with `w-screen` and `w-full` classes ensure proper width behavior, making additional width classes on the inner modal content div redundant.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (21)
unraid-ui/src/components/brand/BrandButton.vue (1)
74-74: Focus state opacity removal looks good.The removal of
group-focus:opacity-60simplifies the button's interactive states to only respond to hover. This aligns with the UI refinement objectives.unraid-ui/src/components/ui/dialog/DialogScrollContent.vue (3)
18-21: Props definition looks good.The
showCloseButtonprop is properly typed as optional boolean and integrated correctly into the props interface.
25-25: Proper prop delegation.Correctly excludes
showCloseButtonfrom delegated props to prevent passing it to child components.
61-61: Conditional rendering logic is correct.The
showCloseButton !== falsecondition properly defaults to showing the close button when the prop is undefined or true.unraid-ui/src/components/ui/dialog/DialogContent.vue (3)
17-19: Props definition is consistent.The
showCloseButtonprop implementation matches the pattern used in DialogScrollContent, ensuring consistency across dialog components.
22-22: Proper prop exclusion.Correctly excludes
showCloseButtonfrom delegated props, maintaining the same pattern as DialogScrollContent.
46-46: Conditional rendering matches expected pattern.The
showCloseButton !== falsecondition is consistent with DialogScrollContent and provides the right default behavior.unraid-ui/src/components/form/select/Select.vue (1)
12-12: Good refactoring to use shared types.Importing
AcceptableValuefromreka-uiinstead of defining it locally improves consistency and reduces code duplication across the UI library.web/components/Activation/WelcomeModal.ce.vue (2)
6-6: LGTM: Import updated for Dialog component.The import correctly changes from Modal to Dialog component from
@unraid/ui, aligning with the refactoring objective.
67-84: LGTM: Modal successfully refactored to Dialog with inline content.The refactoring properly:
- Uses
v-modelfor dialog visibility binding- Configures Dialog with appropriate props (
showFooter: false,showCloseButton: false,size: "full")- Moves content from slot-based structure to inline layout
- Maintains existing functionality while using the new Dialog component
The inline content structure with flexbox layout provides better control over the modal's appearance.
unraid-ui/src/components/common/dialog/Dialog.vue (4)
28-29: LGTM: Props interface enhanced with new functionality.The addition of
'full'to the size union type and the newshowCloseButtonboolean prop provide the necessary API for full-screen dialogs with configurable close button visibility.
45-45: LGTM: Sensible default for showCloseButton.Setting
showCloseButton = trueas the default maintains backward compatibility while allowing components to explicitly disable the close button when needed.
66-66: LGTM: Full-screen dialog styles properly implemented.The full size classes (
w-full max-w-full h-full min-h-screen) correctly implement full-screen dialog functionality.
78-89: LGTM: Enhanced component rendering with conditional styling.The implementation correctly:
- Uses dynamic component selection based on scrollable prop
- Applies conditional classes for full-screen positioning
- Passes the showCloseButton prop to dialog content components
- Maintains proper class composition with the
cnutilityThe full-screen positioning classes ensure proper overlay behavior.
web/components/Activation/ActivationModal.vue (3)
6-6: LGTM: Import updated for Dialog component.The import correctly changes from Modal to Dialog component from
@unraid/ui, consistent with the refactoring pattern.
59-66: LGTM: Dialog component properly configured.The Dialog component is correctly configured with:
model-valuebinding for visibility control- Disabled footer and close button for full-screen experience
- Full size for modal-like behavior
- Proper background class application
67-93: LGTM: Content structure properly refactored to inline layout.The refactoring successfully converts from slot-based content to inline structure:
- Partner logo conditionally rendered inline
- Title and description properly positioned
- Activation button and steps well-organized within flexbox containers
- Documentation links appropriately positioned
The inline structure provides better control over layout and spacing.
web/__test__/components/Activation/WelcomeModal.test.ts (4)
14-33: LGTM: Mock updated to reflect Dialog component structure.The mock correctly:
- Imports and preserves actual components while mocking Dialog
- Defines Dialog with appropriate props and emits
- Provides a simple template with proper slot structure
- Maintains testability while reflecting the new component API
154-154: LGTM: Test assertions updated for Dialog component.The test updates correctly:
- Use simplified
buttonselector instead of test IDs- Verify Dialog component existence and props
- Check
modelValueprop for dialog visibility state- Maintain proper async testing patterns
The assertions properly verify the Dialog component's behavior and state management.
Also applies to: 159-162, 167-169
175-175: LGTM: Button selectors simplified and consistent.The change from
[data-testid="brand-button"]tobuttonselector follows the coding guidelines preference for semantic queries over test IDs.Also applies to: 236-236
245-256: LGTM: Dialog component props and content verification.The tests correctly:
- Verify Dialog component props (
modelValue,showFooter,showCloseButton,size)- Check rendered content through text assertions
- Maintain proper component testing patterns
The prop verification ensures the Dialog component is configured correctly for the full-screen modal use case.
Also applies to: 258-264
mdatelle
force-pushed
the
refactor/update-welcome-modal-and-color-picker
branch
from
e9ceadc to
850b857
Compare
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
3 similar comments
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/components/DummyServerSwitcher.vue (1)
11-14: Items array approach is good, but fix the typo.The declarative approach using an items array is much cleaner than the previous nested markup. However, there's a typo in line 13.
- { value: 'oemActiviation', label: 'OEM Activation' }, + { value: 'oemActivation', label: 'OEM Activation' },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
web/__test__/components/Activation/ActivationModal.test.ts(2 hunks)web/__test__/components/Activation/WelcomeModal.test.ts(5 hunks)web/components/Activation/WelcomeModal.ce.vue(4 hunks)web/components/ColorSwitcher.ce.vue(2 hunks)web/components/DummyServerSwitcher.vue(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- web/components/ColorSwitcher.ce.vue
🚧 Files skipped from review as they are similar to previous changes (2)
- web/components/Activation/WelcomeModal.ce.vue
- web/test/components/Activation/WelcomeModal.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
web/__test__/**/*
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
**/*.{ts,tsx}
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- CLAUDE.md
**/*.test.ts
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/web-testing-rules.mdc
🧠 Learnings (3)
📓 Common learnings
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 : Test component interactions (clicks, inputs, etc.) in tests
Learnt from: mdatelle
PR: unraid/api#1183
File: web/components/Modal.vue:120-120
Timestamp: 2025-02-21T18:59:47.977Z
Learning: In the Modal.vue component, the modal's width is controlled through the `maxWidth` prop (defaults to 'sm:max-w-lg'). The parent containers with `w-screen` and `w-full` classes ensure proper width behavior, making additional width classes on the inner modal content div redundant.
web/__test__/components/Activation/ActivationModal.test.ts (22)
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 : Test component interactions (clicks, inputs, etc.) in 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 : Mock external dependencies appropriately 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.ts : Use factory functions for module mocks to avoid hoisting issues in test files
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 : Don't mix mock declarations and module mocks incorrectly in test files
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__/store/**/*.ts : Test getter dependencies are properly mocked 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.ts : Stub complex child components that aren't the focus of the test
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 `vi.mock()` for module-level mocks in 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.ts : Check component state through rendered output in 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 : Test action side effects if not stubbed 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 web/test/mocks/**/*.ts : Frequently used mocks are stored under `web/test/mocks`
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 : Remember that `vi.mock()` calls are hoisted
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: 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 : Mock external dependencies and services in tests
Learnt from: pujitm
PR: unraid/api#1143
File: web/components/DummyServerSwitcher.vue:16-19
Timestamp: 2025-02-18T17:36:22.670Z
Learning: The DummyServerSwitcher component in web/components/DummyServerSwitcher.vue is a development tool for testing different server states, and includes intentional debug elements to aid development.
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/api-rules.mdc:0-0
Timestamp: 2025-06-30T14:50:52.281Z
Learning: Applies to api/**/*.{test,spec}.{js,ts,tsx} : Prefer to not mock simple dependencies in tests
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__/store/**/*.ts : When testing Store files use `createPinia` and `setActivePinia`
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: 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 : Assert on rendered text content with `wrapper.text()`
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 : Verify that the expected elements are rendered in component 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.ts : Verify element existence with `expect(element.exists()).toBe(true)`
web/components/DummyServerSwitcher.vue (16)
Learnt from: pujitm
PR: unraid/api#1143
File: web/components/DummyServerSwitcher.vue:16-19
Timestamp: 2025-02-18T17:36:22.670Z
Learning: The DummyServerSwitcher component in web/components/DummyServerSwitcher.vue is a development tool for testing different server states, and includes intentional debug elements to aid development.
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: 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: 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: 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: 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: 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: zackspear
PR: unraid/api#0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. The `setActivePinia(createPinia())` call at the module level in store files is intentional and ensures all web components share a single Pinia store instance, which is the desired behavior. This shared state approach is critical for the application's architecture to function correctly.
Learnt from: elibosley
PR: unraid/api#1369
File: web/components/Activation/WelcomeModal.ce.vue:40-40
Timestamp: 2025-04-25T13:29:07.868Z
Learning: The `#confirmPassword` selector in the WelcomeModal.ce.vue component is intentionally looking for an element that exists in the Unraid OS login page (.login.php), not within the component itself. This is used as part of a font-size adjustment workaround to handle different CSS between login and authenticated pages.
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 function correctly.
Learnt from: zackspear
PR: unraid/api#0
File: :0-0
Timestamp: 2025-03-27T23:52:57.888Z
Learning: In the unraid/api project, Vue components are compiled into web components. Using `setActivePinia(createPinia())` in store files ensures that all web components share a single Pinia store instance, which is the desired behavior. Without this initialization, each web component would have its own isolated store, breaking the intended architecture.
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 : Set initial state for focused testing 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 : When testing Store files use `createPinia` and `setActivePinia`
Learnt from: elibosley
PR: unraid/api#1063
File: web/_data/serverState.ts:137-147
Timestamp: 2025-01-27T14:57:46.617Z
Learning: The values in `web/_data/serverState.ts` are used for testing purposes and should remain as hardcoded mock data to facilitate testing different scenarios.
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: 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 : Use `createPinia()` instead of `createTestingPinia()` for most store tests
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Build Web App
- GitHub Check: Test API
- GitHub Check: Build API
- GitHub Check: Deploy Storybook
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
web/__test__/components/Activation/ActivationModal.test.ts (3)
14-39: Well-structured mock setup for @unraid/ui.The mock implementation correctly uses
importOriginalto preserve actual exports while providing simplified test doubles forDialogandBrandButton. The Dialog mock includes proper accessibility attributes and slot handling, making it suitable for testing modal behavior.
43-52: Appropriately simplified mock components.The remaining mock components follow testing best practices by providing minimal implementations that focus on the essential props and test identifiers needed for the tests.
246-246: Improved test assertion using semantic element checking.The change from checking exact HTML output to verifying the absence of the dialog element is more robust and semantic. This approach is less brittle and focuses on the actual behavior rather than implementation details.
web/components/DummyServerSwitcher.vue (3)
4-4: Import simplification looks good.The refactoring to import only the main
Selectcomponent instead of multiple subcomponents aligns with the streamlined Select API from@unraid/ui. This makes the component usage more declarative and consistent with similar changes across the codebase.
8-10: Clean spacing improvements.The added blank lines improve code readability by providing better visual separation between logical sections.
24-24: Excellent refactoring of Select usage.The new declarative approach with
v-model,:items, andplaceholderprops is much cleaner than the previous nested subcomponent pattern. This makes the code more maintainable and follows modern Vue patterns.
| <div | ||
| v-if="variant === 'fill'" | ||
| class="absolute -top-[2px] -right-[2px] -bottom-[2px] -left-[2px] -z-10 bg-gradient-to-r from-unraid-red to-orange opacity-100 transition-all rounded-md group-hover:opacity-60 group-focus:opacity-60" | ||
| class="absolute -top-[2px] -right-[2px] -bottom-[2px] -left-[2px] -z-10 bg-gradient-to-r from-unraid-red to-orange opacity-100 transition-all rounded-md group-hover:opacity-60" |
There was a problem hiding this comment.
Did you ensure there's still a focus style on the button? If yes, then this is an ok change
| <div class="flex flex-col p-6"> | ||
| <div class="mx-auto mt-6 mb-8"> |
There was a problem hiding this comment.
I realized this already existed, but do we want to fix the nested margin top on this second div when the flex col parent already has padding?
| {{ t(`First, you'll create your device's login credentials, then you'll activate your Unraid license—your device's operating system (OS).`) }} | ||
| </p> | ||
| </div> | ||
| <p class="text-18px sm:text-20px opacity-75 text-center mt-2">{{ description }}</p> |
There was a problem hiding this comment.
The parent here was to prevent the description from being too wide (which can be hard to read for a lot of text that's centered aligned). So we should make sure this isn't an accessibility regression.
| <div class="flex flex-col gap-6"> | ||
| <ActivationSteps :active-step="2" class="mt-6" /> | ||
| <h1 class="text-center text-20px sm:text-24px font-semibold mt-4">{{ title }}</h1> | ||
| <p class="text-18px sm:text-20px opacity-75 text-center mt-2">{{ description }}</p> |
There was a problem hiding this comment.
we'll want to make sure these descriptions dont get too wide
| <div class="flex flex-col p-6"> | ||
| <div class="mx-auto mt-6 mb-8"> |
There was a problem hiding this comment.
Potential to simplify the nested spacing here. Margin top w/ a parent that has padding can lead to spacing inconsistencies. Let's double check how this looks.
|
|
||
| <div class="flex flex-col sm:flex-row justify-center gap-4 mx-auto w-full"> | ||
| <BrandButton v-for="button in docsButtons" :key="button.text" v-bind="button" /> | ||
| <div class="flex flex-col sm:flex-row justify-center gap-4 mx-auto mt-8 w-full"> |
There was a problem hiding this comment.
shouldn't need the margin top on this if the parent has a gap. Might cause spacing inconsistency.
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
|
@zackspear I cleaned up the padding and margins to be more simplified. I also put the focus back on since we don't have any specific focus style and we'll be swapping these BrandButtons out soon. |
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
2 similar comments
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
mdatelle
force-pushed
the
refactor/update-welcome-modal-and-color-picker
branch
from
45a65fe to
8de540c
Compare
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
2 similar comments
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
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 (1)
unraid-ui/src/styles/globals.css(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 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: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/components/**/*.ts : Test component interactions (clicks, inputs, etc.)
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/components/**/*.ts : Use semantic queries like `find('button')` or `find('[data-test="id"]')` but prefer not to use data test IDs
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/components/**/*.ts : Use `await nextTick()` for DOM updates in async tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Use factory functions for module mocks to avoid hoisting issues
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/components/**/*.ts : Check for expected prop handling and event emissions in component tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/components/**/*.ts : Test component behavior and output, not implementation details
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Don't mix mock declarations and module mocks incorrectly
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Mock external dependencies and services in tests
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/components/**/*.ts : Stub complex child components that aren't the focus of the test
Learnt from: CR
PR: unraid/api#0
File: .cursor/rules/web-testing-rules.mdc:0-0
Timestamp: 2025-07-21T14:01:04.139Z
Learning: Applies to web/__test__/{components,store,mocks}/**/*.ts : Use `vi.mock()` for module-level mocks
unraid-ui/src/styles/globals.css (8)
Learnt from: elibosley
PR: #1181
File: web/store/theme.ts:210-216
Timestamp: 2025-02-21T18:40:10.810Z
Learning: When updating theme-related CSS variables via cssText, preserve existing non-theme styles by filtering out only theme-related rules (those starting with '--') and combining them with the new theme styles.
Learnt from: elibosley
PR: #1522
File: unraid-ui/src/styles/globals.css:172-174
Timestamp: 2025-07-18T16:13:55.872Z
Learning: In Tailwind CSS v4, --breakpoint-* is valid syntax within the @theme directive for defining custom breakpoints. The wildcard pattern allows you to define breakpoints like --breakpoint-xs, --breakpoint-2xl, etc. Setting --breakpoint-*: initial is the documented way to remove default breakpoints. This syntax is specific to Tailwind's theme system and is not subject to standard CSS custom property naming rules.
Learnt from: mdatelle
PR: #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: #1155
File: web/store/theme.ts:49-50
Timestamp: 2025-02-20T15:52:56.733Z
Learning: CSS variable names in the theme store should be concise and follow established patterns. For example, prefer '--gradient-start' over '--color-customgradient-start' to maintain consistency with other variable names.
Learnt from: elibosley
PR: #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: #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: elibosley
PR: #972
File: web/store/theme.ts:46-49
Timestamp: 2024-12-17T14:59:32.458Z
Learning: In the web/store/theme.ts file of the Unraid web application, the header is intentionally designed to have a light background with dark text in dark mode, and a dark background with light text in light mode.
Learnt from: elibosley
PR: #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.
🪛 Biome (1.9.4)
unraid-ui/src/styles/globals.css
[error] 178-178: Unexpected value or character.
Expected one of:
(parse)
[error] 178-178: Expected a qualified rule, or an at rule but instead found ';'.
Expected a qualified rule, or an at rule here.
(parse)
[error] 179-179: Unexpected value or character.
Expected one of:
(parse)
[error] 179-179: Expected a qualified rule, or an at rule but instead found ';'.
Expected a qualified rule, or an at rule here.
(parse)
[error] 180-180: Unexpected value or character.
Expected one of:
(parse)
[error] 180-180: Expected a qualified rule, or an at rule but instead found ';'.
Expected a qualified rule, or an at rule here.
(parse)
[error] 181-181: Unexpected value or character.
Expected one of:
(parse)
[error] 181-181: Expected a qualified rule, or an at rule but instead found ';'.
Expected a qualified rule, or an at rule here.
(parse)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Web App
- GitHub Check: Test API
- GitHub Check: Build API
- GitHub Check: Analyze (javascript-typescript)
|
Looks like this is still a WIP. But I installed the plg from this point - #1494 (comment) and noticed some stuff Activation Modal has some issues
OS Updates modal
|
|
More testing after updating to #1494 (comment) Issues from my previous comment still apply. But here's one more additional thing. Left is new plg, right is a base OS w/
|
|
These all seem related to breakpoint prefixes not working. |
mdatelle
force-pushed
the
refactor/update-welcome-modal-and-color-picker
branch
from
e795236 to
785e30d
Compare
|
🚀 Storybook has been deployed to staging: https://unraid-ui-storybook-staging.unraid-workers.workers.dev |
|
This plugin has been deployed to Cloudflare R2 and is available for testing. |
|
Button sizing is happening on preview also, not part of this PR or a regression due to your code @mdatelle |
3 participants
Summary by CodeRabbit
New Features
/welcomepage.Refactor
Tests