dcasia / conditional-container

Provides an easy way to conditionally show and hide fields in your Nova resources.
MIT License
115 stars 37 forks source link

Nova Flexible Content - Usage with Presets #24

Open bernhardh opened 4 years ago

bernhardh commented 4 years ago

I know, I am really anoying, but I still can't get it up and running on my real scenario, where I use Flexible Content Presets:

Nova Resource

public function fields() {
    return [
        Tab::make($this->title(), [
            ....
            "Content Blocks" => [
                Flexible::make('', 'gridBlocks')
                    ->preset(GridFlexibel::class)
            ]
        ])  
    ];
}

Flexible Preset

class GridFlexibel extends Preset {
    public function handle(Flexible $field) {
        $field->addLayout(HtmlLayout::class);
    }
}

Flexible HtmlLayout

public function fields() {
    return [
        Select::make("Type", "type")->options([
            "html" => "HTML",
            "plain" => "Plaintext",
        ]),
        ConditionalContainer::make([
            Trix::make('Content', 'html_content'),
        ])->if('type = html'),
        ConditionalContainer::make([
            Textarea::make('Content', 'plain_content'),
        ])->if('type = plain'),
    ];
}

And I get the

cannot read property 'join' of undefined at p.watchableAttributes

error. I tried to add the trait HasConditionalContainer to the model and to the HtmlLayoutas well, but nothing worked.

Sorry for being so anoying.

milewski commented 4 years ago

Fixed pull again

milewski commented 4 years ago

For those coming here use the version on the flexible branch:

composer require digital-creative/conditional-container:dev-flexible-content

I haven't merged it yet to master due the validation is still not working, once I get that sorted out I will merge to master and tag a new release on packagist

bernhardh commented 4 years ago

Still getting the join js error msg. Where do I have to put the HasConditionalContainer?

To the HtmlLayout or the Resource? I tried both.

milewski commented 4 years ago

to your resource, try to clean the cache, and make sure you have the latest version on the flexible branch

I have copied your exact example without the tabs..:

from here:

 Flexible::make('', 'gridBlocks')
                    ->preset(GridFlexibel::class)

and it worked... if it still doesn't maybe is because of the tabs package T_T

bernhardh commented 4 years ago

I have analyzed it and found a couple of issues. First I am using your HasMegaFilterTrait as well, so I had to create my own trait:

trait HasMegaFilterAndConditionalContainer {
    use HasMegaFilterTrait {
        HasMegaFilterTrait::availableFields as availableFieldsForMegafilter;
    }
    use HasConditionalContainer {
        HasConditionalContainer::availableFields as availableFieldsConditionalContainer;
    }

    public function availableFields(NovaRequest $request) {
        // Is an index request, so we need MegaFilter
        $controller = $request->route()->controller;
        if ($controller instanceof ResourceIndexController) {
            return $this->availableFieldsForMegafilter($request);
        }
        elseif ($controller instanceof CreationFieldController || $controller instanceof UpdateFieldController) {
            return $this->availableFieldsConditionalContainer($request);
        }

        // All other requests get the regular availableFields method
        return parent::availableFields($request);
    }
}

The second problem is with the way your implementation works and which is incompatible with the Tabs package and probably with other packages and maybe custom macros in some cases as well. The problem is, that you clone all the fields and layouts, but not all their extra properties and metas. For example, if you use the tabs package, your field inside an flexible will lose the tab and panel information after cloning.

Just my thought (maybe I am wrong): It's maybe not the best solution to clone half of the objects (performance wise its definitely not the best). I understand, that the problem is, that you can't manipulate the protected $fields of the Layout objects directly, so you have to clone at least this fields. So we need a method like setFields on the layout:

public function setFields($fields) {
    $this->fields = $fields;
    return $this;
}

My suggestion for a rewrite of your solution then would be

/**
     * @param NovaRequest $request
     *
     * @return FieldCollection
     */
    public function availableFields(NovaRequest $request) {
        $controller = $request->route()->controller;

        if ($controller instanceof CreationFieldController || $controller instanceof UpdateFieldController) {
            $fields = parent::availableFields($request);
            $containers = $this->findAllContainers($fields);
            $expressionsMap = $containers->flatMap->expressions;
            $hasConditionalContainer = $this;

            // Inject conditional container fields meta to fields
            $fields = $fields->each(function ($field) use ($hasConditionalContainer, $expressionsMap) {
                return $hasConditionalContainer->injectMetaIntoFields($field, $expressionsMap);
            });

            return $fields;
        }

        return parent::availableFields($request);
    }

    /**
     * @param $field
     * @param $expressionsMap
     *
     * @return mixed
     */
    protected function injectMetaIntoFields($field, $expressionsMap) {
        if ($field instanceof Flexible) {
            $field->meta["layouts"] = $field->meta["layouts"]->each(function ($layout) use ($expressionsMap) {
                $layout->setFields(collect($layout->fields())->each(function ($field) use ($expressionsMap) {
                    if ($field instanceof ConditionalContainer) {
                        $field->withMeta(['__uses_flexible_field__' => true]);
                        $field->withMeta(['expressionsMap' => $expressionsMap]);

                        $field->fields->each(static function ($field) {
                            if (method_exists($field, 'withMeta')) {
                                $field->withMeta(['__has_flexible_field__' => true]);
                            }
                        });
                    }

                }));

                return $layout;
            });
        }

        return $field;
    }

Maybe I am missing something, but I removed a lot of stuff of yours, where I didn't understand why you need it at all.

milewski commented 4 years ago

Exactly that was the main problem I couldn't transform the protected $fields from their layout so I had to clone everything, we could reach out to them and submit a PR to add this method, they don't have to use it themselves but would help this package work with theirs.

One small problem I see with your rewrite is that it is not considering that you could have Flexible inside Flexible and a Conditional Container Inside Flexible and Flexible Inside Conditional Container like:

Conditional
   Flexible
      Flexible
          Conditional
Flexible
   Conditional

With the function, I had: findAllFlexibleContentFields would find everything and flatten them all out to inject all the necessary values,

I think it's easy if you could push your changes as a PR and we can discuss directly comment on the code lines