feat: add drag and drop upload#2
Merged
Merged
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR implements a file upload-to-visualizer navigation flow. New app constants and a formatBytes util, a draggable Upload component with validation and simulated progress, route configuration, and home page wiring enable uploading an image and navigating to /visualizer/:id. ChangesUpload Flow Integration
Sequence DiagramsequenceDiagram
participant User
participant Home
participant UploadComponent
participant FileReader
participant Visualizer
User->>Home: Visit home page
User->>UploadComponent: Drop or select file
UploadComponent->>FileReader: readAsDataURL(file)
FileReader->>UploadComponent: base64 encoded file
UploadComponent->>UploadComponent: Simulate progress
UploadComponent->>Home: onComplete(base64)
Home->>Home: handleUploadComplete called
Home->>Visualizer: navigate(/visualizer/${timestamp})
Visualizer->>User: Render visualizer page (placeholder)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
lib/constants.ts (1)
15-15: ⚡ Quick winRemove unused constant.
PROGRESS_STEPis defined but never referenced in the provided code, includingcomponents/Upload.tsxwhich uses the other progress constants.♻️ Remove the unused export
-export const PROGRESS_STEP = 5;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/constants.ts` at line 15, Remove the unused exported constant PROGRESS_STEP from lib/constants.ts: locate the exported symbol PROGRESS_STEP and delete its declaration and export so only used progress constants remain (ensure components/Upload.tsx and other modules still import the remaining constants and run a quick project-wide search to confirm there are no remaining references to PROGRESS_STEP).components/Upload.tsx (2)
27-39: ⚖️ Poor tradeoffConsider reflecting actual file read progress.
The progress bar animation starts only after the file has been completely read into memory (inside
onload). For large files, there may be a noticeable delay with no feedback, followed by a simulated progress bar. Consider usingFileReader.onprogressto track the actual read operation.📊 Track actual read progress
const reader = new FileReader(); +reader.onprogress = (e) => { + if (e.lengthComputable) { + setProgress(Math.round((e.loaded / e.total) * 90)); // Reserve 90-100% for post-processing + } +}; reader.onerror = () => { setFile(null); setProgress(0); }; reader.onload = (e) => { const base64 = e.target?.result as string; - const interval = setInterval(() => { - setProgress((prev) => { - const next = prev + PROGRESS_INCREMENT; - if (next >= 100) { - clearInterval(interval); - setTimeout(() => { - props.onComplete?.(base64); - }, REDIRECT_DELAY_MS); - return 100; - } - return next; - }); - }, PROGRESS_INTERVAL_MS); + setProgress(100); + setTimeout(() => { + props.onComplete?.(base64); + }, REDIRECT_DELAY_MS); };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/Upload.tsx` around lines 27 - 39, The progress UI is simulated after FileReader.onload completes, causing no feedback during long reads; instead, attach a FileReader.onprogress handler to the same FileReader used in the component (the one whose onload currently sets base64 and triggers setProgress via setInterval), and inside onprogress call setProgress(Math.min(100, Math.floor((event.loaded / event.total) * 100))) to reflect real bytes-read; keep the existing onload logic to finalize base64 and call props.onComplete(base64) after REDIRECT_DELAY_MS, and remove or stop the simulated setInterval (PROGRESS_INCREMENT / PROGRESS_INTERVAL_MS) when onprogress reaches 100 or onload fires to avoid conflicting updates.
86-86: ⚡ Quick winUse MIME types in accept attribute.
File extensions (
.jpg,.png) are less reliable than MIME types for theacceptattribute. MIME types provide better cross-browser compatibility and more precise validation.♻️ Switch to MIME types
-accept=".jpg,.jpeg,.png" +accept="image/jpeg,image/png"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/Upload.tsx` at line 86, The file's file-input in the Upload component currently uses file extensions in the accept attribute; update the input element (in the Upload component / JSX where accept=".jpg,.jpeg,.png" is set) to use MIME types instead — e.g., replace the extension list with "image/jpeg,image/png" (or add other required image/* MIME types) so browsers use precise MIME-based filtering and validation.app/routes/home.tsx (1)
18-20: ⚡ Quick winRemove unnecessary
asyncand unused parameter.The function is marked
asyncbut performs no asynchronous operations. Additionally, thebase64parameter is accepted but never used.♻️ Simplify the handler
- const handleUploadComplete = async (base64: string) => { + const handleUploadComplete = () => { navigate(`/visualizer/${Date.now().toString()}`); }If you plan to use the base64 data in the future (e.g., passing it via router state), keep the parameter but pass it along:
- const handleUploadComplete = async (base64: string) => { - navigate(`/visualizer/${Date.now().toString()}`); + const handleUploadComplete = (base64: string) => { + navigate(`/visualizer/${Date.now().toString()}`, { state: { base64 } }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/routes/home.tsx` around lines 18 - 20, The handler handleUploadComplete is marked async and accepts an unused base64 parameter; remove the unnecessary async and either drop the parameter (change to const handleUploadComplete = () => { navigate(`/visualizer/${Date.now().toString()}`); }) or, if you intend to use the base64 later, keep the parameter and pass it via router state when navigating (e.g., navigate(`/visualizer/${Date.now().toString()}`, { state: { base64 } })). Ensure the function signature and body are consistent with the chosen approach.app/routes/visualizer.$id.tsx (1)
3-7: ⚡ Quick winAdd proper TypeScript types for the route component.
The component should follow React Router 7 conventions by importing and using the generated
Routetype from the+typesfile, similar to the pattern inapp/routes/home.tsx.📘 Add proper route types
import React from 'react'; +import type { Route } from "./+types/visualizer.$id"; -function VisualizerId(props) { +function VisualizerId({ params }: Route.ComponentProps) { + // Access the id with: params.id return ( <div></div> ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/routes/visualizer`.$id.tsx around lines 3 - 7, Import the generated Route type from the +types file and type the route component accordingly: add `import type { Route } from "./+types"` and change the component to a typed declaration (e.g., `const VisualizerId: Route = (props) => { ... }`) ensuring it is the default export; update the function signature of VisualizerId to use the Route type so props/params are correctly typed per React Router 7 conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/Upload.tsx`:
- Around line 60-63: Dropped files are passed directly to processFile without
validating type or size; update the drag-drop handler (where droppedFile is
read) and the file input handler (the other block referenced) to perform checks
before calling processFile: verify file.size <= 50 * 1024 * 1024 and that
file.type (and as a fallback the file.name extension) matches the allowed MIME
types/extensions you expect (same list as the accept prop), and if invalid, do
not call processFile and instead surface a clear error (e.g., via setError or
your existing notification/UI error handler) so users cannot bypass client-side
restrictions.
- Line 16: The code uses the AuthContext type in the
useOutletContext<AuthContext>() call but never imports it; add an import for the
AuthContext type at the top of components/Upload.tsx (the same module that
exports AuthContext in your codebase) so TypeScript knows the type for
useOutletContext<AuthContext>(); ensure the import is a type import (if using
TS) and references the exact exported name AuthContext.
- Around line 23-42: Add a FileReader.onerror handler alongside the existing
reader.onload to handle read failures: in the handler clear any running interval
created with PROGRESS_INTERVAL_MS, reset progress via setProgress(0) (or to an
error state), call a failure callback if provided (e.g., props.onError or a new
prop), and log or surface the error so the component doesn't remain in a
selected-but-stalled state; ensure this uses the same reader reference and ties
into the existing props.onComplete/REDIRECT_DELAY_MS flow so failed reads don't
trigger the success timeout.
- Around line 27-39: The interval created inside the progress logic (the
setInterval that updates setProgress using PROGRESS_INCREMENT and
PROGRESS_INTERVAL_MS and calls props.onComplete/REDIRECT_DELAY_MS) is never
cleared if the component unmounts; store the interval id in a ref (e.g.,
progressIntervalRef) or a variable scoped to the component and add a cleanup
function in the surrounding useEffect to call
clearInterval(progressIntervalRef.current) and also clearTimeout for the
redirect timeout if set, ensuring you clear the interval when progress reaches
100 as before and on component unmount to avoid state updates on an unmounted
component.
- Line 100: Resolve the contradictory UI text by sourcing the true enforced
limit from the central place used by upload validation (e.g., the constant like
MAX_UPLOAD_SIZE or UPLOAD_MAX_BYTES) and replace the hardcoded strings in the
Upload component (Upload) and the home route's exported component (Home) with a
formatted display of that constant (e.g., formatBytes(MAX_UPLOAD_SIZE) or
getMaxUploadSizeDisplay()). In practice: confirm the actual enforcement value in
your upload/validation code, create or reuse a single constant/utility for the
human-readable limit, and update the message in Upload and the home component to
reference that shared value instead of hardcoded "50MB"/"10MB".
---
Nitpick comments:
In `@app/routes/home.tsx`:
- Around line 18-20: The handler handleUploadComplete is marked async and
accepts an unused base64 parameter; remove the unnecessary async and either drop
the parameter (change to const handleUploadComplete = () => {
navigate(`/visualizer/${Date.now().toString()}`); }) or, if you intend to use
the base64 later, keep the parameter and pass it via router state when
navigating (e.g., navigate(`/visualizer/${Date.now().toString()}`, { state: {
base64 } })). Ensure the function signature and body are consistent with the
chosen approach.
In `@app/routes/visualizer`.$id.tsx:
- Around line 3-7: Import the generated Route type from the +types file and type
the route component accordingly: add `import type { Route } from "./+types"` and
change the component to a typed declaration (e.g., `const VisualizerId: Route =
(props) => { ... }`) ensuring it is the default export; update the function
signature of VisualizerId to use the Route type so props/params are correctly
typed per React Router 7 conventions.
In `@components/Upload.tsx`:
- Around line 27-39: The progress UI is simulated after FileReader.onload
completes, causing no feedback during long reads; instead, attach a
FileReader.onprogress handler to the same FileReader used in the component (the
one whose onload currently sets base64 and triggers setProgress via
setInterval), and inside onprogress call setProgress(Math.min(100,
Math.floor((event.loaded / event.total) * 100))) to reflect real bytes-read;
keep the existing onload logic to finalize base64 and call
props.onComplete(base64) after REDIRECT_DELAY_MS, and remove or stop the
simulated setInterval (PROGRESS_INCREMENT / PROGRESS_INTERVAL_MS) when
onprogress reaches 100 or onload fires to avoid conflicting updates.
- Line 86: The file's file-input in the Upload component currently uses file
extensions in the accept attribute; update the input element (in the Upload
component / JSX where accept=".jpg,.jpeg,.png" is set) to use MIME types instead
— e.g., replace the extension list with "image/jpeg,image/png" (or add other
required image/* MIME types) so browsers use precise MIME-based filtering and
validation.
In `@lib/constants.ts`:
- Line 15: Remove the unused exported constant PROGRESS_STEP from
lib/constants.ts: locate the exported symbol PROGRESS_STEP and delete its
declaration and export so only used progress constants remain (ensure
components/Upload.tsx and other modules still import the remaining constants and
run a quick project-wide search to confirm there are no remaining references to
PROGRESS_STEP).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2b4e1ab5-6b59-4e4d-9b80-c63f8749fea9
📒 Files selected for processing (6)
.gitignoreapp/routes.tsapp/routes/home.tsxapp/routes/visualizer.$id.tsxcomponents/Upload.tsxlib/constants.ts
Comment on lines
+23
to
+42
| const reader = new FileReader(); | ||
| reader.onload = (e) => { | ||
| const base64 = e.target?.result as string; | ||
|
|
||
| const interval = setInterval(() => { | ||
| setProgress((prev) => { | ||
| const next = prev + PROGRESS_INCREMENT; | ||
| if (next >= 100) { | ||
| clearInterval(interval); | ||
| setTimeout(() => { | ||
| props.onComplete?.(base64); | ||
| }, REDIRECT_DELAY_MS); | ||
| return 100; | ||
| } | ||
| return next; | ||
| }); | ||
| }, PROGRESS_INTERVAL_MS); | ||
| }; | ||
| reader.readAsDataURL(file); | ||
| }; |
Contributor
There was a problem hiding this comment.
Add error handling for FileReader.
The FileReader operations can fail (e.g., for corrupted files or permission issues), but there is no onerror handler. Failed reads will leave the component in an inconsistent state with a selected file but no progress.
🛡️ Add error handling
const reader = new FileReader();
+reader.onerror = () => {
+ setFile(null);
+ setProgress(0);
+ // Consider showing an error message to the user
+};
reader.onload = (e) => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const reader = new FileReader(); | |
| reader.onload = (e) => { | |
| const base64 = e.target?.result as string; | |
| const interval = setInterval(() => { | |
| setProgress((prev) => { | |
| const next = prev + PROGRESS_INCREMENT; | |
| if (next >= 100) { | |
| clearInterval(interval); | |
| setTimeout(() => { | |
| props.onComplete?.(base64); | |
| }, REDIRECT_DELAY_MS); | |
| return 100; | |
| } | |
| return next; | |
| }); | |
| }, PROGRESS_INTERVAL_MS); | |
| }; | |
| reader.readAsDataURL(file); | |
| }; | |
| const reader = new FileReader(); | |
| reader.onerror = () => { | |
| setFile(null); | |
| setProgress(0); | |
| // Consider showing an error message to the user | |
| }; | |
| reader.onload = (e) => { | |
| const base64 = e.target?.result as string; | |
| const interval = setInterval(() => { | |
| setProgress((prev) => { | |
| const next = prev + PROGRESS_INCREMENT; | |
| if (next >= 100) { | |
| clearInterval(interval); | |
| setTimeout(() => { | |
| props.onComplete?.(base64); | |
| }, REDIRECT_DELAY_MS); | |
| return 100; | |
| } | |
| return next; | |
| }); | |
| }, PROGRESS_INTERVAL_MS); | |
| }; | |
| reader.readAsDataURL(file); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/Upload.tsx` around lines 23 - 42, Add a FileReader.onerror handler
alongside the existing reader.onload to handle read failures: in the handler
clear any running interval created with PROGRESS_INTERVAL_MS, reset progress via
setProgress(0) (or to an error state), call a failure callback if provided
(e.g., props.onError or a new prop), and log or surface the error so the
component doesn't remain in a selected-but-stalled state; ensure this uses the
same reader reference and ties into the existing
props.onComplete/REDIRECT_DELAY_MS flow so failed reads don't trigger the
success timeout.
| ): "Sign in or sign up with Puter to upload"} | ||
| </p> | ||
| <p className="help"> | ||
| Maximum file size 50MB. |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for file size limit mentions
rg -n "MB|file size" --type tsx --type ts -C 2Repository: L0GI0/Planly-AI
Length of output: 87
🏁 Script executed:
#!/bin/bash
# Search for file size limit mentions in TypeScript/TSX files
rg -n "MB|file size" -t ts -t js -C 2 --max-count 20Repository: L0GI0/Planly-AI
Length of output: 700
🏁 Script executed:
#!/bin/bash
# Also check if the specific files exist and read them directly
echo "=== Checking components/Upload.tsx ==="
if [ -f "components/Upload.tsx" ]; then
echo "File found, reading around line 100:"
sed -n '95,105p' components/Upload.tsx
else
echo "File not found, searching for Upload files:"
find . -name "*Upload*" -type f
fi
echo ""
echo "=== Checking app/routes/home.tsx or home.ts ==="
if [ -f "app/routes/home.tsx" ]; then
echo "File found, reading around line 55:"
sed -n '50,60p' app/routes/home.tsx
elif [ -f "app/routes/home.ts" ]; then
echo "File found as .ts, reading around line 55:"
sed -n '50,60p' app/routes/home.ts
else
echo "File not found, searching for home files:"
find . -name "*home*" -type f
fiRepository: L0GI0/Planly-AI
Length of output: 1041
🏁 Script executed:
#!/bin/bash
# Look for file size validation in Upload component
echo "=== Checking Upload.tsx for validation logic ==="
wc -l components/Upload.tsx
echo ""
echo "=== Looking for size-related validation ==="
rg -n "50|10|fileSize|maxSize|MAX_SIZE" components/Upload.tsx -C 3Repository: L0GI0/Planly-AI
Length of output: 1771
🏁 Script executed:
#!/bin/bash
# Check for config files or environment variables with size limits
echo "=== Searching for configuration files with size limits ==="
find . -type f \( -name "*.env*" -o -name "*.config.*" -o -name "constants*" -o -name "config*" \) | head -20
echo ""
echo "=== Searching for 50 or 10 MB references in config-like files ==="
rg -l "50|10" --type json --type yaml --type toml 2>/dev/null | head -10Repository: L0GI0/Planly-AI
Length of output: 278
🏁 Script executed:
#!/bin/bash
# Read the constants file
echo "=== Checking lib/constants.ts ==="
cat -n lib/constants.ts | head -50
echo ""
echo "=== Full Upload.tsx to find validation logic ==="
cat -n components/Upload.tsx | grep -E "File|file|size|Size|MAX|upload|validate" -A 2 -B 2Repository: L0GI0/Planly-AI
Length of output: 5534
🏁 Script executed:
#!/bin/bash
# Search for actual file size validation across the codebase
echo "=== Looking for file validation/checking logic ==="
rg -n "new File|FileReader|file\.size|size >|size <|maxSize|MAX_FILE" -C 2Repository: L0GI0/Planly-AI
Length of output: 389
Resolve conflicting file size limit messages in UI.
components/Upload.tsx line 100 displays "Maximum file size 50MB", but app/routes/home.tsx line 55 states "up to 10MB". Users will see contradictory information depending on which page they view. Align both messages with the actual enforced limit.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/Upload.tsx` at line 100, Resolve the contradictory UI text by
sourcing the true enforced limit from the central place used by upload
validation (e.g., the constant like MAX_UPLOAD_SIZE or UPLOAD_MAX_BYTES) and
replace the hardcoded strings in the Upload component (Upload) and the home
route's exported component (Home) with a formatted display of that constant
(e.g., formatBytes(MAX_UPLOAD_SIZE) or getMaxUploadSizeDisplay()). In practice:
confirm the actual enforcement value in your upload/validation code, create or
reuse a single constant/utility for the human-readable limit, and update the
message in Upload and the home component to reference that shared value instead
of hardcoded "50MB"/"10MB".
This was referenced May 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Chores