framework icon indicating copy to clipboard operation
framework copied to clipboard

[11.x] Adding minRatio & maxRatio rules on Dimension validation ruleset

Open CamKem opened this issue 1 year ago • 3 comments

This PR introduces the ability to set a minimum & maximum aspect ratio in the Dimension rule.

Currently the framework only supports fixed ratios within the rule, allowing restriction of aspect ratios to only the defined values, this would allow the user to set a range of aspect ratios that an image would have to fall within.

The premise for this was conceived whilst I was doing further work on the image upload feature for Pinkary. We are have been considering how to deal with very long images, such as screenshots of full webpages taken using devtools, where the aspect ratio is on the extreme end & it would be nice to be able to set a range to handle this through validation within the framework.

Currently we would need to write a custom rule or use a closure like so:

function ($attribute, $value, $fail) {
    [$width, $height] = getimagesize($value->getPathname());
    $aspectRatio = $width / $height;
    if ($aspectRatio > 1 / 2 || $aspectRatio < 1 / 3) {
        $fail("The image aspect ratio must be between 1:2 and 1:3.");
    }

This PR introduces the ability to use it like so:

File::image()
    ->types(['jpeg', 'png', 'gif', 'webp', 'jpg'])
    ->max($this->maxFileSize)
->dimensions(Rule::dimensions()
    ->minRatio(1/2)
    ->maxRatio(1/3)
),

You could use only the minimum or maximum ratio alone, if you don't need upper or lower limits.

I also added a range method, that on evaluations adds a min_ratio and max_ratio constraint.

File::image()
    ->types(['jpeg', 'png', 'gif', 'webp', 'jpg'])
    ->max($this->maxFileSize)
->dimensions(Rule::dimensions()
    ->ratioRange(min: 1/2, max: 1/3)
),

All 3 of the added methods would be a good edition to the Validation capabilities of the framework.

I have added tests to cover the addition of these methods to the Dimension ruleset.

Screenshot 2024-08-14 at 11 56 19 PM

CamKem avatar Aug 14 '24 13:08 CamKem

Please add a translation line for this and then mark ready for review again. 👍

taylorotwell avatar Aug 16 '24 18:08 taylorotwell

@taylorotwell I took your meaning as wanting me add a more granular translation line for the added parameters, however based on the existing implementation of this Dimension rule, without a lot of additional changes we can only utilise the current translation line in validation.php

    'dimensions' => 'The :attribute field has invalid image dimensions.',

We can then use the parameters within the string, which are swapped out for their values in FormatsMessage parser: Currently :width, :min_width, :max_width, :height, :min_height, :max_height & :ratio, Then If you merge this PR the :min_ratio & :max_ratio will also be available.

I spent quite a while looking into & working on refactoring the Dimensions rule & related logic, to bring it inline with the granularity of validation messages afforded to us by the similar File rule, whilst also ensuring there are no breaking changes. It's possible & I have a patch with a lot of the work for an implementation, but will require further consideration, such as how the between rules will interact with the min/max, etc...

Rather than go into it further & then to change the scope of this PR, I believe it would be wise to work on a refactor in a seperate PR that is independent of this one.


Happy to hear your thoughts.

CamKem avatar Aug 28 '24 13:08 CamKem

Tested this validation rule / message combo

 $this->validate(
            rules: [
                'images.*' => [
                    Rule::dimensions()
                        ->maxHeight(4000)
                        ->maxWidth(4000)
                        ->ratioBetween(min: 1 / 2, max: 2 / 5),
                ],
            messages: [
                'images.*.dimensions' => 'The image must be a maximum of :max_width x :max_height pixels & ratio between :min_ratio and :max_ratio.',
            ]
        );
Screenshot 2024-08-28 at 11 05 05 PM

I'll start working on a seperate branch to rewrite the rule to facilitate this kind of granularity. (Pending further comment)

[
   'images.*.max_width' => 'The image width may not be greater than :max_width pixels.',
   'images.*.max_height' => 'The image height may not be greater than :max_height pixels.',
   'images.*.height_between' => 'The image height must be between :min_height and :max_height pixels.',
   'images.*.min_ratio' => 'The image aspect ratio must be at least 1/2.',
   'images.*.max_ratio' => 'The image aspect ratio must be less than 2/5.',
   'images.*.ratio_between' => 'The image aspect ratio must be between 1/2 and 2/5.',
   // Providing default translation line messages for the following:
   // width, min_width, max_width, width_between.
   // height, min_height, max_height, height_between
   // ratio, min_ratio, max_ratio, ratio_between

CamKem avatar Aug 28 '24 13:08 CamKem

Got it - thanks.

taylorotwell avatar Sep 05 '24 21:09 taylorotwell