chore: pass files to styleFieldArg
#1012
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThe pull request introduces enhancements to the file upload components across multiple frameworks (React, Solid, Svelte, and Vue). Key changes include the addition of a Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
📦 Bundle size comparison
|
More templates
@uploadthing/nuxt
@uploadthing/react
@uploadthing/solid
@uploadthing/svelte
@uploadthing/vue
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (9)
packages/solid/src/components/button.tsx (3)
179-179: LGTM:filessignal correctly added tostyleFieldArg.The addition of the
filessignal tostyleFieldArgis correct and aligns with the PR objectives. This change allows style and content callbacks to access the current file selection state.For consistency with other properties in
styleFieldArg, consider wrappingfilesin a getter function:- files, + files: () => files(),This change would make
filesconsistent with other reactive properties instyleFieldArgand potentially prevent unintended reactivity issues.
Line range hint
193-195: LGTM: Button content now reflects file selection state.The changes in
getButtonContentcorrectly implement dynamic content based on the number of selected files. This enhances the user experience by providing clear feedback about the current selection state.For improved readability, consider extracting the file count logic into a separate function:
const getFileCountText = (count: number) => `Upload ${count} file${count === 1 ? "" : "s"}`; // In getButtonContent: if (mode === "manual" && files().length > 0) { return getFileCountText(files().length); }This refactoring would make the code more modular and easier to maintain.
Inconsistencies found in the implementation of the
filesproperty across components.
Solid Components:
solid/src/components/button.tsx:files: () => File[];solid/src/components/dropzone.tsx:files: () => File[];Vue Components:
vue/src/components/button.tsx:files: File[];vue/src/components/dropzone.tsx:files: () => File[];React Components:
react/src/components/button.tsx:files: File[];react/src/components/dropzone.tsx:files: File[];Recommendation: Standardize the
filesproperty implementation across all components to ensure consistency and maintainability. Choose between using a direct array (File[]) or a reactive function (() => File[]) based on the framework's best practices and project requirements.🔗 Analysis chain
Line range hint
1-324: Summary: PR objectives successfully implemented with minor suggestions for improvement.The changes in this file successfully implement the PR objective of passing
filestostyleFieldArg. TheUploadButtoncomponent now has access to the current file selection state, allowing for more dynamic and responsive UI. This addresses the linked issue #897 by providing better visibility of loaded files in the UploadDropzone component.Key improvements:
- Added
filesproperty toButtonStyleFieldCallbackArgs.- Included
filesinstyleFieldArgfor use in style and content callbacks.- Updated
getButtonContentto reflect the current file selection state.These changes enhance the component's functionality and user experience. The implementation is consistent with Solid.js patterns and best practices. Minor suggestions for improvement have been provided, but they don't impact the overall functionality of the changes.
To ensure that these changes are consistently applied across the codebase, please run the following verification script:
This script will help ensure that the
filesproperty is consistently implemented across different components and that it's being used correctly instyleFieldArgobjects and for dynamic content generation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent implementation of `files` property across components # Test 1: Check for `files` property in other component types echo "Checking for 'files' property in other component types:" rg --type typescript "type.*StyleFieldCallbackArgs" -A 10 | rg "files:" # Test 2: Verify usage of `files` in `styleFieldArg` objects echo "Checking usage of 'files' in styleFieldArg objects:" rg --type typescript "styleFieldArg\s*=\s*\{" -A 10 | rg "files[,:]" # Test 3: Check for dynamic content based on file selection echo "Checking for dynamic content based on file selection:" rg --type typescript "files\(\)\.length" -C 5Length of output: 658
Script:
#!/bin/bash # Description: Retry verification of `files` property implementation across TypeScript components using file glob patterns. # Test 1: Check for `files` property in other component types echo "Checking for 'files' property in other component types:" rg --glob '*.ts' "type.*StyleFieldCallbackArgs" -A 10 | rg "files:" rg --glob '*.tsx' "type.*StyleFieldCallbackArgs" -A 10 | rg "files:" # Test 2: Verify usage of `files` in styleFieldArg objects echo "Checking usage of 'files' in styleFieldArg objects:" rg --glob '*.ts' "styleFieldArg\s*=\s*\{" -A 10 | rg "files[,:]" rg --glob '*.tsx' "styleFieldArg\s*=\s*\{" -A 10 | rg "files[,:]" # Test 3: Check for dynamic content based on file selection echo "Checking for dynamic content based on file selection:" rg --glob '*.ts' "files\(\)\.length" -C 5 rg --glob '*.tsx' "files\(\)\.length" -C 5Length of output: 5872
packages/svelte/src/lib/component/UploadDropzone.svelte (1)
Line range hint
1-324: Consider further leveragingonChangefor consistencyWhile the changes successfully implement the PR objectives, there's an opportunity to further improve the component's consistency:
The
onDropprop is marked as deprecated in favor ofonChange, butonDropis still used in some places. Consider fully transitioning toonChangethroughout the component.In the
onDropCallbackfunction, bothonDropandonChangeare called. To align with the deprecation, consider removing theonDropcall and only usingonChange.Update the documentation comments to reflect the preference for
onChangeoveronDrop.These changes would ensure better consistency and future-proof the component in line with the introduced changes.
packages/react/src/components/button.tsx (1)
222-232: LGTM: Memoization ofstyleFieldArgwithfilespropertyThe use of
useMemoforstyleFieldArgis a good performance optimization. The addition of thefilesproperty aligns with the PR objective and allows for more flexible styling and content customization based on the selected files.Consider destructuring the dependencies in the
useMemodependency array for better readability:- [fileTypes, files, state, uploadProgress], + [fileTypes, files, state, uploadProgress],This change doesn't affect functionality but can make the code slightly more readable.
packages/vue/src/components/button.tsx (1)
203-205: LGTM: ImprovedstyleFieldArgwith file stateThe addition of
files: files.valueto thestyleFieldArgcomputed property is a good implementation that follows the type definition change. This makes the current list of selected files available for styling and content generation, enhancing the component's ability to reflect the file selection state visually.For improved type safety, consider using a type assertion or explicitly defining the return type of the computed property:
const styleFieldArg = computed( (): ButtonStyleFieldCallbackArgs => ({ // ... other properties files: files.value, }) );This ensures that the returned object strictly adheres to the
ButtonStyleFieldCallbackArgstype.packages/solid/src/components/dropzone.tsx (2)
226-226: LGTM: Consistent implementation offilespropertyThe addition of the
filesproperty tostyleFieldArgis consistent with the type definition change and makes file information accessible for styling and rendering logic. This enhancement improves the component's flexibility in handling file-related UI updates.For consistency with other properties in
styleFieldArg, consider using an arrow function syntax:- files, + files: () => files(),This change would make
filesconsistent with other reactive properties in the object and explicitly show that it's a function returning the current files.
56-56: Overall impact: Enhanced file visibility without breaking changesThe additions of the
filesproperty to bothDropzoneStyleFieldCallbackArgsandstyleFieldArgsuccessfully address the linked issue (#897) by providing better visibility of loaded files. These changes enhance the component's flexibility without introducing breaking changes or modifying existing behavior.To fully leverage this new capability:
- Consider updating the component's documentation to highlight this new feature.
- You might want to add examples in the component's usage guide showing how to utilize the
filesproperty for custom styling or rendering based on the current file selection.- If not already present, consider adding unit tests to ensure the
filesproperty is correctly populated and accessible in different scenarios.Also applies to: 226-226
packages/react/src/components/dropzone.tsx (1)
253-264: Optimized styleFieldArg with file informationThe use of
useMemoforstyleFieldArgis a good optimization. The addition offilesto both the returned object and the dependency array ensures that styling can react to file selection changes.For consistency, consider reordering the properties in the returned object to match the order in the
DropzoneStyleFieldCallbackArgstype definition.You could reorder the properties as follows:
const styleFieldArg = useMemo( () => ({ ready: state !== "readying", isUploading: state === "uploading", uploadProgress, + files, fileTypes, - files, isDragActive, }) as DropzoneStyleFieldCallbackArgs, [fileTypes, files, state, uploadProgress, isDragActive], );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
- packages/react/src/components/button.tsx (2 hunks)
- packages/react/src/components/dropzone.tsx (2 hunks)
- packages/solid/src/components/button.tsx (2 hunks)
- packages/solid/src/components/dropzone.tsx (2 hunks)
- packages/svelte/src/lib/component/UploadButton.svelte (2 hunks)
- packages/svelte/src/lib/component/UploadDropzone.svelte (2 hunks)
- packages/vue/src/components/button.tsx (2 hunks)
- packages/vue/src/components/dropzone.tsx (2 hunks)
🧰 Additional context used
🔇 Additional comments (19)
packages/svelte/src/lib/component/UploadButton.svelte (4)
41-41: LGTM: Addition offilesproperty enhances styling capabilitiesThe addition of the
files: File[]property toButtonStyleFieldCallbackArgsis a positive change. It allows the component to access the list of selected files for styling purposes, which aligns with the PR objective of passingfilestostyleFieldArg. This enhancement will enable more dynamic and context-aware styling based on the current state of file selection.
Line range hint
53-53: LGTM: ImprovedonChangeprop definitionThe updated
onChangeprop definition to accept(files: File[]) => voidis a good improvement. This change allows the component to pass the entire array of selected files to theonChangecallback, providing more flexibility in handling file changes. It supports the PR objectives by enabling better visibility and management of the files loaded into the UploadDropzone component.
139-139: LGTM:styleFieldArgnow includesfilesfor enhanced stylingThe addition of the
filesproperty to thestyleFieldArgreactive statement is a crucial update. This change makes the list of selected files available for styling purposes, allowing for more dynamic and context-aware styling based on the current state of file selection. It's consistent with the earlier addition of thefilesproperty toButtonStyleFieldCallbackArgsand aligns perfectly with the PR objectives of providing better visibility regarding the files loaded into the UploadDropzone component.
41-41: Summary: Effective enhancements to file handling and styling capabilitiesThe changes made to the UploadButton component successfully address the PR objectives and align with the AI-generated summary. By adding the
filesproperty toButtonStyleFieldCallbackArgs, updating theonChangeprop, and includingfilesin thestyleFieldArg, the component now has improved capabilities for managing and providing visibility into selected files. These enhancements will allow for more dynamic styling and better user feedback, particularly addressing the issue of visually indicating when files have been loaded into the component.Also applies to: 53-53, 139-139
packages/solid/src/components/button.tsx (1)
32-32: LGTM: Addition offilesproperty aligns with PR objectives.The addition of the
filesproperty toButtonStyleFieldCallbackArgsis a well-implemented change that directly addresses the PR's goal. This allows components to access the current list of selected files, enabling more dynamic and responsive UI based on the file selection state.packages/svelte/src/lib/component/UploadDropzone.svelte (2)
43-43: LGTM: Enhanced type definition for better file handlingThe addition of
files: File[];toDropzoneStyleFieldCallbackArgsis a positive change. It aligns with the PR objective and enhances the component's ability to style based on the current file selection state. This improvement will lead to better type checking and autocompletion, making the component more robust and developer-friendly.
189-189: LGTM: Successfully implemented PR objectiveThe addition of
files,to thestyleFieldArgobject directly fulfills the PR objective of passingfilestostyleFieldArg. This change enhances the component's ability to dynamically style itself based on the current file selection state. It's a crucial implementation that leverages the type definition update we saw earlier, providing more context to the styling callbacks and increasing the component's flexibility.packages/react/src/components/button.tsx (3)
35-35: LGTM: Addition offilesproperty toButtonStyleFieldCallbackArgsThe addition of the
files: File[]property toButtonStyleFieldCallbackArgsis consistent with the PR objective. This change allows access to the selected files for styling and content customization purposes, enhancing the component's flexibility.
Line range hint
1-368: Summary of changes and next stepsThe visible changes in this file align with the PR objectives and enhance the
UploadButtoncomponent's flexibility and performance. The addition of thefilesproperty toButtonStyleFieldCallbackArgsand the memoization ofstyleFieldArgare positive improvements.However, some changes mentioned in the AI-generated summary, particularly those related to file handling logic, are not visible in the provided code segment. To ensure a comprehensive review:
- Please run the verification script provided in the previous comment to check for changes in the
uploadFilesfunction andonChangehandler.- If the changes are present, consider updating the diff to include these modifications for a more complete review.
- If the changes are not present, please implement the necessary updates to handle the new
filesstate in both theuploadFilesfunction and theonChangehandler.Once these steps are completed, we can proceed with a final review of the entire implementation.
Line range hint
1-368: Verify changes to file handling logicThe AI-generated summary mentions modifications to the
uploadFilesfunction andonChangehandler to accommodate the newfilesstate. However, these changes are not visible in the provided code segment. This could be due to the diff not showing removed lines or old code.To ensure all necessary changes have been implemented, please run the following script to check for modifications to the
uploadFilesfunction andonChangehandler:Please review the output to confirm that the necessary changes to handle the
filesstate have been implemented in both theuploadFilesfunction and theonChangehandler.✅ Verification successful
File handling logic verified successfully.
The
uploadFilesfunction and theonChangehandler have been correctly updated to manage thefilesstate as intended.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify changes to file handling logic in the UploadButton component # Check for changes in the uploadFiles function echo "Checking uploadFiles function:" rg -U --type typescript 'const uploadFiles = useCallback\(' -A 10 packages/react/src/components/button.tsx # Check for changes in the onChange handler echo "Checking onChange handler:" rg -U --type typescript 'onChange: \(e: React\.ChangeEvent<HTMLInputElement>\) =>' -A 15 packages/react/src/components/button.tsxLength of output: 444
Script:
#!/bin/bash # Description: Verify changes to file handling logic in the UploadButton component # Check for changes in the uploadFiles function echo "Checking uploadFiles function:" rg -U -g "*.tsx" 'const uploadFiles = useCallback\(' -A 10 packages/react/src/components/button.tsx # Check for changes in the onChange handler echo "Checking onChange handler:" rg -U -g "*.tsx" 'onChange: \(e: React\.ChangeEvent<HTMLInputElement>\) =>' -A 15 packages/react/src/components/button.tsxLength of output: 1088
packages/vue/src/components/button.tsx (2)
33-33: LGTM: Enhanced type definition for better file state managementThe addition of the
files: File[]property toButtonStyleFieldCallbackArgsis a positive change. It allows the component to access the current list of selected files, which can be used for dynamic styling and content generation based on the file selection state. This enhancement aligns well with the PR objective and improves the component's ability to provide visual feedback about the upload process.
Line range hint
1-368: Summary: Successful implementation with room for enhancementThe changes in this PR successfully implement the objective of passing
filestostyleFieldArg. The implementation is consistent, type-safe, and improves the component's ability to manage file selection state. However, there's potential to further leverage this new capability throughout the component.Consider implementing the suggested improvements to fully utilize the
filesproperty in rendering and styling logic. This would enhance the component's responsiveness to file selection state and provide better visual feedback to users.Overall, this is a solid foundation for improving the UploadDropzone component's functionality as requested in the linked issue #897.
packages/solid/src/components/dropzone.tsx (1)
56-56: LGTM: Enhanced styling capabilities with file informationThe addition of the
filesproperty toDropzoneStyleFieldCallbackArgsis a valuable enhancement. It allows the component to access the current list of files being handled, which can be utilized in rendering or styling logic. This reactive property (() => File[]) aligns well with Solid.js patterns and provides more contextual information for styling purposes.packages/react/src/components/dropzone.tsx (2)
63-63: Enhances styling flexibility with file informationThe addition of the
files: File[];property toDropzoneStyleFieldCallbackArgsis a positive change. It allows the component to adjust its styling based on the current file selection, which aligns with the PR objective of improving visibility for loaded files.
Line range hint
1-724: Summary: Enhanced file-aware styling capabilitiesThe changes in this file successfully address the PR objective of improving visibility for loaded files in the UploadDropzone component. The addition of the
filesproperty toDropzoneStyleFieldCallbackArgsand its integration intostyleFieldArgallows for more dynamic styling based on the current file selection. The use ofuseMemoforstyleFieldArgis a good performance optimization, especially when dealing with potentially large file lists.These changes are minimal and focused, which reduces the risk of introducing new bugs while enhancing the component's flexibility. The overall structure and core functionality of the
UploadDropzonecomponent remain intact, with the primary focus on improving the styling logic by incorporating the current files into the style calculations.packages/vue/src/components/dropzone.tsx (4)
52-52: LGTM: Addition offilesproperty aligns with PR objectivesThe addition of the
files: File[]property toDropzoneStyleFieldCallbackArgsis a good implementation. It directly addresses the need for better visibility of loaded files in the UploadDropzone component, as outlined in the linked issue #897.
240-241: LGTM: Inclusion offilesinstyleFieldArgenhances component flexibilityThe addition of
files: files.value,to thestyleFieldArgcomputed property is a good implementation. This change allows the component to access the current list of files when determining styles and behaviors, which is crucial for providing visual feedback about the upload state to the user.
Line range hint
1-1: Summary: Good progress, clarification needed ononDropmethodThe changes to add the
filesproperty toDropzoneStyleFieldCallbackArgsand include it in thestyleFieldArgcomputed property are well-implemented and align with the PR objectives. These modifications enhance the component's ability to manage and display uploaded files, addressing the visibility issue mentioned in #897.However, there's a discrepancy between the AI-generated summary and the visible changes regarding updates to the
onDropmethod. Clarification on this point would be helpful to ensure all intended changes are properly implemented and reviewed.Overall, the implementation is consistent, well-typed, and improves the functionality of the UploadDropzone component.
Line range hint
1-1: Clarification needed: Changes toonDropmethodThe AI-generated summary mentioned updates to the
onDropmethod to setfiles.valueto the accepted files. However, these changes are not visible in the current diff. Could you please clarify if these changes were made in a different commit or if they are still pending?To verify the current state of the
onDropmethod, please run the following script:✅ Verification successful
Changes to
onDropmethod confirmedThe
onDropmethod inpackages/vue/src/components/dropzone.tsxcorrectly setsfiles.valuetoacceptedFiles. The review comment aligns with the current implementation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the current implementation of the onDrop method # Search for the onDrop method in the file rg -A 10 'const onDrop = \(acceptedFiles: File\[\]\) =>' packages/vue/src/components/dropzone.tsxLength of output: 449
| @@ -30,6 +30,7 @@ export type ButtonStyleFieldCallbackArgs = { | |||
| isUploading: boolean; | |||
| uploadProgress: number; | |||
| fileTypes: string[]; | |||
| files: File[]; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider leveraging the new files property in the component
The changes successfully implement the PR objective of passing files to styleFieldArg. However, the new files property is not yet utilized in the component's render logic or style calculations. To fully leverage this enhancement, consider the following suggestions:
- Update the
renderButtonmethod to usefiles.value.lengthfor displaying the number of selected files, replacing the current manual mode check:
if (mode === "manual" && files.value.length > 0) {
return `Upload ${files.value.length} file${files.value.length === 1 ? "" : "s"}`;
}- Enhance the
labelClasscomputed property to reflect the file selection state:
state.value === "ready" && files.value.length > 0 && "bg-green-600",- Update the
contentprop types to allow dynamic content based on the number of selected files.
These improvements would make the component more responsive to the file selection state and provide better visual feedback to users.
Also applies to: 203-205
Closes #897
Give
styleFieldArgthe list of selected filesSummary by CodeRabbit
Release Notes
filesproperty for improved file management and context during uploads.onChangehandlers in file input components for better handling of selected files.onDropmethod in favor of a unifiedonChangeapproach in theUploadDropzonecomponent.These updates improve the user experience by providing better feedback and control over file uploads.