Skip to content

fix: Banner uploader fix error handling and reduce image size jump#21903

Merged
anikdhabal merged 8 commits into
mainfrom
fix/image-uploader
Jun 19, 2025
Merged

fix: Banner uploader fix error handling and reduce image size jump#21903
anikdhabal merged 8 commits into
mainfrom
fix/image-uploader

Conversation

@alishaz-polymath
Copy link
Copy Markdown
Member

@alishaz-polymath alishaz-polymath commented Jun 18, 2025

What does this PR do?

Adds support for JPEG images in base64 conversion, leading to better adjusted images. Earlier we were forcing base64 png even on jpeg uploads where it would result in a jump of even 30 times in size, in complex images.

Visual Demo (For contributors especially)

A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).

Video Demo (if applicable):

  • Show screen recordings of the issue or feature.
  • Demonstrate how to reproduce the issue, the behavior before and after the change.

Image Demo (if applicable):

  • Add side-by-side screenshots of the original and updated change.
  • Highlight any significant change(s).

Mandatory Tasks (DO NOT REMOVE)

  • I have self-reviewed the code (A decent size PR without self-review might be rejected).
  • I have updated the developer docs in /docs if this PR makes changes that would require a documentation change. If N/A, write N/A here and check the checkbox.
  • I confirm automated tests are in place that prove my fix is effective or that my feature works.

How should this be tested?

  • Are there environment variables that should be set?
  • What are the minimal test data to have?
  • What is expected (happy path) to have (input and output)?
  • Any other important info that could help to test that PR

Checklist

  • I haven't read the contributing guide
  • My code doesn't follow the style guidelines of this project
  • I haven't commented my code, particularly in hard-to-understand areas
  • I haven't checked if my changes generate no new warnings

Summary by cubic

Improved the banner image uploader to support JPEG images, reduce large file size jumps, and show clearer error messages when uploads fail.

  • Bug Fixes
    • Preserves original image format (JPEG or PNG) during upload to prevent unnecessary file size increases.
    • Adds better error handling and user feedback for oversized images.

@alishaz-polymath alishaz-polymath requested review from a team as code owners June 18, 2025 12:56
@linear
Copy link
Copy Markdown

linear Bot commented Jun 18, 2025

@github-actions github-actions Bot added the 🧹 Improvements Improvements to existing features. Mostly UX/UI label Jun 18, 2025
@keithwillcode keithwillcode added core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO labels Jun 18, 2025
@dosubot dosubot Bot added the 🐛 bug Something isn't working label Jun 18, 2025
@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 18, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
cal ⬜️ Ignored (Inspect) Visit Preview Jun 19, 2025 3:18am
cal-eu ⬜️ Ignored (Inspect) Visit Preview Jun 19, 2025 3:18am

@vercel vercel Bot temporarily deployed to Preview – cal June 18, 2025 12:57 Inactive
@vercel vercel Bot temporarily deployed to Preview – api June 18, 2025 12:57 Inactive
const mutation = trpc.viewer.organizations.update.useMutation({
onError: (err) => {
// Handle JSON parsing errors from body size limit exceeded
if (err.message.includes("Unexpected token") && err.message.includes("Body excee")) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The unhandled error is: "Unexpected token 'B', "Body excee"... is not valid JSON"
So this takes care of that.

Comment on lines -227 to +234
return canvas.toDataURL("image/png");
// Use original format with quality setting for JPEG
return canvas.toDataURL(originalFormat, originalFormat === "image/jpeg" ? 0.6 : undefined);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't force convert to png, which was the primary cause for the crazy size jump.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cubic found 1 issue across 5 files. Review it in cubic.dev

React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.

Comment thread packages/features/ee/organizations/pages/settings/profile.tsx Outdated
Copy link
Copy Markdown
Contributor

@anikdhabal anikdhabal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Good

@anikdhabal anikdhabal enabled auto-merge (squash) June 18, 2025 13:12
@heykandoi
Copy link
Copy Markdown

I may be wrong here, but doesn't this limit the image size to just 1mb?

@alishaz-polymath
Copy link
Copy Markdown
Member Author

I may be wrong here, but doesn't this limit the image size to just 1mb?

No, that part never changed. We've always had that limit, but here's what is happening behind the scenes:

  1. A user uploads an image, the limit of the image selected by the user in terms of size there is 5mb.
  2. The uploaded image is then cropped (during upload) and encoded into base64. It automatically reduces in size considerably here now.
  3. When we proceed with the mutation (click save), there the limit is 1mb and with these code changes, that should almost always satisfy.

@heykandoi
Copy link
Copy Markdown

I may be wrong here, but doesn't this limit the image size to just 1mb?

No, that part never changed. We've always had that limit, but here's what is happening behind the scenes:

  1. A user uploads an image, the limit of the image selected by the user in terms of size there is 5mb.
  2. The uploaded image is then cropped (during upload) and encoded into base64. It automatically reduces in size considerably here now.
  3. When we proceed with the mutation (click save), there the limit is 1mb and with these code changes, that should almost always satisfy.

Does this mean I can upload a 4.5mb PNG file without any issue? Does it get converted to JPEG?

Copy link
Copy Markdown
Contributor

@emrysal emrysal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice improvement

@alishaz-polymath
Copy link
Copy Markdown
Member Author

I may be wrong here, but doesn't this limit the image size to just 1mb?

No, that part never changed. We've always had that limit, but here's what is happening behind the scenes:

  1. A user uploads an image, the limit of the image selected by the user in terms of size there is 5mb.
  2. The uploaded image is then cropped (during upload) and encoded into base64. It automatically reduces in size considerably here now.
  3. When we proceed with the mutation (click save), there the limit is 1mb and with these code changes, that should almost always satisfy.

Does this mean I can upload a 4.5mb PNG file without any issue? Does it get converted to JPEG?

PNG will not be converted into JPEG. But it would still be compressed enough when cropping. So it would be worth a try 😅
I'd recommend using JPEG though.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 18, 2025

E2E results are ready!

@github-actions github-actions Bot added organizations area: organizations, orgs ⚡ Quick Wins A collection of quick wins/quick fixes that are less than 30 minutes of work labels Jun 18, 2025
@anikdhabal anikdhabal merged commit a5fe101 into main Jun 19, 2025
37 of 38 checks passed
@anikdhabal anikdhabal deleted the fix/image-uploader branch June 19, 2025 03:35
BKM14 pushed a commit to BKM14/cal.com that referenced this pull request Jun 26, 2025
…alcom#21903)

* improve uploader

* better error handling

* unnecessary check

* more adjustments

* remove logs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working core area: core, team members only enterprise area: enterprise, audit log, organisation, SAML, SSO 🧹 Improvements Improvements to existing features. Mostly UX/UI organizations area: organizations, orgs ⚡ Quick Wins A collection of quick wins/quick fixes that are less than 30 minutes of work ready-for-e2e

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve image uploader Fix error message when saving new uploaded image

6 participants