Fix/web image upload cropper#339
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
Warning Review limit reached
More reviews will be available in 41 minutes and 33 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces client-side image cropping utilities and a reusable modal component, refactors avatar and store image upload flows from immediate uploads to draft-based workflows, improves upload response parsing to handle multiple URL field names, and integrates the new cropping workflows into shopper and store settings screens. ChangesImage cropping and draft-based uploads
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 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 unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
apps/web/src/components/store-settings/StoreImageUpload.tsx (1)
24-31: ⚡ Quick winRename the boolean prop to
isDisabled.This introduces a new boolean field that doesn't follow the repo naming rule. Keep the native
disabledattribute at the DOM boundary, but exposeisDisabledfrom this component API.As per coding guidelines, "All boolean field names must be prefixed with 'is', 'has', 'can', or 'should' (e.g. isActive, hasVariants)".
Also applies to: 41-41
🤖 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 `@apps/web/src/components/store-settings/StoreImageUpload.tsx` around lines 24 - 31, The component prop named disabled must be renamed to isDisabled in the Props interface and component API (update interface Props and the StoreImageUpload component's props destructure) while keeping the native DOM attribute on the actual input/button by passing disabled={isDisabled} when rendering; update every occurrence inside StoreImageUpload (and the prop passed down at the render site around line 41) to use isDisabled, adjust any prop forwarding, TypeScript types and call sites inside this file accordingly, and ensure external usages are updated to the new isDisabled prop name.
🤖 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 `@apps/web/src/app/`(store)/store/settings/StoreSettingsClient.tsx:
- Around line 43-49: The type guard isStoreImageUploadError is too broad (any
Error has a string message) so update it to only match the upload-specific error
shape used by the upload code path: check that value is an object and then
either that v.code is a string AND is one of the known upload error codes (e.g.
'IMAGE_TOO_LARGE', 'INVALID_IMAGE_TYPE', 'IMAGE_UPLOAD_FAILED' — replace with
the actual codes used by the upload API) or that it contains an upload-specific
field (e.g. v.upload?.error or v.uploadError) that your upload routine returns;
change isStoreImageUploadError to use that explicit set/field check so errors
thrown by updateStoreProfile or fetchOwnerStoreFull are not misclassified as
image-upload failures.
- Around line 84-89: The cleanup currently revokes both logoDraft?.previewUrl
and bannerDraft?.previewUrl whenever either changes; change this to revoke only
the preview URL for the draft that actually changed by using two separate
effects (one for logoDraft and one for bannerDraft) or by tracking the previous
previewUrl per draft (e.g., prevLogoRef/prevBannerRef) and revoking only that
value in each effect's cleanup; locate the current useEffect in
StoreSettingsClient.tsx and split it into two useEffect blocks (or add per-draft
prev refs) that call revokeObjectUrl(logoDraft?.previewUrl) only from the logo
effect and revokeObjectUrl(bannerDraft?.previewUrl) only from the banner effect
so an unchanged draft's previewUrl is not revoked.
In `@apps/web/src/components/image/ImageCropperModal.tsx`:
- Around line 43-47: The component currently retains previous session state
(crop, zoom, pixelCrop, error) which can be applied to a new image; update the
component to reset these when the modal input changes by adding an effect that
watches props used to open/replace the image (e.g., imageSrc and open) and on
change calls setCrop({x:0,y:0}), setZoom(1), setPixelCrop(null) and
setError(null); ensure the same reset logic covers any other stateful values
referenced (processing if needed) so the new modal starts with a clean crop
state before onCropComplete runs.
In `@apps/web/src/components/store-settings/StoreImageUpload.tsx`:
- Around line 44-75: cropSource blob URLs are revoked in
handlePick/handleCropCancel/handleCropConfirm but not when the component
unmounts; add a useEffect that watches cropSource and returns a cleanup which
calls revokeObjectUrl(cropSource) so any active URL is revoked on unmount or
when cropSource changes. Update the component to include this effect alongside
the existing state and handlers (cropSource, handlePick, handleCropCancel,
handleCropConfirm) to ensure no blob URLs leak.
In `@apps/web/src/lib/image-crop.ts`:
- Around line 30-31: The validation message is hardcoded to "5MB" while the
check uses the dynamic maxBytes parameter; update the error string produced when
file.size > maxBytes to reflect maxBytes (e.g., compute a human-readable value
from maxBytes like MB) instead of the literal "5MB" so callers with different
limits see the correct size in the message; locate the size check that compares
file.size to maxBytes and replace the static text with a formatted
representation of maxBytes.
---
Nitpick comments:
In `@apps/web/src/components/store-settings/StoreImageUpload.tsx`:
- Around line 24-31: The component prop named disabled must be renamed to
isDisabled in the Props interface and component API (update interface Props and
the StoreImageUpload component's props destructure) while keeping the native DOM
attribute on the actual input/button by passing disabled={isDisabled} when
rendering; update every occurrence inside StoreImageUpload (and the prop passed
down at the render site around line 41) to use isDisabled, adjust any prop
forwarding, TypeScript types and call sites inside this file accordingly, and
ensure external usages are updated to the new isDisabled prop name.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1fc079e8-6fa4-404c-add6-faca51a4317b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
apps/web/package.jsonapps/web/src/app/(shopper)/buyer/settings/SettingsClient.tsxapps/web/src/app/(store)/store/settings/StoreSettingsClient.tsxapps/web/src/components/image/ImageCropperModal.tsxapps/web/src/components/store-settings/StoreImageUpload.tsxapps/web/src/lib/image-crop.tsapps/web/src/lib/store-settings.tsapps/web/src/lib/user.ts
What does this PR do?
This PR fixes profile/store image upload behavior and adds a reusable image cropping flow for Twizrr profile and store images. Users can now crop user profile photos, store logos, and store banner images before upload, preview them locally, upload through the backend
/upload/imageendpoint, and persist the returned image URL when saving profile/store settings.Type of change
Area affected
How to test this
Checkout the branch and run web checks:
Start the app:
pnpm run dev
Smoke-test image flows:
/buyer/settings
/store/settings
Expected result: selecting a profile/store image opens a cropper, cropped previews appear correctly, uploads go through the backend /upload/image endpoint, and the returned image URL is persisted only when the user saves settings.
Pre-commit checklist
console.logleft in production code.envfiles committedanytypes addeddb pushScreenshots
Notes for reviewer
Upload behavior:
Uploads still go through backend:
POST /upload/image
No direct frontend Cloudinary uploads
No Cloudinary secrets exposed to frontend
Cropped image is converted before upload
Local preview updates before saving
Final persisted URL is saved through existing profile/store settings update flow
Bug root cause:
The previous frontend behavior uploaded images immediately on file select but did not reliably persist the returned upload URL to PUT /users/me or PUT /stores/me during Save.
This made the upload flow feel like profile/store images were lost or not updating.
This PR keeps the cropped/uploaded image URL in form state and persists it during Save.
Summary by CodeRabbit
New Features
Improvements