Skip to content

Commit

Permalink
Checkbox: allow $uncheckedValue to be nullable to distinguish unset…
Browse files Browse the repository at this point in the history
… property from empty string and zero string (#242)

* FormMultiCheckbox should respect 0 as uncheckedValue in an hidden element

* add a failing test to showcase the bug

Signed-off-by: Thomas Rieschl <[email protected]>

* Checkbox: allow `$uncheckedValue` to be nullable to distinguish unset property from empty string and zero string

Signed-off-by: Filippo Tessarotto <[email protected]>

---------

Signed-off-by: Thomas Rieschl <[email protected]>
Signed-off-by: Filippo Tessarotto <[email protected]>
Co-authored-by: Filippo Tessarotto <[email protected]>
  • Loading branch information
rieschl and Slamdunk authored Oct 17, 2023
1 parent c6100a6 commit 8883a89
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 11 deletions.
6 changes: 3 additions & 3 deletions src/Element/Checkbox.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class Checkbox extends Element implements InputProviderInterface
/** @var bool */
protected $useHiddenElement = true;

/** @var string */
/** @var null|string */
protected $uncheckedValue = '0';

/** @var string */
Expand Down Expand Up @@ -83,7 +83,7 @@ public function useHiddenElement(): bool
*
* @return $this
*/
public function setUncheckedValue(string $uncheckedValue)
public function setUncheckedValue(?string $uncheckedValue)
{
$this->uncheckedValue = $uncheckedValue;
return $this;
Expand All @@ -92,7 +92,7 @@ public function setUncheckedValue(string $uncheckedValue)
/**
* Get the value to use when checkbox is unchecked
*/
public function getUncheckedValue(): string
public function getUncheckedValue(): ?string
{
return $this->uncheckedValue;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Element/MultiCheckbox.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ class MultiCheckbox extends Checkbox
/** @var bool */
protected $useHiddenElement = false;

/** @var string */
protected $uncheckedValue = '';
/** @var null|string */
protected $uncheckedValue;

/** @var array */
protected $valueOptions = [];
Expand Down
10 changes: 5 additions & 5 deletions src/View/Helper/FormMultiCheckbox.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ class FormMultiCheckbox extends FormInput
/**
* The unchecked value used when "UseHiddenElement" is turned on
*
* @var string
* @var null|string
*/
protected $uncheckedValue = '';
protected $uncheckedValue;

/**
* Form input helper instance
Expand Down Expand Up @@ -243,7 +243,7 @@ protected function renderHiddenElement(MultiCheckboxElement $element): string
{
$closingBracket = $this->getInlineClosingBracket();

$uncheckedValue = $element->getUncheckedValue() ?: $this->uncheckedValue;
$uncheckedValue = $element->getUncheckedValue() ?? $this->uncheckedValue;

$hiddenAttributes = [
'name' => $element->getName(),
Expand Down Expand Up @@ -354,7 +354,7 @@ public function getUseHiddenElement(): bool
*
* @return $this
*/
public function setUncheckedValue(string $value)
public function setUncheckedValue(?string $value)
{
$this->uncheckedValue = $value;
return $this;
Expand All @@ -363,7 +363,7 @@ public function setUncheckedValue(string $value)
/**
* Returns the unchecked value used when "UseHiddenElement" is turned on.
*/
public function getUncheckedValue(): string
public function getUncheckedValue(): ?string
{
return $this->uncheckedValue;
}
Expand Down
27 changes: 26 additions & 1 deletion test/View/Helper/FormMultiCheckboxTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ public function testGetUseHiddenElementSetToTrue(): void
public function testGetUncheckedValueReturnsDefaultEmptyString(): void
{
$uncheckedValue = $this->helper->getUncheckedValue();
self::assertSame('', $uncheckedValue);
self::assertNull($uncheckedValue);
}

public function testGetUncheckedValueSetToFoo(): void
Expand Down Expand Up @@ -419,4 +419,29 @@ public function testRenderWithoutValueOptions(): void

self::assertEmpty($this->helper->render($element));
}

public function testRendersZeroAsUncheckedValueOfElement(): void
{
$element = $this->getElement();
$element->setUseHiddenElement(true);

$markup = $this->helper->render($element);

$expectedElement = '<input type="hidden" name="foo" value="">';
self::assertStringContainsString($expectedElement, $markup);

$element->setUncheckedValue('');

$markup = $this->helper->render($element);

$expectedElement = '<input type="hidden" name="foo" value="">';
self::assertStringContainsString($expectedElement, $markup);

$element->setUncheckedValue('0');

$markup = $this->helper->render($element);

$expectedElement = '<input type="hidden" name="foo" value="0">';
self::assertStringContainsString($expectedElement, $markup);
}
}

0 comments on commit 8883a89

Please sign in to comment.