Skip to content
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

[BUG] x-adminlte-input-switch component: boolean error with Laravel 11 #1316

Closed
Figoware opened this issue Oct 31, 2024 · 17 comments · Fixed by #1317
Closed

[BUG] x-adminlte-input-switch component: boolean error with Laravel 11 #1316

Figoware opened this issue Oct 31, 2024 · 17 comments · Fixed by #1317
Labels
enhancement Improvement related to this package question Just a question, not an issue or bug

Comments

@Figoware
Copy link

Describe the bug

Unable to use <x-adminlte-input-switch>: string "true" is not allowed anymore in Laravel 11.
Even by using valdator string|in:true,false did not works (string value "true" won't be converted in boolean, causing DB insert error).

Changed in Laravel

File vendor/laravel/framework/src/Illuminate/Validation/Concerns/ValidatesAttributes.php changed in Laravel 11.

Laravel <=10

   public function validateBoolean($attribute, $value)
    {
        $acceptable = [true, false, 0, 1, '0', '1'];

        return in_array($value, $acceptable, true);
    }

Laravel 11

    public function validateBoolean($attribute, $value)
    {
        $acceptable = [true, false, 0, 1, '0', '1'];

        return in_array($value, $acceptable, true);
    }

Working solution

In vendor\laravel\framework\src\Illuminate\Validation\Concerns\ValidatesAttributes.php, change value="true" by value="1":

@section('input_group_item')

    {{-- Input Switch --}}
    <input type="checkbox" id="{{ $id }}" name="{{ $name }}" value="1"
        {{ $attributes->merge(['class' => $makeItemClass()]) }}>

@overwrite

Useful links

https://stackoverflow.com/questions/61713836/laravels-validation-failed-for-boolean-value-sended-by-postman
https://echebaby.com/blog/2021-12-30-laravel-validate-true-and-false-as-booleans/

Thank you!

@dfsmania
Copy link
Collaborator

Hello @Figoware , the <x-adminlte-input-switch> is just a nice checkbox, so when submitting the value you would check whether the field name is present in the request to act in consequence (and yes, it's currently submitting the string 'true' when present). I mostly use it like this with validations, use it as reference:

/**
 * Store a newly created resource in storage.
 */
public function store(Request $request): RedirectResponse
{
    // Map some request data to their correct values.

    $request->merge([
        'checkbox_name' => $request->has('checkbox_name'),
    ]);

    // Now, perfom validations on the submitted data.

    $validatedData = $request->validate([
        'other_field' => 'required|string|size:14',
        'checkbox_name' => 'bool',
    ]);

    // Other code...
    ...
}

So, if you still think there's a problem or issue with the component, please share the details about it.

@dfsmania dfsmania added the question Just a question, not an issue or bug label Oct 31, 2024
@Figoware
Copy link
Author

Thank you @dfsmania!

Sure, there are workarounds. But it's not as elegant as possible (in my opinion)!

The best would be to change the value when checked to 1, or to add a option in the component for checked value...

BTW, sorry for the [BUG] tag, my mistake, obviously it's not a bug.

@dfsmania dfsmania added the enhancement Improvement related to this package label Oct 31, 2024
@dfsmania
Copy link
Collaborator

dfsmania commented Nov 2, 2024

@Figoware would you test the minor modification proposed in the previous PR? Your feedback about this will be useful. You just need to edit and apply the change in your vendor/jeroennoten/laravel-adminlte/resources/views/components/form/input-switch.blade.php file. Thanks!

@Figoware
Copy link
Author

Figoware commented Nov 3, 2024

Yes, it's working, and it's a nice/elegant solution, thank you!

BTW, what's the best solution to handle "checked" attribute at init. Tried "{{ $value ? 'checked' :'' }}" : not working.
It there better than checked="{{ $value ? 'checked': '' }"?
Thanks!

@dfsmania
Copy link
Collaborator

dfsmania commented Nov 4, 2024

I don't think checked="{{ $value ? 'checked' : '' }}" would work, if the checked attribute exists, then the checkbox will be considered as checked no matter its value. Read https://stackoverflow.com/questions/4228658/what-values-for-checked-and-selected-are-false

If you want to setup the initial state of the input-switch based on some condition I would use the underlying plugin configuration state property, for example:

@php
    $config = [
        'state' => /* The condition to evaluate (true or false) */,
    ];
@endphp

<x-adminlte-input-switch name="my_checkbox" label="My Checkbox" :config="$config"/>

The underlying plugin configurations are documented here

@Figoware
Copy link
Author

Figoware commented Nov 5, 2024

Thank you for the solution!
But a bit too verbose for me... It should be simple to use, so I changed the component like this:

@section('input_group_item')

    {{-- Input Switch --}}
    <input type="checkbox" id="{{ $id }}" name="{{ $name }}" @checked($getOldValue($errorKey,$attributes['state']??false))
        {{ $attributes->merge(['class' => $makeItemClass(), 'value' => 'true' ]) }}

@overwrite

and using <x-adminlte-input-switch name="myBoolean" state="{{ $record->boolean}}" value="1" />

Hope it can help someone!

@dfsmania
Copy link
Collaborator

dfsmania commented Nov 5, 2024

@Figoware good one, take into consideration that changes inside the vendor folder would be overwritten by packages updates. So, instead you should publish the components. Then, you can customize and use the published version of the input-switch instead of the one provided by this package.

@Figoware
Copy link
Author

Figoware commented Nov 7, 2024

Sure! (I've read the doc ^^)
Thank you @dfsmania!

@Figoware Figoware closed this as completed Nov 7, 2024
@dfsmania
Copy link
Collaborator

dfsmania commented Nov 7, 2024

Let's keep this open, I will try to add a new attribute called is-checked to implement the same functionality you did with the state one.

@dfsmania dfsmania reopened this Nov 7, 2024
@dfsmania
Copy link
Collaborator

dfsmania commented Nov 7, 2024

@Figoware please review #1317 again, I have added support for is-checked attribute, what do you think?

@Figoware
Copy link
Author

Figoware commented Nov 8, 2024

Great!
Thank you very much!

BTW, there is a bug in Bootstrap Switch: if checked at first page render, submiting the form won't include the checkbox in fields values... Or I am missing something.

@Figoware
Copy link
Author

Figoware commented Nov 8, 2024

I ran some other tests: config['state'] has issues.

Here is what is really working for me + new feature (add unchecked support, i.e. see here):

Undo your PR change.
In input-switch.blade.php:

    {{-- Input Switch --}}
    @if($enabledUnchecked) <input type="hidden" name="{{ $name }}" value="0"/>@endif
    <input type="checkbox" id="{{ $id }}" name="{{ $name }}"
        {{ $attributes->merge(['class' => $makeItemClass(), 'value' => 'true', (($attributes['isChecked']?? false) ? 'checked': '') => '']) }}
        >

In InputSwitch.php:

    public $enableUnchecked = false;
    /**
     * Create a new component instance.
     * Note this component requires the 'Bootstrap Switch' plugin.
     *
     * @return void
     */
    public function __construct(
        $name, $id = null, $label = null, $igroupSize = null, $labelClass = null,
        $fgroupClass = null, $igroupClass = null, $disableFeedback = null,
        $errorKey = null, $config = [],  $enableUnchecked = null,
        $enableOldSupport = null,
    ) {
        parent::__construct(
            $name, $id, $label, $igroupSize, $labelClass, $fgroupClass,
            $igroupClass, $disableFeedback, $errorKey
        );

        if ($enableUnchecked) $this->enableUnchecked = true;
        $this->config = is_array($config) ? $config : [];
        $this->enableOldSupport = isset($enableOldSupport);
    }

Usage :

<x-adminlte-input-switch name="myBool" enable-old-support enableUnchecked isChecked="{{ $myBool }}" value="1" />

How does it sound for you?

@dfsmania
Copy link
Collaborator

dfsmania commented Nov 8, 2024

@Figoware Oh my gosh, it seems the plugin do not work nicely when setting the state on initialization procedure. I just added a workaround to address this, could you check changes on #1317 again. Also, for what I've seen, the version provided by AdminLTE of this plugin is the most stable (v.3.3.4), details here

@dfsmania
Copy link
Collaborator

dfsmania commented Nov 8, 2024

I ran some other tests: config['state'] has issues.

Here is what is really working for me + new feature (add unchecked support, i.e. see here):

Undo your PR change. In input-switch.blade.php:

    {{-- Input Switch --}}
    @if($enabledUnchecked) <input type="hidden" name="{{ $name }}" value="0"/>@endif
    <input type="checkbox" id="{{ $id }}" name="{{ $name }}"
        {{ $attributes->merge(['class' => $makeItemClass(), 'value' => 'true', (($attributes['isChecked']?? false) ? 'checked': '') => '']) }}
        >

In InputSwitch.php:

    public $enableUnchecked = false;
    /**
     * Create a new component instance.
     * Note this component requires the 'Bootstrap Switch' plugin.
     *
     * @return void
     */
    public function __construct(
        $name, $id = null, $label = null, $igroupSize = null, $labelClass = null,
        $fgroupClass = null, $igroupClass = null, $disableFeedback = null,
        $errorKey = null, $config = [],  $enableUnchecked = null,
        $enableOldSupport = null,
    ) {
        parent::__construct(
            $name, $id, $label, $igroupSize, $labelClass, $fgroupClass,
            $igroupClass, $disableFeedback, $errorKey
        );

        if ($enableUnchecked) $this->enableUnchecked = true;
        $this->config = is_array($config) ? $config : [];
        $this->enableOldSupport = isset($enableOldSupport);
    }

Usage :

<x-adminlte-input-switch name="myBool" enable-old-support enableUnchecked isChecked="{{ $myBool }}" value="1" />

How does it sound for you?

Hi @Figoware , thanks for your time invested here, but I don't want to over complicate things with the enableUnchecked option. A checkbox only submits a value when it's checked. So you can detect that in the controller as I have explained before. Or if you want to always submit a value, you can still add the hidden input before the <x-adminlte-input-switch> definition. For example:

<input type="hidden" name="myBool" value="0"/>
<x-adminlte-input-switch name="myBool" enable-old-support is-checked="{{ $myBool }}" value="1" />

@Figoware
Copy link
Author

Figoware commented Nov 8, 2024

enableUncheck: I understand.
For me it's part of the full functionality, so I'll keep it, to keep Controller, Model and View as simple as possible, reducing the risk of oversight (I often forget this detail, which plays tricks on me).

But the checked part is to keep (or improve)!

Thank you @dfsmania!

@Figoware
Copy link
Author

Figoware commented Nov 8, 2024

@Figoware Oh my gosh, it seems the plugin do not work nicely when setting the state on initialization procedure. I just added a workaround to address this, could you check changes on #1317 again. Also, for what I've seen, the version provided by AdminLTE of this plugin is the most stable (v.3.3.4), details here

Sorry, I miss this one!
Indeed, it's much better with your last changes. I tested, it's OK for me!

Now, I'll try version 3.3.4 of adminLTE, thanks for pointing that out!

@dfsmania
Copy link
Collaborator

dfsmania commented Nov 8, 2024

@Figoware Oh my gosh, it seems the plugin do not work nicely when setting the state on initialization procedure. I just added a workaround to address this, could you check changes on #1317 again. Also, for what I've seen, the version provided by AdminLTE of this plugin is the most stable (v.3.3.4), details here

Sorry, I miss this one! Indeed, it's much better with your last changes. I tested, it's OK for me!

Now, I'll try version 3.3.4 of adminLTE, thanks for pointing that out!

Sorry for the misunderstood, I tried to point the most stable version of bootstrap-switch plugin, it's the one currently available on the AdminLTE distribution files and the current version we use here if you have followed the normal setup procedure of the plugin. This note must only be taken into consideration if you are using CDN files to include the bootstrap-switch plugin into your project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement related to this package question Just a question, not an issue or bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants