[Form] Added "html5" option to both MoneyType and PercentType #35956
Conversation
romaricdrigon
force-pushed
the
feat-money-percent-html5
branch
from
e337ce9 to
3c686f2
Compare
|
We need to be careful here, type=number is increasingly seen as providing bad UX. |
|
Thats' a good point, and actually it is specified in the norm that they should not be used for non-sequential inputs (ie., bank account numbers...). Maybe it would be worth a warning in Fortunately here with monetary amounts & percentages we should be safe, there are legit use cases. |
|
(rebase needed) |
romaricdrigon
force-pushed
the
feat-money-percent-html5
branch
3 times, most recently
from
e30e36e to
8efe194
Compare
|
Thanks you for the heads up, I rebased code & updated |
romaricdrigon
force-pushed
the
feat-money-percent-html5
branch
7 times, most recently
from
a97942d to
e079a9a
Compare
|
Fabbot and Travis are green, AppVeyor failed because of a quota. If someone can please restart it, it should pass. |
| * @throws UnexpectedTypeException if the given value of type is unknown | ||
| */ | ||
| public function __construct(int $scale = null, string $type = null, ?int $roundingMode = null) | ||
| public function __construct(int $scale = null, string $type = null, ?int $roundingMode = null, bool $html5Format = false) |
There was a problem hiding this comment.
I think I would rather allow passing a $locale and $grouping argument like in the NumberToLocalizedStringTransformer.
There was a problem hiding this comment.
I have missed feelings about that: HTML5 format happens to be en locale without grouping, but it's accidental. I feel like it is correct to expose clearly that we want HTML5-compatible format in the normalizer interface.
romaricdrigon
force-pushed
the
feat-money-percent-html5
branch
from
e079a9a to
9e4ea16
Compare
|
What's the status of this PR? |
|
For me, the code part was done. I was waiting for some "formal" approval before doing the doc update. |
nicolas-grekas
force-pushed
the
feat-money-percent-html5
branch
from
9e4ea16 to
935dfdb
Compare
nicolas-grekas
force-pushed
the
feat-money-percent-html5
branch
from
935dfdb to
f7312a4
Compare
nicolas-grekas
left a comment
nicolas-grekas
left a comment
There was a problem hiding this comment.
Works for me (I rebased + made some minor tweaks)
|
Thank you @romaricdrigon. |
Thanks you very much @nicolas-grekas for finishing the PR! |
…pe and PercentType (romaricdrigon) This PR was merged into the master branch. Discussion ---------- [Form] Added documentation for "html5" option of MoneyType and PercentType Hello, Following symfony/symfony/pull/35956, which was merged for 5.2, this PR adds the corresponding documentation. Commits ------- d8fc70d [Form] Added documentation for "html5" option of MoneyType and PercentType
6 participants
Hello,
In the same way that NumberType offers a
html5option to render anumberinput instead of atextinput, this PR adds the same option to bothMoneyTypeandPercentType. Number inputs offer a better UI, especially on mobile (specific keyboard), and they accept extra HTML attributes, such asmax=100, which are quite useful.The challenge is that
numberinputs need a "raw" value, nor formatted nor localized.Format is described here https://www.w3.org/TR/html51/sec-forms.html#date-time-and-number-formats (non-normative, but tested on a few browsers). It matches number formatting for
enlocale (which is great, since it is the one provided by Symfony Intl polyflil), without grouping.Implementation was done in a manner similar to
NumberTypeforMoneyType.PercentTyperequired to modifyPercentToLocalizedStringTransformertoo.As a bonus,
PercentTypehad no tests at all, I added a few.I wanted to get some feedback on the idea first, remaining steps:
CHANGELOG.md