Skip to content

Conversation

nikosv
Copy link
Contributor

@nikosv nikosv commented Aug 29, 2025

This fixes an issue with boolean values on inputs. And more specifically if the value is false (boolean) the string casting ((string) $value) produces an empty string instead of 'false'. Moreover true produces '1' which is probably not the expected behavior when a user passes true as $value.

Even when a user try to avoid the issue by passing string values ('true', 'false'), if the model values passed through html()->model() are boolean then there are not auto-selected as the strict comparison of the values doesn't work.

This PR fixes the problem for all cases as long as are consistent with form values and model values.

Examples of current behavior:

// init with true value (boolean) - result: input with wrong value
html()->radio('test', null, true)   
<input type="radio" name="test" id="test_1" value="1">

// init with false value (boolean) - result: input with no value
html()->radio('test', null, false)
<input type="radio" name="test" id="test_" value>

// init with 'false' value (string) - result: no auto-selection (missing checked attribute)
html()->model(['test' => false])
html()->radio('test', null, 'false')
<input type="radio" name="test" id="test_false" value="false">

Examples of behavior after fix:

html()->radio('test', null, true)  
<input type="radio" name="test" id="test_true" value="true">

html()->radio('test', null, false)  
<input type="radio" name="test" id="test_false" value="false">

html()->model(['test' => false])
html()->radio('test', null, false)
<input type="radio" name="test" id="test_false" value="false" checked>

This fixes an issue with boolean values on inputs. An more specifically if the value is false the string casting ((string) $value) produces an empty string instead of 'false'. Moreover true produces '1' which is probably not the expected behavior when a user passes true as $value.
@nikosv nikosv changed the title Update BaseElement.php Fix issues with boolean values Aug 29, 2025
@nikosv
Copy link
Contributor Author

nikosv commented Aug 29, 2025

I also noticed that the strict comparison in radio button and true/ false values is also unsuitable as in an implementation without the package it would be something like old('field', $model->field) == 'true' as the old() function returns strings but model returns boolean values in this case.

So would it be ok to change this line
->attributeIf((! is_null($value) && $this->old($name) === $value) || $checked, 'checked');
to this
->attributeIf((! is_null($value) && $this->old($name) == $value) || $checked, 'checked');
?

@freekmurze
Copy link
Member

Could you add tests for this?

public function radio($name = null, $checked = null, $value = null)
{
{
$value_as_string = is_bool($value) ? json_encode($value) : $value;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We never use snake case for variables names. Please use package conventions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants