Skip to content

[Form] [Forms] rounding default const up to down #16757

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

Merged

Conversation

sebastienfuchs
Copy link

@sebastienfuchs sebastienfuchs requested a review from xabbuh as a code owner April 27, 2022 13:08
@sebastienfuchs sebastienfuchs changed the title [Fixed] [Forms] rounding default const up to down [Forms] rounding default const up to down Apr 27, 2022
@xabbuh
Copy link
Member

xabbuh commented Apr 29, 2022

I think we need to find another solution as the rounding_mode option does exist in more than one form type, namely these, and they have different default values:

  • IntegerType (ROUND_DOWN)
  • MoneyType (ROUND_HALF_UP)
  • NumberType (ROUND_HALF_UP)

@sebastienfuchs
Copy link
Author

Yes sure, can you prefer like this ? @xabbuh

@carsonbot carsonbot changed the title [Forms] rounding default const up to down [Form] [Forms] rounding default const up to down Apr 29, 2022
@javiereguiluz javiereguiluz added this to the 5.4 milestone Apr 29, 2022
@javiereguiluz javiereguiluz changed the base branch from 6.0 to 5.4 April 29, 2022 14:35
@javiereguiluz javiereguiluz force-pushed the fix-forms-rounding-default-const branch from a2369b4 to f7318bb Compare April 29, 2022 14:35
@javiereguiluz javiereguiluz merged commit 93bb0e4 into symfony:5.4 Apr 29, 2022
@javiereguiluz
Copy link
Member

@sebastienfuchs thanks and congrats on your first Symfony Docs contribution 🎉

At the end I tweaked this a bit to display the two possible default values on the same line. I think it's readable enough:

default-value

@xabbuh
Copy link
Member

xabbuh commented Apr 29, 2022

Nice work 👍 Thanks @sebastienfuchs and Javier! ❤️

@sebastienfuchs sebastienfuchs deleted the fix-forms-rounding-default-const branch April 29, 2022 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants