feat(vault): Vault File Browser UI — New menu, upload modals & mock service#125
feat(vault): Vault File Browser UI — New menu, upload modals & mock service#125ajinkyap9 wants to merge 3 commits into
Conversation
|
@ajinkyap9 please attach relavent screenshots |
|
@tarinagarwal Attached now you can check and if any change required let me know |
|
@tarinagarwal please can you review the pr |
|
@tarinagarwal can you review asap |
There was a problem hiding this comment.
Changes Requested 🐈
This PR adds a new Vault file browser UI with sidebar, file/folder views, and upload modals for the frontend. The implementation is readable and functional, but documentation is lacking for new services and API endpoints. No critical bugs or security issues were found, but documentation improvements are needed for maintainability.
There are a few things I'd like to see addressed before we merge this:
Before merging
- Add JSDoc comments to all exported functions and types in vaultService.ts and related files.
- Document all new API endpoints in server/routes/vault.js, including method, path, request/response, and authentication.
- Update README and .env.example to reflect the new Vault feature and its configuration.
Findings breakdown (25 total)
2 high / 16 medium / 3 low / 4 info
Confidence: 90%
🔗 View Full Review Report — detailed findings, severity breakdown, and agent analysis
Reviewed by Looks Good To Meow — AI-powered code review
💬 You can interact with me directly in this PR:
@tarin-lgtm fix [any constraints]@tarin-lgtm explain [your question]@tarin-lgtm improve [focus area]@tarin-lgtm test [what to focus on]
| @@ -0,0 +1,188 @@ | |||
| export type VaultItemBase = { | |||
There was a problem hiding this comment.
Add JSDoc comments to all exported types and functions, including @param and @returns for functions, and a description for each type.
documentation
| @@ -0,0 +1,296 @@ | |||
| import express from "express"; | |||
There was a problem hiding this comment.
Add JSDoc-style or block comments above each route handler describing the endpoint, HTTP method, path, expected request body, response schema, authentication requirements, and possible error responses.
documentation
| </div> | ||
| <div className="flex-1 bg-[#0f1724] rounded-lg p-4"> | ||
| <div className="flex items-center justify-between mb-4"> | ||
| <VaultBreadcrumb currentFolderId={currentFolderId} onNavigate={setCurrentFolderId} /> |
There was a problem hiding this comment.
🔍 Medium — The storage usage display is hardcoded as '120MB / 2GB'. This is misleading and not dynamic.
Fetch and display the actual storage usage from the backend or mock service instead of hardcoding the values.
readability
| <input value={name} onChange={(e) => setName(e.target.value)} className="w-full p-2 rounded bg-[#0b1220] border border-slate-700 text-slate-200" placeholder="Folder name" /> | ||
| <div className="mt-4 flex justify-end gap-2"> | ||
| <button className="px-3 py-1 border rounded" onClick={onClose}>Cancel</button> | ||
| <button className="px-3 py-1 bg-alien-green text-black rounded" onClick={() => { onCreate(name); setName(""); }}>Create</button> |
There was a problem hiding this comment.
🔍 Medium — The 'Create' button allows creating a folder with an empty name, which can lead to confusing or invalid folder entries.
Disable the 'Create' button or prevent submission when the folder name is empty or only whitespace.
readability
| <div className="mt-4 flex items-center justify-center gap-4"> | ||
| <input id="upload-files-input" type="file" multiple className="hidden" onChange={(e) => setFiles(e.target.files ? Array.from(e.target.files) : null)} /> | ||
| <label htmlFor="upload-files-input" className="px-3 py-2 bg-slate-800 text-slate-200 rounded cursor-pointer">Choose files</label> | ||
| <button className="px-4 py-2 bg-alien-green text-black rounded" onClick={() => { onUpload(files); }}>Upload</button> |
There was a problem hiding this comment.
🔍 Medium — The 'Upload' button is always enabled, allowing upload with no files selected. This can cause errors or confusion.
Disable the 'Upload' button when no files are selected.
readability
| import React, { useState } from "react"; | ||
| import DropZone from "../DropZone"; | ||
|
|
||
| const UploadFileModal: React.FC<{ open: boolean; onClose: () => void; onUpload: (files: File[] | FileList | null) => void }> = ({ open, onClose, onUpload }) => { |
There was a problem hiding this comment.
🔍 Medium — UploadFileModal is an exported React component but lacks a JSDoc comment describing its purpose and props.
Add a JSDoc comment above UploadFileModal describing its purpose and the props it accepts.
documentation
| @@ -0,0 +1,39 @@ | |||
| import React, { useState } from "react"; | |||
|
|
|||
| const UploadFolderModal: React.FC<{ open: boolean; onClose: () => void; onUpload: (files: File[] | FileList | null) => void }> = ({ open, onClose, onUpload }) => { | |||
There was a problem hiding this comment.
🔍 Medium — UploadFolderModal is an exported React component but lacks a JSDoc comment describing its purpose and props.
Add a JSDoc comment above UploadFolderModal describing its purpose and the props it accepts.
documentation
| return ( | ||
| <div className="pl-2"> | ||
| <div className="flex items-center gap-2 py-1 cursor-pointer hover:bg-slate-800 rounded" onClick={() => { setOpen(!open); onNavigate(node.id); }}> | ||
| <div className="w-5 h-5 bg-slate-600 rounded flex items-center justify-center text-xs">F</div> |
There was a problem hiding this comment.
💡 Suggestion — The folder icon is rendered as a literal 'F' character, which is not visually descriptive and may confuse users.
Replace the 'F' with an actual folder icon (e.g., an SVG or emoji like 📁) for better clarity and consistency with other parts of the UI.
readability
| <div className="text-sm text-slate-200 truncate">{item.name}</div> | ||
| <div className="text-xs text-slate-500 flex justify-between"> | ||
| <span>{isFolder ? "Folder" : `${(item as FileItem).size} bytes`}</span> | ||
| <button className="text-red-400" onClick={(e) => { e.stopPropagation(); onRequestDelete(item.id); }}>Delete</button> |
There was a problem hiding this comment.
💡 Suggestion — The 'Delete' button is rendered for both files and folders, but the label does not clarify what is being deleted. This could be confusing in a mixed grid view.
Consider updating the button label or adding a tooltip to clarify whether a file or folder will be deleted.
readability
| </div> | ||
| <div className="text-xs text-slate-400 flex gap-4 items-center"> | ||
| <div>{isFolder ? "Folder" : `${(item as FileItem).size} bytes`}</div> | ||
| <button className="text-red-400" onClick={() => onRequestDelete(item.id)}>Delete</button> |
There was a problem hiding this comment.
💡 Suggestion — The 'Delete' button is rendered for both files and folders, but the label does not clarify what is being deleted. This could be confusing in a mixed list view.
Consider updating the button label or adding a tooltip to clarify whether a file or folder will be deleted.
readability
Description
Add a Google Drive / Dropbox–style Vault file browser UI and upload workflow for the client (frontend-only, development mode).
Key additions
VaultPage.tsxclient/src/components/vault/VaultSidebar.tsxFileGrid.tsxFileList.tsxVaultBreadcrumb.tsxwebkitdirectoryDropZonecomponentUploadFileModal&UploadFolderModalclient/src/services/vaultService.tsgetChildrengetFolderTreecreateFolderuploadFilerenamedeletegetPath/vault/*route inApp.tsx🔗 Related Issue
Closes: #72
🏷️ Type of Change
Screenshots (if applicable)
Checklist
Testing
How I tested locally