Skip to content

DependsOnNot bug fix with strict check#184

Open
xUJYx wants to merge 8 commits intoepartment:masterfrom
xUJYx:dependsOnNot_fix
Open

DependsOnNot bug fix with strict check#184
xUJYx wants to merge 8 commits intoepartment:masterfrom
xUJYx:dependsOnNot_fix

Conversation

@xUJYx
Copy link
Copy Markdown

@xUJYx xUJYx commented Jul 19, 2021

For 'dependsOnNot' - changed 'strict check' to 'check with coercion' cause all form values are always strings.
Error occurs when checking for integer values and they comparing with string equivalents from html form.

@AlbinoDrought
Copy link
Copy Markdown

AlbinoDrought commented Jan 31, 2022

Ran into this issue with the unforked epartment/nova-dependency-container as well, here's a quick repro:

image

<?php

namespace App\Nova\Actions;

use Epartment\NovaDependencyContainer\NovaDependencyContainer;
use Laravel\Nova\Actions\Action;
use Laravel\Nova\Fields\Select;
use Laravel\Nova\Fields\Text;

class SampleDependsOnNot44 extends Action
{
    const SOME_OPTION_A = 1;
    const SOME_OPTION_B = 2;
    const SOME_OPTION_C = 3;

    public function fields()
    {
        return [
            Select::make('Option', 'option')
                ->options([
                    self::SOME_OPTION_A => 'A, show dependency field',
                    self::SOME_OPTION_B => 'B, hide dependency field',
                    self::SOME_OPTION_C => 'C, show dependency field',
                ]),

            NovaDependencyContainer::make([
                Text::make('Hide me for option B'),
            ])->dependsOnNot('option', self::SOME_OPTION_B),
        ];
    }
}

Expected result:

image

@xUJYx
Copy link
Copy Markdown
Author

xUJYx commented Jan 31, 2022

Expected result:

image

Have you used code from my fork? Or from epartment/nova-dependency-container itself?

@AlbinoDrought
Copy link
Copy Markdown

Have you used code from my fork? Or from epartment/nova-dependency-container itself?

Sorry, just from epartment/nova-dependency-container. I'll update my above comment

@xUJYx
Copy link
Copy Markdown
Author

xUJYx commented Feb 1, 2022

Have you used code from my fork? Or from epartment/nova-dependency-container itself?

Sorry, just from epartment/nova-dependency-container. I'll update my above comment

That functionality wasnt merged into main repo. So this functionality doesnt work on original epartment/nova-dependency-container package...

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