feat(ui): update image dimensions and mime type handling#407
feat(ui): update image dimensions and mime type handling#407eliot488995568 wants to merge 4 commits intodevelopfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses Rozier’s “Adjust” tab behavior for image edits by preserving WebP output (instead of unintentionally converting to PNG) and ensuring cropped image dimensions are updated end-to-end (backend persistence + frontend display).
Changes:
- Pass document MIME type into the Blanchette editor so canvas export can target the original format (e.g., WebP).
- Persist updated image width/height on document overwrite and return these dimensions in the JSON response for UI refresh.
- Adjust dev
nodeservice settings incompose.yml(interactive TTY/stdin + restart policy).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/Rozier/app/store/modules/BlanchetteEditorStoreModule.js | Returns backend JSON data from save action so callers can update UI state (e.g., dimensions). |
| lib/Rozier/app/containers/BlanchetteEditorContainer.vue | Adds MIME type awareness for export, updates displayed width/height after overwrite, and adjusts overwrite filename logic. |
| lib/RoadizRozierBundle/templates/documents/adjust.html.twig | Passes document.mimeType to the Vue editor container. |
| lib/RoadizRozierBundle/src/Controller/Document/DocumentAdjustController.php | Computes and persists image dimensions from the uploaded edited file; includes dimensions in JSON response. |
| compose.yml | Changes runtime behavior of the dev Node service (TTY/stdin/restart). |
compose.yml
Outdated
| stdin_open: true | ||
| tty: true | ||
| restart: unless-stopped |
There was a problem hiding this comment.
These Docker Compose settings (stdin_open, tty, restart) change how the dev Node service behaves (it will keep running and restart automatically). This seems unrelated to the PR's stated goal (image mime type/dimensions) and could have side effects for contributors; consider moving this to a separate PR or documenting the rationale.
| return data | ||
| } else { | ||
| throw new Error('No path found') | ||
| } |
There was a problem hiding this comment.
throw new Error('No path found') will be handled by the .catch(...) below, but that catch currently expects an HTTP-style error with error.response.json(). For this thrown Error (and for fetch network failures), error.response will be undefined and the error handling will itself throw. Update the catch logic to safely handle plain Error objects (and consider checking response.ok before parsing JSON).
| resolveOutputMimeType(mimeType) { | ||
| switch (mimeType) { | ||
| case 'image/jpeg': | ||
| case 'image/png': | ||
| case 'image/webp': | ||
| return mimeType | ||
| case 'image/gif': | ||
| default: | ||
| return 'image/png' | ||
| } |
There was a problem hiding this comment.
resolveOutputMimeType now allows image/webp, but the crop implementation later treats any non-PNG output as needing a white fillColor background. Since WebP supports alpha like PNG, this will flatten transparency when outputting WebP. Adjust the crop canvas options logic to avoid forcing a fill color for WebP outputs.
| /** @var UploadedFile $uploadedFile */ | ||
| $uploadedFile = $fileForm->get('editDocument')->getData(); | ||
|
|
||
| $imageSize = @\getimagesize($uploadedFile->getPathname()); |
There was a problem hiding this comment.
$uploadedFile = $fileForm->get('editDocument')->getData() can be null (field is required: false and the File constraint allows null). Calling $uploadedFile->getPathname() will then fatally error. Add an instanceof UploadedFile guard (or make the field required) and return a proper validation error response if no file was provided.
| $imageSize = @\getimagesize($uploadedFile->getPathname()); | ||
| if (\is_array($imageSize)) { | ||
| $document->setImageWidth((int) $imageSize[0]); |
There was a problem hiding this comment.
Using @\getimagesize(...) suppresses warnings and makes failures hard to diagnose. Prefer explicit handling (e.g., validate the MIME type first and handle expected failures) rather than error suppression so unexpected problems surface during debugging.
| } else { | ||
| $document->setImageWidth(0); | ||
| $document->setImageHeight(0); |
There was a problem hiding this comment.
When getimagesize fails, the code overwrites existing dimensions with 0. This can regress SVG (and any unsupported image) documents by wiping previously known sizes. Consider leaving the existing width/height unchanged on failure, or using the same size-resolution logic used elsewhere for SVG/image documents instead of forcing 0.
| } else { | |
| $document->setImageWidth(0); | |
| $document->setImageHeight(0); |
…406-webp-image-adjustments-change-mime-type-and-crop-does-not-update-dimensions
Closes #406