Skip to content

[Form] Fix moneytype step #62621

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
nicolas-grekas merged 1 commit into from
Dec 5, 2025
Merged

[Form] Fix moneytype step #62621

nicolas-grekas merged 1 commit into from
Dec 5, 2025
+52 −6

Conversation

@crownbackend
Copy link
Contributor

@crownbackend crownbackend commented Dec 2, 2025

Q A
Branch? 6.4
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #62620
License MIT

This PR fixes an inconsistency between MoneyType and NumberType when the html5 option is enabled.

NumberType automatically adds a step="any" attribute when none is provided, which allows decimal input in browsers.
MoneyType did not add this attribute, causing inconsistent behavior and preventing decimal input in some cases.

What this PR does

  • Adds step="any" when html5 is true and no custom step is set
  • Ensures MoneyType behaves like NumberType

Tests

A new test has been added to ensure the behavior is correctly applied and will not regress.

Why it matters

This improves consistency across form types and fixes a long-standing UX issue mentioned in #62620.

@carsonbot carsonbot added this to the 8.1 milestone Dec 2, 2025
@carsonbot

This comment was marked as outdated.

Sign in to view
@carsonbot carsonbot changed the title Fix moneytype step Fix moneytype step Dec 2, 2025
@crownbackend crownbackend changed the base branch from 8.1 to 6.4 December 2, 2025 11:00
@GromNaN GromNaN added the Form label Dec 2, 2025
@carsonbot carsonbot changed the title Fix moneytype step [Form] Fix moneytype step Dec 2, 2025
@OskarStark OskarStark modified the milestones: 8.1, 6.4 Dec 2, 2025
@alcohol
Copy link
Contributor

alcohol commented Dec 4, 2025

I wonder, should it also duplicate the line regarding inputmode like numbertype does?

See https://github.com/symfony/form/blob/a8f32aa19b322bf46cbaaafa89c132eb662ecfe5/Extension/Core/Type/NumberType.php#L49 and https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Global_attributes/inputmode for reference.

@crownbackend
Copy link
Contributor Author

crownbackend commented Dec 4, 2025

Thanks for the suggestion!

You're right — NumberType sets the inputmode attribute when html5 is disabled, to help browsers provide the appropriate numeric keyboard.

I'll update MoneyType to match the same behavior:

if (!$options['html5']) {
    $view->vars['attr']['inputmode'] = 0 === $options['scale'] ? 'numeric' : 'decimal';
}

I’ll push the update shortly.

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Dec 4, 2025

Test failures look related 🤔

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Dec 5, 2025

Thank you @crownbackend.

@nicolas-grekas nicolas-grekas merged commit 69876aa into symfony:6.4 Dec 5, 2025
8 of 11 checks passed
@crownbackend
Copy link
Contributor Author

crownbackend commented Dec 5, 2025

Thank you for running the tests. I didn’t have the time to do it last night. Sorry for the inconvenience, and thanks again for the merge.

@alcohol
Copy link
Contributor

alcohol commented Dec 5, 2025

Kudos to you both, thanks!

This was referenced Dec 7, 2025
Merged
Merged
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xabbuh xabbuh Awaiting requested review from xabbuh xabbuh is a code owner

@yceruto yceruto Awaiting requested review from yceruto yceruto is a code owner

Assignees

No one assigned

Labels

Bug Form Status: Needs Review

Projects

None yet

Milestone

6.4

Development

Successfully merging this pull request may close these issues.

MoneyType does not set steps like NumberType does

6 participants

@crownbackend @carsonbot @alcohol @nicolas-grekas @GromNaN @OskarStark