-
Notifications
You must be signed in to change notification settings - Fork 0
feat: migrate file upload to shadcn components #181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR removes the DropdownMenu and Label UI components from the frontend, eliminates their auto-import and type declarations across configuration files, and refactors the project creation flow to use a store-based file management approach instead of FileUpload component events. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as CreateProject.vue
participant Composable as useProjectCreationComposable
participant Store as create-project.store
participant API as API
rect rgb(220, 240, 255)
Note over User,API: Before: FileUpload Event-Driven
User->>UI: Upload file via FileUpload
UI->>Composable: createProject(event)
Composable->>Composable: Extract file from event
Composable->>API: Upload file
end
rect rgb(240, 255, 220)
Note over User,API: After: Store-Based File Management
User->>UI: Select file + click button
UI->>Store: setFileToUpload(file)
UI->>Composable: createProject()
Composable->>Store: Get fileToUpload
Composable->>API: Upload file
Composable->>Store: setFileToUpload(undefined)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes The changes involve straightforward component and export removals (homogeneous, low cognitive load), but also include logic refactoring in the composable and store that requires understanding the project creation feature flow and verifying the store-based file management works correctly with the new UI interaction pattern. Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
frontend/src/features/project-management/pages/CreateProject.vue (2)
7-7: Remove unused ref.The
fileInputref is declared but never used in the component. If you don't need programmatic access to the input element, remove this declaration.Apply this diff:
-const fileInput = ref<HTMLInputElement>()And update line 26:
- <Input ref="fileInput" id="file" type="file" accept=".json,.csv" :disabled="store.isCreating" @change="handleFileChange" /> + <Input id="file" type="file" accept=".json,.csv" :disabled="store.isCreating" @change="handleFileChange" />
26-36: Consider displaying the selected filename.After a user selects a file, there's no visual feedback showing which file is selected. This could confuse users who want to verify their selection before clicking "Create Project."
Consider adding a text display showing the selected filename:
<Input id="file" type="file" accept=".json,.csv" :disabled="store.isCreating" @change="handleFileChange" /> + <p v-if="store.fileToUpload" class="text-sm text-gray-600 mt-2"> + Selected: {{ store.fileToUpload.name }} + </p> <Button :disabled="!store.fileToUpload || store.isCreating" type="submit" class="mt-4" @click="createProject" >frontend/src/features/project-management/composables/useProjectCreationComposable.ts (1)
25-26: Reconsider state cleanup timing.The state is cleared immediately after the API call, even before checking for errors. This means if the upload fails, users must re-select their file to retry. Consider moving the
setFileToUpload(undefined)call inside the success path (after line 41) to preserve the file selection on errors.Apply this diff:
store.setIsCreating(false) - store.setFileToUpload(undefined) if (error) { showError(error.value as ApiErrors) return } if (!data?.data?.id) { showError([ { code: 'PROJECT_CREATION_FAILED', message: 'Failed to create project. Please try again.', }, ]) return } + + store.setFileToUpload(undefined) setTimeout(() => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
biome.json(1 hunks)frontend/.biomelintrc-auto-import.json(0 hunks)frontend/auto-imports.d.ts(0 hunks)frontend/components.d.ts(0 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenu.vue(0 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuCheckboxItem.vue(0 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuContent.vue(0 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuGroup.vue(0 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuItem.vue(0 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuLabel.vue(0 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuRadioGroup.vue(0 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuRadioItem.vue(0 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuSeparator.vue(0 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuShortcut.vue(0 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuSub.vue(0 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuSubContent.vue(0 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuSubTrigger.vue(0 hunks)frontend/src/components/ui/dropdown-menu/DropdownMenuTrigger.vue(0 hunks)frontend/src/components/ui/dropdown-menu/index.ts(0 hunks)frontend/src/components/ui/label/Label.vue(0 hunks)frontend/src/components/ui/label/index.ts(0 hunks)frontend/src/features/project-management/composables/useProjectCreationComposable.ts(1 hunks)frontend/src/features/project-management/pages/CreateProject.vue(1 hunks)frontend/src/features/project-management/stores/create-project.store.ts(1 hunks)frontend/vite.config.ts(0 hunks)
💤 Files with no reviewable changes (21)
- frontend/src/components/ui/label/index.ts
- frontend/src/components/ui/dropdown-menu/DropdownMenuContent.vue
- frontend/src/components/ui/dropdown-menu/DropdownMenuSubContent.vue
- frontend/src/components/ui/dropdown-menu/DropdownMenuGroup.vue
- frontend/src/components/ui/label/Label.vue
- frontend/src/components/ui/dropdown-menu/index.ts
- frontend/src/components/ui/dropdown-menu/DropdownMenuCheckboxItem.vue
- frontend/src/components/ui/dropdown-menu/DropdownMenuSeparator.vue
- frontend/src/components/ui/dropdown-menu/DropdownMenuTrigger.vue
- frontend/vite.config.ts
- frontend/src/components/ui/dropdown-menu/DropdownMenuShortcut.vue
- frontend/src/components/ui/dropdown-menu/DropdownMenuSub.vue
- frontend/src/components/ui/dropdown-menu/DropdownMenuRadioItem.vue
- frontend/components.d.ts
- frontend/src/components/ui/dropdown-menu/DropdownMenuLabel.vue
- frontend/auto-imports.d.ts
- frontend/src/components/ui/dropdown-menu/DropdownMenuItem.vue
- frontend/.biomelintrc-auto-import.json
- frontend/src/components/ui/dropdown-menu/DropdownMenuSubTrigger.vue
- frontend/src/components/ui/dropdown-menu/DropdownMenuRadioGroup.vue
- frontend/src/components/ui/dropdown-menu/DropdownMenu.vue
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/features/project-management/composables/useProjectCreationComposable.ts (4)
frontend/src/features/project-management/stores/create-project.store.ts (1)
useCreateProjectStore(1-22)frontend/src/shared/composables/useErrorHandling.ts (1)
useErrorHandling(1-48)frontend/src/core/plugins/api.ts (1)
api(34-37)backend/src/types/error-schemas.ts (2)
ApiErrors(20-26)ApiErrors(27-27)
🔇 Additional comments (4)
biome.json (1)
10-10: Correctly excludes auto-generated type declaration files from biome's scope.Adding
!**/*.d.tsprevents biome from processing generated TypeScript declaration files, which aligns with the PR's cleanup of auto-imports and component type declarations.Verify that this exclusion works as intended by confirming that auto-generated
.d.tsfiles (e.g.,frontend/auto-imports.d.ts,frontend/components.d.ts) are properly excluded and that the build and type-checking processes remain unaffected.frontend/src/features/project-management/stores/create-project.store.ts (1)
1-22: LGTM! Clean store implementation.The store follows Pinia best practices with clear state management and properly typed actions. The addition of
fileToUploadstate andsetFileToUploadaction aligns well with the new store-based file management approach.frontend/src/features/project-management/composables/useProjectCreationComposable.ts (2)
43-57: Clarify or remove the arbitrary delay.The 1-second setTimeout before navigation appears arbitrary. If this is working around a race condition or waiting for backend processing, please document why. Otherwise, consider removing the delay for a more responsive UX.
Can you explain the reason for the 1-second delay? If it's not needed, consider this refactor:
store.setFileToUpload(undefined) - setTimeout(() => { - router - .push({ - name: 'ProjectView', - params: { id: data.data.id, tab: 'data' }, - }) - .catch(() => { - showError([ - { - code: 'PROJECT_CREATION_FAILED', - message: 'Failed to update URL.', - }, - ]) - }) - }, 1000) + router + .push({ + name: 'ProjectView', + params: { id: data.data.id, tab: 'data' }, + }) + .catch(() => { + showError([ + { + code: 'PROJECT_CREATION_FAILED', + message: 'Failed to update URL.', + }, + ]) + })
33-41: Good defensive programming.The added check for missing
idin the response is a good defensive measure that prevents navigating with an undefined ID.
No description provided.