-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix: update compression to be more aggressive #2962
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdded optional compression options to the in-browser image compression utility and updated the chat input to call it with specific constraints (size, dimensions, quality). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant CI as ChatInput (Web)
participant IU as Image Utility
U->>CI: Drop/Paste image file
CI->>IU: compressImageInBrowser(file, {maxSizeMB:0.2, maxWidthOrHeight:512, quality:0.6})
Note right of IU: Merge options with defaults<br/>Perform compression
IU-->>CI: Compressed image (Blob/File) or error
CI-->>U: Proceed with upload/send or show error
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
| maxSizeMB: compressionOptions?.maxSizeMB ?? 0.2, | ||
| maxWidthOrHeight: compressionOptions?.maxWidthOrHeight ?? 512, | ||
| quality: compressionOptions?.quality ?? 0.6, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There appears to be a discrepancy between the PR description and implementation. The PR description states that the default compression values should be:
maxSizeMB: 2maxWidthOrHeight: 2048quality: 0.8
However, the implementation sets more aggressive defaults:
maxSizeMB: 0.2maxWidthOrHeight: 512quality: 0.6
These more aggressive defaults will result in smaller file sizes but potentially lower image quality than what was specified in the PR description. Consider aligning the implementation with the documented defaults to ensure consistent behavior.
| maxSizeMB: compressionOptions?.maxSizeMB ?? 0.2, | |
| maxWidthOrHeight: compressionOptions?.maxWidthOrHeight ?? 512, | |
| quality: compressionOptions?.quality ?? 0.6, | |
| maxSizeMB: compressionOptions?.maxSizeMB ?? 2, | |
| maxWidthOrHeight: compressionOptions?.maxWidthOrHeight ?? 2048, | |
| quality: compressionOptions?.quality ?? 0.8, |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
compressImageInBrowserinimage.tsupdated for more aggressive image compression with new default settings and optional configuration.compressImageInBrowserinimage.tsnow uses more aggressive default compression options:maxSizeMBreduced to 0.2,maxWidthOrHeightreduced to 512, andqualityset to 0.6.compressionOptionsparameter tocompressImageInBrowserfor flexible tuning of compression settings.This description was created by
for cbc3f98. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit