Skip to content

fix(web): theme import no longer says a file just under 1 MB is "1024 KB" - #30330

Open
spoukyii wants to merge 2 commits into
pingdotgg:mainfrom
spoukyii:users/spoukyii/theme-import-size-1024-kb
Open

spoukyii wants to merge 2 commits into
pingdotgg:mainfrom
spoukyii:users/spoukyii/theme-import-size-1024-kb

Conversation

@spoukyii

@spoukyii spoukyii commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Settings > Appearance > Import theme refuses a theme file over 256 KB with a notice that gives its size. A file from 1,048,064 to 1,048,575 bytes reads "That file is 1024 KB" when it should read "1.00 MB".

The dialog formats that size with its own formatByteSize, which picks KB before rounding, so Math.round(bytes / 1024) reaches 1024 while the unit is still KB.

Change

ThemeImportDialog drops formatByteSize and uses the shared apps/web/src/lib/formatBytes, which picks the unit after rounding. The notice for an oversized paste goes through the same function and changes the same way.

Beyond the boundary fix, sizes now read the way every other size in the app does:

Size Before After
262,145 B / 524,288 B 256 KB / 512 KB 256 KB / 512 KB
1,048,100 B 1024 KB 1.00 MB
1,572,864 B 1.5 MB 1.50 MB
12,897,485 B 12.3 MB 12.3 MB
157,286,400 B 150.0 MB 150 MB
1,073,731,338 B 1024.0 MB 1.00 GB

The limit in the same sentence still reads "256 KB".

Scope and approval

This is a small, focused fix for an obvious bug, so it needs no prior issue. #26067 fixed this rounding in the shared formatBytes, and #29165 fixed it in attachment sizes. #28300 fixed it in Diagnostics by replacing that view's private formatter with formatBytes, and its description names this dialog as the follow-up: "Other private copies with the same rounding bug stay out of this PR, one problem each: the theme import size in ThemeImportDialog.tsx and mobile's Client Storage settings." This PR is that follow-up for the web copy, done the same way. Mobile's copy is #28648.

Verification

apps/web/src/components/settings/ThemeImportDialog.browser.test.tsx gets one case. It uploads a 1,048,100-byte theme file and checks the notice. It sits at the dialog because that is the lowest layer that fails on main: the shared formatBytes already passes this size, and the broken helper was private to the dialog.

On main with only the test commit (21aac83), from apps/web, vp test run src/components/settings/ThemeImportDialog.browser.test.tsx:

× sizes an oversized theme file just under 1 MB as 1.00 MB
Expected element to have text content:
  That file is 1.00 MB. Theme files are only a few KB, so this one was not read (limit 256 KB).
Received:
  That file is 1024 KB. Theme files are only a few KB, so this one was not read (limit 256 KB).
Tests  1 failed | 6 passed (7)

With the fix, from apps/web:

  • vp test run src/components/settings/ThemeImportDialog.browser.test.tsx src/lib/formatBytes.test.ts: 2 files, 18 tests passed.
  • vp run check:changed from the repo root: format, lint, knip, typecheck, tests beside the change and the React Compiler check all pass.

In a running dev build (Windows, vp run dev), Settings > Appearance > Add theme, choosing a 1,048,100-byte .json file:

Before (main) After (this branch)
Before: That file is 1024 KB After: That file is 1.00 MB

Bug: theme import says a file just under 1 MB is "1024 KB"

This was written with Claude Opus 5.5 in Claude Code, driven from T3 Code.

Uploads a 1,048,100-byte theme file and reads the oversized notice.
The dialog's own formatter picks KB before rounding, so the notice
says "1024 KB" here and the test fails.
… KB"

The theme import dialog formatted the size in its "That file is ..."
notice with its own helper, which picked KB before rounding. A file
from 1,048,064 to 1,048,575 bytes rounded to 1024 and read "1024 KB".
That is the defect pingdotgg#26067 fixed in the shared formatBytes, and pingdotgg#28300
and pingdotgg#29165 removed from the Diagnostics and attachment sizes.

The dialog now uses formatBytes from lib, so it picks the unit after
rounding like the rest of the app. The 256 KB limit still reads
"256 KB".

Bug: theme import says a file just under 1 MB is "1024 KB"
@github-actions github-actions Bot added the vouch:unvouched PR author is not yet trusted in the VOUCHED list. label Nov 3, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at c34c3ec

Macroscope's review found this PR approvable — Theme-import errors now use the shared byte formatter to avoid displaying a rounded size as “1024 KB.” This is a presentation-only change; file-size limits and import behavior remain unchanged.

You can add or adjust custom eligibility rules. Learn more.

@github-actions github-actions Bot added the size:XS 0-9 changed lines (additions + deletions). label Nov 3, 2026
@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

The theme import dialog now uses the shared formatBytes function in oversized-file messages. A browser test checks the displayed size and 256 KB limit message.

Changes: Theme import size display

Layer / File(s) Summary
Oversized file message and test
apps/web/src/components/settings/ThemeImportDialog.tsx, apps/web/src/components/settings/ThemeImportDialog.browser.test.tsx
The dialog formats the file size and limit with formatBytes. The size acceptance condition is unchanged. A browser test checks the message for a file just under 1 MB.

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Suggested reviewers: macroscopeapp

Merge Risk: Minimal · up to c34c3

The formatting fix is tested and does not alter the file-size acceptance rule.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title names the main change: the theme import notice no longer reports a file just under 1 MB as "1024 KB". The changeset replaces the dialog's private size formatter with the shared formatBytes, …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description includes Problem, Change, Scope and approval, and Verification sections. It explains the bug, the implementation, scope rationale, regression test, validation results, manual UI verifi…
Full details: Title check

Explanation

The title names the main change: the theme import notice no longer reports a file just under 1 MB as "1024 KB". The changeset replaces the dialog's private size formatter with the shared formatBytes, which chooses the unit after rounding, and adds a regression test for the 1,048,100-byte case. The title is specific, follows conventional commit style, and is accurate.

Full details: Description check

Explanation

The description includes Problem, Change, Scope and approval, and Verification sections. It explains the bug, the implementation, scope rationale, regression test, validation results, manual UI verification, screenshots, and the agent used.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between dd24b37 and c34c3ec.

Files selected for processing: 2
  • apps/web/src/components/settings/ThemeImportDialog.browser.test.tsx
  • apps/web/src/components/settings/ThemeImportDialog.tsx

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@Acedia11

Acedia11 commented Nov 3, 2026

Copy link
Copy Markdown

Much needed!

@maria-rcks

maria-rcks commented Nov 3, 2026 •

Copy link
Copy Markdown
Collaborator

Note

Written by gpt-6.1-sol on behalf of Maria

Verified in a real dev environment

verified unchanged head c34c3ec10852b3be11b68969c5f8b239a57fdae6 on linux with repository Playwright and /usr/bin/google-chrome, using the actual web app and server. installed dependencies with vp i on both checkouts and ran only one background dev server at a time, with fresh isolated state and no mocks or seeded data.

  • reproduced on github/main (7c22e24e3fa1): settings > appearance > add theme > choose files, uploading a 1,048,100-byte JSON file shows "That file is 1024 KB".
  • the exact PR head shows "That file is 1.00 MB" for the same file and oversized JSON pasted into the editor. the limit still reads "256 KB".
  • a 262,145-byte file is rejected; a valid theme of exactly 262,144 bytes imports, becomes selectable, and remains installed after reload. invalid JSON still reports its validation error. importing a small valid file after rejection also succeeds.
  • added the real repository as a project through the UI and submitted a real Codex gpt-6.1-sol turn. it completed after one automatic provider recovery and displayed both command results and its final response:
pwd
/home/user/t3code
git rev-parse --is-inside-work-tree
true
THEME_VERIFICATION_OK

vp run check:changed bff68fb6b6f86346f780024db62ba75873f70854 passed through blacksmith-check: format, lint, knip, affected typechecks, all 7 theme-dialog browser tests, and the React Compiler check. all 11 shared formatter tests also passed. the runner needed browser installation and comparison-history setup first. no source changes were necessary.

before, main:

main: the same 1,048,100-byte theme file incorrectly reads 1024 KB

after, PR head:

pr: the same theme file correctly reads 1.00 MB with the unchanged 256 KB limit

file rejection, paste rejection, limit rejection, and recovery to a valid import:

theme import interaction recording

real codex turn with command output and completed response

Not verified

native Electron file picking, macOS/Windows, and native iOS/Android clients were not exercised on this Linux VM. Open VSX downloads, drag-and-drop, batch imports, and other providers were outside this focused pass. the changed formatter is in the web dialog; the native desktop bridge itself is unchanged.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants