Skip to content

[Validator] Fix minRatio and maxRatio when getting rounded #45204

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 31, 2022
Merged

[Validator] Fix minRatio and maxRatio when getting rounded #45204

merged 1 commit into from
Jan 31, 2022

Conversation

alexander-schranz
Copy link
Contributor

@alexander-schranz alexander-schranz commented Jan 27, 2022 •
edited
Loading

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #...
License MIT
Doc PR symfony/symfony-docs#...

Currently the generated ratio get rounded to 1.33 but the inputted ration not. This can end into a strange behaviour when working with example:

new Image([
    'maxRatio' => 64 / 48,
    'minRatio' => 64 / 48,
]),

Workaround currently required rounding input value itself:

new Image([
    // round ratio the same way as the validator: https://github.com/symfony/symfony/blob/4834d9e224b97f946fe8e13b2ba403bbc212c4ad/src/Symfony/Component/Validator/Constraints/ImageValidator.php#L165
    'maxRatio' => round(64 / 48, 2),
    'minRatio' => round(64 / 48, 2),
]),

Another solution would be removed:

-   $ratio = round($width / $height, 2);

instead. What do you think?

@carsonbot carsonbot added this to the 4.4 milestone Jan 27, 2022
@nicolas-grekas
Copy link
Member

Needs tests :)

@alexander-schranz
Copy link
Contributor Author

@nicolas-grekas added :)

fancyweb
fancyweb suggested changes Jan 27, 2022
View reviewed changes
johdcode
johdcode suggested changes Jan 29, 2022
View reviewed changes
@fancyweb
Copy link
Contributor

Thank you @alexander-schranz.

@fancyweb fancyweb merged commit 19da12f into symfony:4.4 Jan 31, 2022
@alexander-schranz alexander-schranz deleted the patch-10 branch January 31, 2022 09:25
This was referenced Feb 28, 2022
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

+2 more reviewers

@johdcode johdcode johdcode requested changes

@fancyweb fancyweb fancyweb approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Bug Status: Reviewed Validator
Projects
None yet
Milestone
4.4
Development

Successfully merging this pull request may close these issues.
5 participants
@alexander-schranz @nicolas-grekas @fancyweb @johdcode @carsonbot