fix: accept vehicle images of any size and aspect ratio - #4
Merged
Conversation
Two separate causes, both in the image field of VehicleForm.
imageAspectRatio('4:3') was not only a hint for the browser crop editor.
It also produces a server-side Rule::dimensions()->ratio(4/3) via
BaseFileUpload::getRules(), so any image in a different ratio was rejected
outright — reproduced by the first test in VehicleImageUploadTest:
Component has errors: "data.image_path" =>
["The fahrzeugbild field has invalid image dimensions."]
automaticallyResizeImagesToWidth('2000') ran with Filament's default
shouldAutomaticallyUpscaleImagesWhenResizing = true, so an 800 pixel image
was scaled *up* to 2000 — visibly soft and several times the original file
size. That half happens in the browser (FilePondPluginImageResize) and is
not observable from a server-side test; it is verified by uploading and
reading back the stored dimensions.
The resize mode moves from 'cover' to 'contain': without a fixed aspect
ratio and with no target height set, 'cover' would crop a panorama to a
square. 'contain' only ever scales down.
The crop editor stays available, it is just no longer mandatory. The 10 MB
limit is untouched — it is now the only constraint on an image.
Display needs no change: Filament already applies object-cover to both
table image columns and infolist images (tables/resources/css/columns/
image.css, infolists/resources/css/components/image.css), so mixed aspect
ratios do not make rows or cards uneven.
The existing test asserting the 4:3 rejection tested the reported bug as if
it were a feature; it now asserts that a square image is accepted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The upscaling half of the bug happens in the browser, inside FilePondPluginImageResize, so no server-side test can observe the result. This asserts the instruction to not upscale actually reaches the component, so a later edit cannot drop it silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two separate causes in the image field of
VehicleForm, both hit as soon as an image is not exactly 4:3 or is smaller than 2000 pixels.1. The aspect ratio was enforced on the server.
imageAspectRatio('4:3')is not only a hint for the browser crop editor — it also producesRule::dimensions()->ratio(4/3)viaBaseFileUpload::getRules(). Any image in a different ratio was rejected outright:2. Small images were scaled up.
automaticallyResizeImagesToWidth('2000')ran with Filament's defaultshouldAutomaticallyUpscaleImagesWhenResizing = true, so an 800 pixel image was blown up to 2000 — visibly soft, and several times the original file size.Change
imageAspectRatio('4:3')andautomaticallyOpenImageEditorForAspectRatio()removed, along with the now-deaddimensionsvalidation message. The crop editor stays available, it is just no longer mandatory.automaticallyUpscaleImagesWhenResizing(false)added.cover→contain. Without a fixed aspect ratio and with no target height set,coverwould crop a panorama to a square;containonly ever scales down.maxSize(10 MB)untouched — it is now the only constraint on an image.Display needed no change: Filament already applies
object-coverto both table image columns and infolist images (tables/resources/css/columns/image.css:5,infolists/resources/css/components/image.css:5), so mixed aspect ratios do not make rows or cards uneven.Verification
Full suite: 388 passed (1241 assertions). Pint clean.
New
VehicleImageUploadTestcovers: an off-ratio image is accepted, a small image is accepted, a non-image is rejected, and an image above 10 MB is still rejected.One half of this cannot be proven server-side. The upscaling happens in the browser, inside
FilePondPluginImageResize, which afillForm()test never runs. What the suite does assert is that the instruction reaches the component (shouldAutomaticallyUpscaleImagesWhenResizing() === false, modecontain), so a later edit cannot drop it silently. The actual pixel result should be confirmed once by hand: upload an 800×600 image and check the stored file is still 800×600.The existing test that asserted the 4:3 rejection was testing the reported bug as if it were a feature; it now asserts that a square image is accepted.
🤖 Generated with Claude Code