Skip to content

Commit cd48985

Browse files
sukhwinder33445sukhwinder33445
committed
(Radio|Select)Element::getOption(): Remove support for null as param
- (Radio|Select)Option: Remove support for `null` value by defining the parameter types for the constructor - Ensure empty-string `''` option cannot be `disabled|checked|selected` by falsy values except `null` and `''` - Adjust tests accordingly and remove redundant tests
1 parent b2b0269 commit cd48985

6 files changed

Lines changed: 46 additions & 97 deletions

File tree

‎src/FormElement/RadioElement.php‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public function setOptions(array $options): self
6666
*
6767
* @throws InvalidArgumentException If no option with the specified value exists
6868
*/
69-
public function getOption($value): RadioOption
69+
public function getOption(string|int $value): RadioOption
7070
{
7171
if (! isset($this->options[$value])) {
7272
throw new InvalidArgumentException(sprintf('There is no such option "%s"', $value));
@@ -127,10 +127,15 @@ protected function assemble()
127127
'checked',
128128
function () use ($option) {
129129
$optionValue = $option->getValue();
130+
$value = $this->getValue();
131+
132+
if ($optionValue === '' && $value === null) {
133+
return true;
134+
}
130135

131136
return ! is_int($optionValue)
132-
? $this->getValue() === $optionValue
133-
: $this->getValue() == $optionValue;
137+
? $value === $optionValue
138+
: $value == $optionValue;
134139
}
135140
)
136141
->registerAttributeCallback(

‎src/FormElement/RadioOption.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class RadioOption
99
/** @var string The default label class */
1010
public const LABEL_CLASS = 'radio-label';
1111

12-
/** @var string|int|null Value of the option */
12+
/** @var string|int Value of the option */
1313
protected $value;
1414

1515
/** @var string Label of the option */
@@ -27,12 +27,12 @@ class RadioOption
2727
/**
2828
* RadioOption constructor.
2929
*
30-
* @param string|int|null $value
30+
* @param string|int $value
3131
* @param string $label
3232
*/
33-
public function __construct($value, string $label)
33+
public function __construct(string|int $value, string $label)
3434
{
35-
$this->value = $value === '' ? null : $value;
35+
$this->value = $value;
3636
$this->label = $label;
3737
}
3838

@@ -63,9 +63,9 @@ public function getLabel(): string
6363
/**
6464
* Get the value of the option
6565
*
66-
* @return string|int|null
66+
* @return string|int
6767
*/
68-
public function getValue()
68+
public function getValue(): string|int
6969
{
7070
return $this->value;
7171
}

‎src/FormElement/SelectElement.php‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
namespace ipl\Html\FormElement;
44

5-
use InvalidArgumentException;
65
use ipl\Html\Attributes;
76
use ipl\Html\Common\MultipleAttribute;
87
use ipl\Html\Html;
@@ -32,11 +31,11 @@ class SelectElement extends BaseFormElement
3231
/**
3332
* Get the option with specified value
3433
*
35-
* @param string|int|null $value
34+
* @param string|int $value
3635
*
3736
* @return ?SelectOption
3837
*/
39-
public function getOption($value): ?SelectOption
38+
public function getOption(string|int $value): ?SelectOption
4039
{
4140
return $this->options[$value] ?? null;
4241
}
@@ -75,7 +74,7 @@ public function setDisabledOptions(array $disabledOptions): self
7574
$option->setAttribute(
7675
'disabled',
7776
in_array($optionValue, $disabledOptions, ! is_int($optionValue))
78-
|| ($optionValue === null && in_array('', $disabledOptions, true))
77+
|| ($optionValue === '' && in_array(null, $disabledOptions, true))
7978
);
8079
}
8180

@@ -119,12 +118,12 @@ public function getNameAttribute()
119118
/**
120119
* Make the selectOption for the specified value and the label
121120
*
122-
* @param string|int|null $value Value of the option
121+
* @param string|int $value Value of the option
123122
* @param string|array $label Label of the option
124123
*
125124
* @return SelectOption|HtmlElement
126125
*/
127-
protected function makeOption($value, $label)
126+
protected function makeOption(string|int $value, string|array $label)
128127
{
129128
if (is_array($label)) {
130129
$grp = Html::tag('optgroup', ['label' => $value]);
@@ -150,27 +149,23 @@ protected function makeOption($value, $label)
150149
/**
151150
* Get whether the given option is selected
152151
*
153-
* @param int|string|null $optionValue
152+
* @param string|int $optionValue
154153
*
155154
* @return bool
156155
*/
157-
protected function isSelectedOption($optionValue): bool
156+
protected function isSelectedOption(string|int $optionValue): bool
158157
{
159158
$value = $this->getValue();
160159

161-
if ($optionValue === '') {
162-
$optionValue = null;
163-
}
164-
165160
if ($this->isMultiple()) {
166161
if (! is_array($value)) {
167162
throw new UnexpectedValueException(
168163
'Value must be an array when the `multiple` attribute is set to `true`'
169164
);
170165
}
171166

172-
return in_array($optionValue, $this->getValue(), ! is_int($optionValue))
173-
|| ($optionValue === null && in_array('', $this->getValue(), true));
167+
return in_array($optionValue, $value, ! is_int($optionValue))
168+
|| ($optionValue === '' && in_array(null, $value, true));
174169
}
175170

176171
if (is_array($value)) {
@@ -179,6 +174,10 @@ protected function isSelectedOption($optionValue): bool
179174
);
180175
}
181176

177+
if ($optionValue === '' && $value === null) {
178+
return true;
179+
}
180+
182181
return is_int($optionValue)
183182
// The loose comparison is required because PHP casts
184183
// numeric strings to integers if used as array keys

‎src/FormElement/SelectOption.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ class SelectOption extends BaseHtmlElement
88
{
99
protected $tag = 'option';
1010

11-
/** @var string|int|null Value of the option */
11+
/** @var string|int Value of the option */
1212
protected $value;
1313

1414
/** @var string Label of the option */
@@ -17,12 +17,12 @@ class SelectOption extends BaseHtmlElement
1717
/**
1818
* SelectOption constructor.
1919
*
20-
* @param string|int|null $value
20+
* @param string|int $value
2121
* @param string $label
2222
*/
23-
public function __construct($value, string $label)
23+
public function __construct(string|int $value, string $label)
2424
{
25-
$this->value = $value === '' ? null : $value;
25+
$this->value = $value;
2626
$this->label = $label;
2727

2828
$this->getAttributes()->registerAttributeCallback('value', [$this, 'getValueAttribute']);
@@ -55,7 +55,7 @@ public function getLabel(): string
5555
/**
5656
* Get the value of the option
5757
*
58-
* @return string|int|null
58+
* @return string|int
5959
*/
6060
public function getValue()
6161
{

‎tests/FormElement/RadioElementTest.php‎

Lines changed: 4 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ public function testRadioNotValidIfCheckedValueIsDisabled()
282282
$this->assertFalse($radio->isValid());
283283
}
284284

285-
public function testNullAndTheEmptyStringAreEquallyHandled()
285+
public function testNullAndTheEmptyStringValuesAreEquallyHandled()
286286
{
287287
$form = new Form();
288288
$form->addElement('radio', 'radio', [
@@ -303,8 +303,6 @@ public function testNullAndTheEmptyStringAreEquallyHandled()
303303
$this->assertNull($radio2->getValue());
304304

305305
$this->assertInstanceOf(RadioOption::class, $radio->getOption(''));
306-
$this->assertInstanceOf(RadioOption::class, $radio2->getOption(null));
307-
$this->assertInstanceOf(RadioOption::class, $radio->getOption(null));
308306
$this->assertInstanceOf(RadioOption::class, $radio2->getOption(''));
309307

310308
$this->assertTrue($radio->isValid());
@@ -401,63 +399,36 @@ public function testGetOptionReturnsPreviouslySetOption()
401399
$radio = new RadioElement('radio');
402400
$radio->setOptions(['' => 'Empty String', 'foo' => 'Foo', 'bar' => 'Bar']);
403401

404-
$this->assertNull($radio->getOption('')->getValue());
402+
$this->assertSame('', $radio->getOption('')->getValue());
405403
$this->assertSame('Empty String', $radio->getOption('')->getLabel());
406404

407405
$this->assertSame('foo', $radio->getOption('foo')->getValue());
408406
$this->assertSame('Foo', $radio->getOption('foo')->getLabel());
409407

410408
$radio->setOptions(['' => 'Please Choose', 'car' => 'Car', 'train' => 'Train']);
411409

412-
$this->assertNull($radio->getOption('')->getValue());
410+
$this->assertSame('', $radio->getOption('')->getValue());
413411
$this->assertSame('Please Choose', $radio->getOption('')->getLabel());
414412

415413
$this->assertSame('car', $radio->getOption('car')->getValue());
416414
$this->assertSame('Car', $radio->getOption('car')->getLabel());
417415
}
418416

419-
public function testNullAndTheEmptyStringAreAlsoEquallyHandledWhileDisablingOptions()
417+
public function testNullAndTheEmptyStringAreEquallyHandledWhileDisablingOptions()
420418
{
421419
$radio = new RadioElement('radio');
422420
$radio->setOptions(['' => 'Foo', 'bar' => 'Bar']);
423421
$radio->setDisabledOptions([null]);
424422

425-
$this->assertTrue($radio->getOption(null)->isDisabled());
426-
427-
$radio = new RadioElement('radio');
428-
$radio->setOptions(['' => 'Foo', 'bar' => 'Bar']);
429-
$radio->setDisabledOptions(['']);
430-
431423
$this->assertTrue($radio->getOption('')->isDisabled());
432424

433425
$radio = new RadioElement('radio');
434426
$radio->setOptions(['' => 'Foo', 'bar' => 'Bar']);
435427
$radio->setDisabledOptions(['']);
436428

437-
$this->assertTrue($radio->getOption(null)->isDisabled());
438-
$radio = new RadioElement('radio');
439-
$radio->setOptions(['' => 'Foo', 'bar' => 'Bar']);
440-
$radio->setDisabledOptions([null]);
441-
442429
$this->assertTrue($radio->getOption('')->isDisabled());
443430
}
444431

445-
public function testGetOptionGetValueAndElementGetValueHandleNullAndTheEmptyStringEqually()
446-
{
447-
$radio = new RadioElement('radio');
448-
$radio->setOptions(['' => 'Foo']);
449-
$radio->setValue('');
450-
451-
$this->assertNull($radio->getValue());
452-
$this->assertNull($radio->getOption('')->getValue());
453-
454-
$radio = new RadioElement('radio');
455-
$radio->setOptions(['' => 'Foo']);
456-
457-
$this->assertNull($radio->getValue());
458-
$this->assertNull($radio->getOption(null)->getValue());
459-
}
460-
461432
public function testIsDecoratedWithLabelAndDescription(): void
462433
{
463434
$radio = new RadioElement('radio', ['id' => 'radio']);

‎tests/FormElement/SelectElementTest.php‎

Lines changed: 10 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ public function testGetValueReturnsAnArrayWhenMultipleAttributeIsSet()
441441
$this->assertSame([], $select->getValue());
442442
}
443443

444-
public function testNullAndTheEmptyStringAreEquallyHandled()
444+
public function testNullAndTheEmptyStringValuesAreEquallyHandled()
445445
{
446446
$form = new Form();
447447
$form->addElement('select', 'select', [
@@ -462,8 +462,6 @@ public function testNullAndTheEmptyStringAreEquallyHandled()
462462
$this->assertNull($select2->getValue());
463463

464464
$this->assertInstanceOf(SelectOption::class, $select->getOption(''));
465-
$this->assertInstanceOf(SelectOption::class, $select2->getOption(null));
466-
$this->assertInstanceOf(SelectOption::class, $select->getOption(null));
467465
$this->assertInstanceOf(SelectOption::class, $select2->getOption(''));
468466

469467
$this->assertTrue($select->isValid());
@@ -496,25 +494,6 @@ public function testNullAndTheEmptyStringAreEquallyHandled()
496494
$this->assertNull($select2->getValue());
497495
}
498496

499-
public function testGetOptionGetValueAndElementGetValueHandleNullAndTheEmptyStringEqually()
500-
{
501-
$select = new SelectElement('select');
502-
$select->setOptions(['' => 'Foo']);
503-
$select->setValue('');
504-
505-
$this->assertNull($select->getValue());
506-
$this->assertNull($select->getOption('')->getValue());
507-
508-
$select = new SelectElement('select');
509-
$select->setOptions(['' => 'Foo']);
510-
511-
$this->assertNull($select->getValue());
512-
$this->assertNull($select->getOption(null)->getValue());
513-
}
514-
515-
/**
516-
* @depends testNullAndTheEmptyStringAreEquallyHandled
517-
*/
518497
public function testDisablingOptionsIsWorking()
519498
{
520499
$form = new Form();
@@ -536,29 +515,18 @@ public function testDisablingOptionsIsWorking()
536515
$this->assertHtml($html, $form->getElement('select'));
537516
}
538517

539-
public function testNullAndTheEmptyStringAreAlsoEquallyHandledWhileDisablingOptions()
518+
public function testNullAndTheEmptyStringAreEquallyHandledWhileDisablingOptions()
540519
{
541520
$select = new SelectElement('select');
542521
$select->setOptions(['' => 'Foo', 'bar' => 'Bar']);
543522
$select->setDisabledOptions([null]);
544523

545-
$this->assertTrue($select->getOption(null)->getAttributes()->get('disabled')->getValue());
546-
547-
$select = new SelectElement('select');
548-
$select->setOptions(['' => 'Foo', 'bar' => 'Bar']);
549-
$select->setDisabledOptions(['']);
550-
551524
$this->assertTrue($select->getOption('')->getAttributes()->get('disabled')->getValue());
552525

553526
$select = new SelectElement('select');
554527
$select->setOptions(['' => 'Foo', 'bar' => 'Bar']);
555528
$select->setDisabledOptions(['']);
556529

557-
$this->assertTrue($select->getOption(null)->getAttributes()->get('disabled')->getValue());
558-
$select = new SelectElement('select');
559-
$select->setOptions(['' => 'Foo', 'bar' => 'Bar']);
560-
$select->setDisabledOptions([null]);
561-
562530
$this->assertTrue($select->getOption('')->getAttributes()->get('disabled')->getValue());
563531
}
564532

@@ -614,13 +582,19 @@ public function testGetOptionReturnsPreviouslySetOption()
614582
$select = new SelectElement('select');
615583
$select->setOptions(['' => 'Empty String', 'foo' => 'Foo', 'bar' => 'Bar']);
616584

617-
$this->assertNull($select->getOption('')->getValue());
585+
$this->assertSame('', $select->getOption('')->getValue());
586+
$this->assertSame('Empty String', $select->getOption('')->getLabel());
587+
618588
$this->assertSame('foo', $select->getOption('foo')->getValue());
589+
$this->assertSame('Foo', $select->getOption('foo')->getLabel());
619590

620591
$select->setOptions(['' => 'Please Choose', 'car' => 'Car', 'train' => 'Train']);
621592

622-
$this->assertNull($select->getOption('')->getValue());
593+
$this->assertSame('', $select->getOption('')->getValue());
594+
$this->assertSame('Please Choose', $select->getOption('')->getLabel());
595+
623596
$this->assertSame('car', $select->getOption('car')->getValue());
597+
$this->assertSame('Car', $select->getOption('car')->getLabel());
624598
}
625599

626600
public function testSetDisabledOptionsResetsDisabledOptions()

0 commit comments

Comments
 (0)