added folder upload option for website #387
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @PaperBoardOfficial, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request addresses issue #377 by adding a folder upload option to the website. The author, PaperBoardOfficial, implemented this by creating a separate button for folder uploads, as a single input tag cannot handle both file and folder uploads simultaneously. The buttons now use icons instead of words for a cleaner interface. The author also ran npm run test and npm run lint.
Highlights
- New Feature: Added folder upload functionality to the website.
- UI Enhancement: Replaced text labels on upload buttons with icons.
- Implementation: Implemented a separate input tag for folder uploads due to limitations of single input tags.
Changelog
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Did you know?
The concept of folders (or directories) was introduced in the 1960s to better organize files on storage devices.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request introduces folder upload functionality, which is a valuable addition to the website. The use of separate buttons with icons for file and folder uploads enhances the user experience. The checklist items indicate that the code has been tested and linted, which is good practice.
Merge Readiness
The pull request appears to be in good shape for merging. The functionality seems well-implemented, and the code has been tested and linted. However, it's always a good idea to have another reviewer take a look before merging.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #387 +/- ##
=======================================
Coverage 90.17% 90.17%
=======================================
Files 72 72
Lines 3472 3472
Branches 750 750
=======================================
Hits 3131 3131
Misses 341 341 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughThis update introduces a new folder upload mode into the existing file upload functionality of the application. In the Sequence Diagram(s)sequenceDiagram
participant U as User
participant T as TryIt.vue
participant F as TryItFolderUpload.vue
participant JS as JSZip Library
U->>T: Click folder mode button (📁)
Note over T: setMode('folder') is invoked
T-->>F: Render TryItFolderUpload component
U->>F: Select a folder (drag-drop or file input)
F->>F: Validate folder files and size (<10MB)
alt Valid folder
F->>JS: Generate ZIP file from folder contents
JS-->>F: Return created ZIP file
F->>T: Emit upload event with ZIP file
T->>T: Update submission logic to consider the uploaded ZIP
else Invalid folder
F->>F: Display error message to user
end
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
website/client/components/Home/TryItFolderUpload.vue (2)
40-51: Add type declaration for the folderName parameter.The
folderNameparameter in thegetZipFilefunction is missing a type declaration.-async function getZipFile(files: FileList, folderName) { +async function getZipFile(files: FileList, folderName: string) {
53-66: Consider adding a loading state during zip creation.For larger folders (even under 10MB), the zip creation process might take some time. Consider adding a loading state during this operation to provide feedback to the user.
+const zipping = ref(false); async function handleFileSelect(files: FileList | null) { if (!files || files.length === 0) return; const relativePath = files[0].webkitRelativePath; const folderName = relativePath.split('/')[0]; + zipping.value = true; const zipFile = await getZipFile(files, folderName); + zipping.value = false; if (validateFolder(files)) { selectedFolder.value = folderName; emit('upload', zipFile); } else { selectedFolder.value = null; } }Don't forget to update the template to show a loading indicator when
zippingis true.website/client/components/Home/TryIt.vue (2)
10-26: Consider using consistent icons for mode selection.The buttons use different representations: one uses an emoji (📁), one uses an image (.zip icon), and one uses a text character (🔗). Consider using a consistent approach for better UI coherence.
Consider using Lucide icons for all three buttons for visual consistency:
- 🔗 + <Link class="mode-icon" size="20" /> - <img class="zip-icon" src="../../src/public/images/zip-folder-icon.png" alt="zip-folder" /> + <FileZip class="mode-icon" size="20" /> - 📁 + <FolderOpen class="mode-icon" size="20" />Don't forget to import the necessary icons from lucide-vue-next.
151-151: Potential issue with repository URL setting.The function sets
inputRepositoryUrl.value = url.valueregardless of the mode. When in folder or file mode, this might not be appropriate since no URL is being used.- inputRepositoryUrl.value = url.value; + if (mode.value === 'url') { + inputRepositoryUrl.value = url.value; + } else { + inputRepositoryUrl.value = ''; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
website/client/src/public/images/zip-folder-icon.pngis excluded by!**/*.png
📒 Files selected for processing (3)
website/client/components/Home/TryIt.vue(7 hunks)website/client/components/Home/TryItFolderUpload.vue(1 hunks)website/client/package.json(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Test (macos-latest, 23.x)
- GitHub Check: Test (windows-latest, 23.x)
- GitHub Check: Test (windows-latest, 22.x)
- GitHub Check: Test (windows-latest, 21.x)
- GitHub Check: Build and run (windows-latest, 22.x)
- GitHub Check: Test (windows-latest, 20.x)
- GitHub Check: Test (windows-latest, 19.x)
- GitHub Check: Test (windows-latest, 18.x)
- GitHub Check: Test (windows-latest, 18.0.0)
- GitHub Check: Build and run (windows-latest, 18.x)
🔇 Additional comments (16)
website/client/package.json (1)
10-10:✅ Verification successful
Added JSZip dependency for folder upload functionality.
This looks good. The JSZip library is an appropriate choice for creating ZIP files from folder contents, which is necessary for the folder upload feature.
🌐 Web query:
Is jszip 3.10.1 the latest stable version? Are there any known security vulnerabilities?💡 Result:
JSZip 3.10.1 is the latest stable version as of August 2022, according to its official GitHub repository and documentation[4][7]. This version includes critical updates like filename sanitization to prevent "zip slip" directory traversal attacks[4], improved TypeScript support[4], and dependency optimizations[4].
Security status:
- No CVEs are listed for JSZip 3.10.1 in the National Vulnerability Database or security tracking platforms like CVE Details[2][5].
- The library addressed a directory traversal vulnerability in v3.8.0 (March 2022) by sanitizing filenames during ZIP file extraction[4].
- Security scans from Snyk show no unpatched vulnerabilities in 3.10.1[10], though older versions (<3.8.0) had security risks.
Key security considerations:
- The v3.x branch uses modern async APIs that reduce race condition risks compared to v2.x[1][4]
- Contains protections against ZIP bomb attacks through stream-based processing[4]
- Maintainers actively respond to security reports, as shown in the Telerik forum discussion about false-positive vulnerability alerts[8]
For secure implementation:
// Always sanitize user-provided filenames before adding to ZIP zip.file(sanitize(filename), contentPromise); // Use async methods with proper error handling zip.generateAsync({type: "blob"}) .then(blob => saveAs(blob)) .catch(error => console.error("ZIP creation failed:", error));Citations:
- 1: https://stuk.github.io/jszip/documentation/upgrade_guide.html
- 2: https://www.cvedetails.com/version/1145782/Jszip-Project-Jszip-3.10.1.html
- 3: ZigBee2MQTT service continuously restarting Koenkk/zigbee2mqtt#21463
- 4: https://github.com/Stuk/jszip/blob/main/CHANGES.md
- 5: https://www.cvedetails.com/product/98772/Jszip-Project-Jszip.html?vendor_id=25021
- 6: https://cve.mitre.org/cgi-bin/cvekey.cgi
- 7: https://stuk.github.io/jszip/
- 8: https://www.telerik.com/forums/security-for-excel-exports
- 9: https://cdnjs.com/libraries/jszip
- 10: https://security.snyk.io/package/npm/jszip
JSZip Dependency Verified as Appropriate
The JSZip dependency in
website/client/package.jsonat line 10 ("jszip": "^3.10.1") is correct. Version 3.10.1 is the latest stable release and has no known security vulnerabilities, making it a well-suited choice for the folder upload feature.website/client/components/Home/TryItFolderUpload.vue (8)
1-6: Component imports look good.The necessary dependencies are correctly imported including JSZip, Lucide icons, Vue ref, and the PackButton component.
7-7: File size limit clearly defined.Good practice to define the maximum file size as a constant at the top of the component, making it easy to adjust if needed.
9-16: Props and emits are well-structured.The component correctly defines its props and emits interfaces with TypeScript, ensuring type safety.
18-21: State variables are appropriately defined.Good use of Vue refs for component state management.
23-38: Folder validation logic looks correct.The validation function properly checks the total size of all files in the folder and sets an appropriate error message.
97-106: Error handling and UI feedback look good.The component provides clear feedback to users with error messages and selected folder information.
119-229: CSS styling is well-organized and responsive.The styling uses CSS variables for theming consistency and includes responsive adjustments for different screen sizes. The use of scoped styles prevents conflicts with other components.
82-90:❓ Verification inconclusive
Browser compatibility concerns with directory attributes.
The
directory,webkitdirectory, andmozdirectoryattributes are non-standard and have varying support across browsers. Consider adding a fallback mechanism or a browser compatibility notice.
🌐 Web query:
Which browsers support webkitdirectory attribute for folder upload?💡 Result:
The
webkitdirectoryattribute, which enables folder uploads via<input type="file">, is supported in most modern browsers except Internet Explorer. Here's the compatibility breakdown:
Browser Supported Versions Notes Sources Chrome 30+ (Desktop) Includes Chrome for Android [6][10] Firefox 50+ (Desktop) Not supported in Firefox for Android [6][10] Edge 14+ Legacy Edge (12-13) unsupported [6][7] Safari 11.1+ (Desktop/iOS) Requires macOS 10.13+ / iOS 11.3+ [4][6] Opera 17+ (Desktop) Opera Mobile (80+) also supported [6][10] Key Details:
- Mobile Support: Works in Chrome for Android and Safari iOS (11.3+), but not Firefox for Android[10].
- Non-Standard Feature: Despite wide adoption, remains a non-standard attribute (originally WebKit-specific)[1][5].
- Fallback Behavior: Browsers without support default to regular file selection, making it safe for progressive enhancement[1][8].
Internet Explorer (all versions) and legacy browsers like Edge 12-13 lack support entirely[6][7]. For server-side handling, PHP 8.1+ natively supports directory uploads via
$_FILES, while frameworks like Spring Boot require custom path reconstruction[2][8].Citations:
- 1: https://www.smashingmagazine.com/2017/09/uploading-directories-with-webkitdirectory/
- 2: https://www.springcloud.io/post/2022-11/springboot-folder-upload/
- 3: Issue with folder upload when webkitdirectory enabeld in word add-in OfficeDev/office-js#4322
- 4: https://www.lambdatest.com/web-technologies/input-file-directory-safari
- 5: https://www.netsted.co.za/uploading-directories-with-webkitdirectory/
- 6: https://caniuse.com/input-file-directory
- 7: https://www.lambdatest.com/web-technologies/input-file-directory-edge
- 8: https://php.watch/articles/php81-directory-uploads
- 9: https://developer.mozilla.org/en-US/docs/Web/API/File/webkitRelativePath
- 10: https://caniuse.com/mdn-api_htmlinputelement_webkitdirectory
Action: Enhance browser compatibility support for folder uploads
The directory attributes (
directory,webkitdirectory, andmozdirectory) are non-standard and, as verified, work in most modern browsers (Chrome 30+, Firefox 50+ on desktop, Edge 14+, Safari 11.1+, and Opera 17+), but they aren’t supported in Internet Explorer and certain legacy browsers (like older Edge versions). To improve user experience, please consider one or both of the following:
- Fallback Mechanism: Implement a fallback (or a graceful degradation) to the standard file selection behavior for browsers without folder upload support.
- Compatibility Notice: Include a clear notice in the UI/documentation alerting users about potential limitations when using unsupported browsers.
Additional references: the latest compatibility data confirms these browser versions, so updating the user instructions or adding runtime checks may enhance usability.
website/client/components/Home/TryIt.vue (7)
36-41: Folder upload component integration looks good.The TryItFolderUpload component is correctly integrated with consistent props and event handling like the file upload component.
82-93: New component import added correctly.The TryItFolderUpload component is properly imported.
113-113: Type definition updated to include folder mode.The mode ref type is correctly updated to include the new 'folder' option.
117-127: IsSubmitValid logic updated to handle folder mode.The computed property correctly checks for the presence of an uploaded file in both 'file' and 'folder' modes.
130-130: SetMode function type updated correctly.The function's type signature is updated to include 'folder' as a valid mode.
181-181: File check updated to include folder mode.The component correctly includes the folder mode when checking for an uploaded file.
258-263: CSS for the zip icon looks good.The styling for the zip icon is appropriately defined to ensure proper sizing and positioning.
There was a problem hiding this comment.
PR Overview
This PR adds folder upload functionality for the website by introducing a new component and updating the main upload view.
- Introduces TryItFolderUpload.vue to allow folder selection and zipping.
- Updates TryIt.vue to integrate a folder upload button and handling alongside URL and file upload.
Reviewed Changes
| File | Description |
|---|---|
| website/client/components/Home/TryItFolderUpload.vue | Implements folder upload with zip packaging of selected folder |
| website/client/components/Home/TryIt.vue | Adds folder mode support and adjusts the file upload handling accordingly |
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
website/client/components/Home/TryItFolderUpload.vue:58
- Consider validating the total file size before generating the zip file to avoid unnecessary processing when the folder exceeds the 10MB limit.
const zipFile = await getZipFile(files, folderName);
|
I'll take a look at this a bit later! |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
website/client/components/Home/TryItFileUpload.vue (1)
75-75: Consider adding an accessible label for the folder icon.Although the folder icon is purely decorative, adding an
aria-labelattribute or similar accessible text can help screen readers.-<FolderArchive v-else class="icon-folder" size="20" /> +<FolderArchive + v-else + class="icon-folder" + size="20" + aria-label="Zip file icon" +/>website/client/components/Home/TryItFolderUpload.vue (3)
40-51: Add a TypeScript type annotation for the folder name.Declaring a specific type (e.g.,
string) helps maintain clarity.-async function getZipFile(files: FileList, folderName) { +async function getZipFile(files: FileList, folderName: string) {
53-66: Refine error handling flow when the folder is invalid.You generate the ZIP file before validating total size. If the folder is invalid (exceeds size), you still do partial work. For large folders, this extra processing might be unnecessary. Consider validating first, then zipping.
1-231: Potential code duplication with TryItFileUpload.Both folder-based and file-based upload components handle validations, drag events, and error messages similarly. Extracting shared logic into a helper module or composable function could reduce duplication.
website/client/components/Home/TryIt.vue (1)
11-12: Ensure a descriptive label or tooltip for the URL icon.Like with other icons, an accessible tooltip or label can assist screen readers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
website/client/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
website/client/components/Home/TryIt.vue(8 hunks)website/client/components/Home/TryItFileUpload.vue(2 hunks)website/client/components/Home/TryItFolderUpload.vue(1 hunks)website/client/package.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- website/client/package.json
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Test (macos-latest, 23.x)
- GitHub Check: Test (windows-latest, 23.x)
- GitHub Check: Test (windows-latest, 22.x)
- GitHub Check: Test (windows-latest, 21.x)
- GitHub Check: Test (windows-latest, 20.x)
- GitHub Check: Build and run (windows-latest, 22.x)
- GitHub Check: Test (windows-latest, 19.x)
- GitHub Check: Test (windows-latest, 18.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (10)
website/client/components/Home/TryItFileUpload.vue (1)
2-2: Icon import update looks good.Replacing
FolderwithFolderArchivealigns with the intended icon change. No issues observed.website/client/components/Home/TryItFolderUpload.vue (1)
23-38: Validate empty or zero-byte folder scenario.Currently, an empty folder selection passes validation, but it may be desirable to display an error if no files exist or if total size is zero. Consider whether you want to allow uploading an empty folder.
Would you like me to generate a follow-up script to search the codebase for calls of
validateFolderand confirm handling of empty folders?website/client/components/Home/TryIt.vue (8)
13-19: Good approach to separate folder mode from regular file mode.This separation avoids user confusion when toggling between file and folder uploads.
25-25: Consistent icon usage.Using
FolderArchivefor file uploads keeps the UI consistent with the rest of the icon set.
36-41: Folder upload integration is well-structured.The new
TryItFolderUploadcomponent is properly conditionally rendered and seamlessly integrated.
84-84: Icon imports look correct.These imports from
lucide-vue-nextmatch the usage within the template.
114-114: Mode variable update is correct.Refactoring the type to include
'folder'is essential for the new functionality.
123-124: Properly validating 'folder' mode.Ensuring the presence of an uploaded file for both 'file' and 'folder' modes is consistent.
131-131: Mode setter accurately accommodates 'folder'.Clean approach to toggling modes without extra branching logic.
182-182: Handling uploaded file for folder mode is correct.Passing
uploadedFilefor both 'file' and 'folder' modes is a straightforward addition.
|
@PaperBoardOfficial I appreciate you adding icons, but I'm sorry to say that we should use Lucide icons to maintain consistency across all devices. I've added a commit to address this icon implementation. 
I'll review the code a bit more, but so far it looks perfect! |
|
I was thinking of using lucide but I thought lets use these icons. But I didn't know that different icons may become inconsistent on different devices. Thanks for guiding me, I'll take care of this next time. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
website/client/components/Home/TryItFolderUpload.vue (6)
30-35: Consider making MAX_FILE_SIZE configurableThe 10MB file size limit is hardcoded. Consider making it a prop so the parent component can configure it as needed.
-const MAX_FILE_SIZE = 10 * 1024 * 1024; // 10MB - const props = defineProps<{ loading: boolean; showButton?: boolean; + maxFileSize?: number; }>(); + +const MAX_FILE_SIZE = props.maxFileSize || 10 * 1024 * 1024; // 10MB
46-63: Round file size in error message for better readabilityWhen displaying the file size error, consider rounding the size to 1 decimal place for better readability.
- const sizeMB = (totalSize / (1024 * 1024)).toFixed(1); + const sizeMB = Math.round((totalSize / (1024 * 1024)) * 10) / 10; errorMessage.value = `File size (${sizeMB}MB) exceeds the 10MB limit`;
65-76: Consider memory usage for large filesThe current zip creation process loads all files into memory at once. For large folders with many files (even if under 10MB total), this could cause performance issues. Consider using streaming or chunking for better performance.
const createZipFile = async (files: File[], folderName: string): Promise<File> => { const zip = new JSZip(); for (const file of files) { const path = file.webkitRelativePath || file.name; - zip.file(path, file); + // Use streaming API for larger files + if (file.size > 1024 * 1024) { // 1MB + zip.file(path, file.stream()); + } else { + zip.file(path, file); + } } - const zipBlob = await zip.generateAsync({ type: 'blob' }); + const zipBlob = await zip.generateAsync({ + type: 'blob', + compression: 'DEFLATE', + compressionOptions: { + level: 6 // Balanced between speed and compression ratio + } + }); return new File([zipBlob], `${folderName}.zip`); }
119-129: Improve error messages for different failure scenariosThe error handling could be more specific about what went wrong with the folder processing.
try { const files = await collectFilesFromEntry(entry); await processFolder(files, folderName); } catch (error) { console.error('Error processing dropped folder:', error); - errorMessage.value = 'Failed to process the folder. Please try again or use the browse button.'; + if (error instanceof Error) { + errorMessage.value = `Error: ${error.message}. Please try again or use the browse button.`; + } else { + errorMessage.value = 'Failed to process the folder. Please try again or use the browse button.'; + } }
131-184: Simplify nested promise structureThe nested promise structure in the collectFilesFromEntry function could be simplified using async/await more consistently.
// Recursively collect files from entry const collectFilesFromEntry = async (entry: WebKitEntry, path = ''): Promise<File[]> => { if (entry.isFile) { - return new Promise((resolve, reject) => { - entry.file((file: File) => { + return new Promise<File[]>((resolve, reject) => { + entry.file(async (file: File) => { // Create custom file with path information const customFile = new File([file], file.name, { type: file.type, lastModified: file.lastModified, }); // Add relative path information Object.defineProperty(customFile, 'webkitRelativePath', { value: path ? `${path}/${file.name}` : file.name, }); resolve([customFile]); }, reject); }); } if (entry.isDirectory && entry.createReader) { - return new Promise((resolve, reject) => { + return new Promise<File[]>((resolve, reject) => { const dirReader = entry.createReader(); const allFiles: File[] = []; // Function to read entries in directory - function readEntries() { + async function readEntries() { dirReader.readEntries(async (entries: WebKitFileEntry[]) => { if (entries.length === 0) { resolve(allFiles); } else { try { // Process each entry for (const childEntry of entries) { const newPath = path ? `${path}/${entry.name}` : entry.name; const files = await collectFilesFromEntry(childEntry as WebKitEntry, newPath); allFiles.push(...files); } // Continue reading (some browsers return entries in batches) readEntries(); } catch (error) { reject(error); } } }, reject); } readEntries(); }); } return []; // If neither file nor directory }
297-307: Improve text overflow handling for error messagesThe CSS sets
text-overflow: ellipsisbut long error messages might still be cut off. Consider improving the handling of long error messages, especially for mobile views..upload-text p { margin: 0; - white-space: nowrap; + white-space: normal; + display: -webkit-box; + -webkit-line-clamp: 2; + -webkit-box-orient: vertical; overflow: hidden; text-overflow: ellipsis; + max-height: 2.8em; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/client/components/Home/TryItFolderUpload.vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: Test (macos-latest, 23.x)
- GitHub Check: Test (macos-latest, 22.x)
- GitHub Check: Test (macos-latest, 21.x)
- GitHub Check: Test (macos-latest, 19.x)
- GitHub Check: Test (windows-latest, 23.x)
- GitHub Check: Test (windows-latest, 22.x)
- GitHub Check: Test (windows-latest, 21.x)
- GitHub Check: Test (windows-latest, 20.x)
- GitHub Check: Test (windows-latest, 19.x)
- GitHub Check: Test (windows-latest, 18.x)
- GitHub Check: Test (windows-latest, 18.0.0)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
website/client/components/Home/TryItFolderUpload.vue (4)
1-6: Good use of Lucide iconsNice work using the recommended Lucide icons (FolderOpen, AlertTriangle) to maintain consistency across devices, as mentioned in the PR feedback.
106-118: Good browser compatibility checkNice implementation of browser capability detection before attempting to use WebKit-specific APIs.
343-347: Good responsive designThe responsive breakpoint for smaller screens is well implemented.
7-29:❓ Verification inconclusive
Consider adding browser compatibility comments
The WebKit FileSystem API interfaces are well-defined, but these APIs aren't universally supported across all browsers. Consider adding a comment about browser compatibility or implement feature detection beyond just the check on line 109.
🏁 Script executed:
#!/bin/bash # Check browser compatibility for directory upload features echo "Checking browser compatibility for directory upload features..." rg -i "webkitdirectory|directory|mozdirectory" --type html # Check if there's any fallback mechanism for browsers that don't support these features rg -i "browser.*(support|compat)" --type ts --type vue -A 5 -B 5Length of output: 441
Action: Enhance Browser Compatibility Documentation
The WebKit FileSystem API interfaces in this component are browser-specific and may not be supported universally. To improve maintainability and clarity for future developers, please add a comment noting which browsers support these APIs and suggest a fallback approach or more robust feature detection than the current check on line 109.
- File:
website/client/components/Home/TryItFolderUpload.vue- Lines: 7-29
Please manually verify that no browser compatibility comments or fallback mechanisms are implemented elsewhere in this component or globally in the application.
There was a problem hiding this comment.
PR Overview
This PR adds a folder upload option to the website by introducing a dedicated folder upload component that compresses files into a ZIP before uploading. Key changes include:
- Implementation of folder upload functionality in TryItFolderUpload.vue.
- Updated icon usage in TryItFileUpload.vue.
- Integration of the new folder upload mode in TryIt.vue.
Reviewed Changes
| File | Description |
|---|---|
| website/client/components/Home/TryItFolderUpload.vue | New folder upload functionality with ZIP packaging logic. |
| website/client/components/Home/TryItFileUpload.vue | Updated file upload component with revised icon usage. |
| website/client/components/Home/TryIt.vue | Extended mode support to include folder uploads and adjusted UI. |
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
website/client/components/Home/TryItFolderUpload.vue (4)
43-53: Add progress feedback for large zip operationsWhen creating zip files from large folders, users might be confused by the lack of feedback during processing.
Consider adding a progress indicator:
// Create ZIP file -const createZipFile = async (files: File[], folderName: string): Promise<File> => { +const createZipFile = async (files: File[], folderName: string, onProgress?: (percent: number) => void): Promise<File> => { const zip = new JSZip(); for (const file of files) { const path = file.webkitRelativePath || file.name; zip.file(path, file); } - const zipBlob = await zip.generateAsync({ type: 'blob' }); + const zipBlob = await zip.generateAsync({ + type: 'blob', + compression: 'DEFLATE', + compressionOptions: { level: 6 }, + onUpdate: (metadata) => { + if (onProgress) onProgress(metadata.percent); + } + }); return new File([zipBlob], `${folderName}.zip`); };Then update the
processFolderfunction to show progress to the user through a new reactive state variable.
108-113: Add JSDoc comments for type guard functionsThe type guard functions lack documentation explaining their purpose and return values.
+/** + * Type guard to check if an entry is a file + * @param entry FileSystemEntry to check + * @returns Boolean indicating if the entry is a file + */ const isFileEntry = (entry: FileSystemEntry): entry is FileSystemFileEntry => { return entry.isFile; } +/** + * Type guard to check if an entry is a directory + * @param entry FileSystemEntry to check + * @returns Boolean indicating if the entry is a directory + */ const isDirectoryEntry = (entry: FileSystemEntry): entry is FileSystemDirectoryEntry => { return entry.isDirectory; }
27-40: Consider more detailed folder validationThe current validation only checks if the folder is empty or exceeds the size limit, but doesn't validate file types or structures.
Consider adding more comprehensive validation:
const validateFolder = (files: File[]): boolean => { errorMessage.value = null; if (files.length === 0) { errorMessage.value = 'The folder is empty.'; return false; } + // Check if the folder contains too many files + if (files.length > 1000) { + errorMessage.value = `The folder contains too many files (${files.length}). Maximum allowed is 1000.`; + return false; + } + + // Check for unsupported file types + const bannedExtensions = ['.exe', '.dll', '.bat', '.sh']; + const hasBannedFiles = files.some(file => { + return bannedExtensions.some(ext => file.name.toLowerCase().endsWith(ext)); + }); + + if (hasBannedFiles) { + errorMessage.value = 'The folder contains files with unsupported extensions (.exe, .dll, .bat, .sh).'; + return false; + } const totalSize = files.reduce((sum, file) => sum + file.size, 0); if (totalSize > MAX_FILE_SIZE) { const sizeMB = (totalSize / (1024 * 1024)).toFixed(1); errorMessage.value = `File size (${sizeMB}MB) exceeds the 10MB limit`; return false; } return true; };
99-105: Improve error handlingThe error handling currently logs to console but doesn't provide specific error details to users.
Consider showing more specific error messages based on the type of error:
try { const files = await collectFilesFromEntry(entry); await processFolder(files, folderName); } catch (error) { console.error('Error processing dropped folder:', error); - errorMessage.value = 'Failed to process the folder. Please try again or use the browse button.'; + if (error instanceof DOMException && error.name === 'SecurityError') { + errorMessage.value = 'Cannot access folder contents due to browser security restrictions.'; + } else if (error instanceof Error) { + errorMessage.value = `Failed to process the folder: ${error.message}. Please try again.`; + } else { + errorMessage.value = 'Failed to process the folder. Please try again or use the browse button.'; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/client/components/Home/TryItFolderUpload.vue(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: Test (macos-latest, 23.x)
- GitHub Check: Test (macos-latest, 22.x)
- GitHub Check: Test (windows-latest, 23.x)
- GitHub Check: Test (windows-latest, 22.x)
- GitHub Check: Test (windows-latest, 21.x)
- GitHub Check: Test (windows-latest, 20.x)
- GitHub Check: Test (windows-latest, 19.x)
- GitHub Check: Test (windows-latest, 18.x)
- GitHub Check: Test (windows-latest, 18.0.0)
- GitHub Check: Build and run (windows-latest, 19.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (7)
website/client/components/Home/TryItFolderUpload.vue (7)
71-72: Add safety check for webkitRelativePathThere's no null check for webkitRelativePath, which might cause errors in some browsers that don't support this property.
const fileArray = Array.from(files); -const folderName = files[0].webkitRelativePath.split('/')[0]; +// Safely extract folder name or fallback to timestamp +const folderName = files[0].webkitRelativePath + ? files[0].webkitRelativePath.split('/')[0] + : `folder_${new Date().getTime()}`;
150-150: Fix incorrect path construction in collectFilesFromEntryThe code mistakenly uses 'entry.name' instead of 'childEntry.name' to build the relative path.
-const newPath = path ? `${path}/${entry.name}` : entry.name; +const newPath = path ? `${path}/${childEntry.name}` : childEntry.name;
184-192: Add browser compatibility notes for directory attributesThe directory, webkitdirectory, and mozdirectory attributes are non-standard HTML attributes.
Add a comment explaining browser compatibility:
<input ref="fileInput" type="file" + <!-- + These are non-standard attributes for enabling directory uploads: + Desktop support: Chrome 30+, Edge 14+, Firefox 50+, Safari 11.1+, Opera 17+ + Mobile support: Not supported on iOS Safari, Chrome for Android, or Firefox for Android + --> directory webkitdirectory mozdirectory class="hidden-input" @change="(e) => handleFileSelect((e.target as HTMLInputElement).files)" />
1-7: LGTM! The imports and constants look good.The component correctly imports JSZip for handling folder compression, Lucide icons for the UI, and defines a reasonable 10MB file size limit.
9-16: Well-defined props and emitsThe component has clearly defined props for loading state and the optional button display, with proper emit typing for the upload event.
175-219: Clean and accessible template structureThe template has a good structure with appropriate feedback for different states (drag active, error, selected folder). The nested conditional rendering provides a good user experience.
221-331: Well-organized and responsive CSSThe styling is clean, uses CSS variables for theming, and includes responsive adjustments for mobile devices. The visual feedback for different states is well-implemented.
|
@PaperBoardOfficial I'll merge this PR! |
|
This time I was not using AI at all (I just took it as a challenge, btw I also put its stream on my yt), so had to read a lot on how to do directory upload in vue. Also, I had to read the ancient jszip documentation as well. So, it took a lot of time and I didn't test the drag and drop. But, yes I'll provide better PR next time. |
|
I can't believe you didn't use AI—it was incredibly well done, including the design! By the way, I'd like to create a release tag and announce this, but since there are no changes to the source code yet, I'll wait until the next source code updates are ready and then publish an npm release with an announcement. |
Issue Link: #377
@yamadashy I have added folder upload functionality. I have created a separate button for it because single input tag cannot have both file and folder upload. We need separate input tag for file upload and separate input tag for folder upload. I have also made the buttons have icons instead of words.
here are the screenshot: A and B
Checklist
npm run testnpm run lint