-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
Fix wrong boolean values #61296
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
Fix wrong boolean values #61296
Conversation
@@ -2141,7 +2141,7 @@ public function testMoney() | |||
public function testMoneyWithoutCurrency() | |||
{ | |||
$form = $this->factory->createNamed('name', 'Symfony\Component\Form\Extension\Core\Type\MoneyType', 1234.56, [ | |||
'currency' => false, | |||
'currency' => null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing false
is actually what is documented as the way to suppress showing the currency symbol:
You can also set this to false to hide the currency symbol.
see https://symfony.com/doc/current/reference/forms/types/money.html#currency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/cc @Girgias
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well I'm not sure if this a documentation issue or a core bug where the field type should be widened
@@ -1386,7 +1385,6 @@ public function testNormalizerCanAccessLazyOptions() | |||
$this->resolver->setDefault('norm', 'baz'); | |||
|
|||
$this->resolver->setNormalizer('norm', function (Options $options) { | |||
/** @var TestCase $test */ | |||
Assert::assertEquals('bar', $options['lazy']); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assert::assertEquals('bar', $options['lazy']); | |
Assert::assertSame('bar', $options['lazy']); |
?
$this->assertTrue($this->resolver->isDefined('foo')); | ||
$this->assertTrue($this->resolver->isDeprecated('foo')); | ||
$this->assertTrue($this->resolver->hasDefault('foo')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one
2adde56
to
12d0aa9
Compare
This slipped in with symfony#61296. Technically, there shouldn't be a difference but since we explicitly mention false in the documentation let's better be safe here.
…y symbol (xabbuh) This PR was merged into the 6.4 branch. Discussion ---------- [Form] use `false` instead of `null` to hide the currency symbol | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | | License | MIT This slipped in with #61296. Technically, there shouldn't be a difference but since we explicitly mention false in the documentation let's better be safe here. Commits ------- ecd674d use false instead of null to hide the currency symbol
The subset of #61203 that's obviously mistakes