-
Notifications
You must be signed in to change notification settings - Fork 44
fix: image serialization #2295
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
base: main
Are you sure you want to change the base?
fix: image serialization #2295
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2295 +/- ##
==========================================
+ Coverage 86.75% 86.76% +0.01%
==========================================
Files 724 724
Lines 38741 38810 +69
Branches 9586 9595 +9
==========================================
+ Hits 33608 33674 +66
- Misses 4829 5127 +298
+ Partials 304 9 -295
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
codap-v3
|
||||||||||||||||||||||||||||
| Project |
codap-v3
|
| Branch Review |
CODAP-1074-image-serialization
|
| Run status |
|
| Run duration | 08m 09s |
| Commit |
|
| Committer | Kirk Swenson |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
3
|
|
|
48
|
|
|
0
|
|
|
301
|
| View all changes introduced in this branch ↗︎ | |
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.
Pull request overview
This PR fixes image serialization by converting images to data URLs instead of using ephemeral object URLs, enabling proper persistence and export of image tiles.
Changes:
- Adds utility functions to convert files to data URLs and downscale large images to prevent bloat
- Updates image import to use data URLs with automatic downscaling to 512px max dimension
- Ensures image tiles export correctly as
DG.ImageComponentViewtype in v2 format
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| v3/src/utilities/importable-files.ts | Adds stripExtensionFromFilename utility to extract clean filename for image titles |
| v3/src/utilities/importable-files.test.ts | Tests for the new filename stripping utility |
| v3/src/utilities/image-utils.ts | Adds fileToDataUrl and downscaleImageFile functions for image conversion and size optimization |
| v3/src/utilities/image-utils.test.ts | Tests for data URL conversion and downscaling functions |
| v3/src/hooks/use-import-helpers.ts | Updates image import to use data URLs instead of object URLs, adds filename as title |
| v3/src/components/web-view/web-view-registration.ts | Fixes image export to use correct DG.ImageComponentView component type |
| v3/src/components/web-view/web-view-registration.test.ts | Updates and adds tests for image tile export verification |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bfinzer
left a comment
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.
👍🏻LGTM
I'm going to write a new story describing the problem when running on MacOS26.
[CODAP-1074] Image tile serialization of dropped images is broken