Make it possible to define custom acf types#30
Make it possible to define custom acf types#30kevinkiel wants to merge 6 commits intocorcel:developfrom
Conversation
|
Hi @kevinkiel thanks for contributing with Corcel ;-)
What do you think? JG |
There was a problem hiding this comment.
@kevinkiel take a look on this. Your commit is associated with an unrecognized email address:
Unrecognized author. If this is you, make sure kevin@impres.nl is associated with your account. Click to add email addresses in your account settings.
I suggest you to add kevin@impres.nl to your Github account or change your local Git email address. This is important to Github to follow all contributions you did.
src/FieldFactory.php
Outdated
| $type = $fakeText->fetchFieldType($key); | ||
| } | ||
|
|
||
| $default_types = [ |
There was a problem hiding this comment.
Let's move this to an class variable (static):
protected static $defaultTypes = [...];|
|
||
| $types = array_merge($default_types, $custom_types); | ||
|
|
||
| if (!empty($types[$type])) { |
There was a problem hiding this comment.
I'd suggest here:
if (!empty($types[$type])) {
$field = new $types[$type]($post);
$field->process($name);
return $field;
}
return null;
src/FieldFactory.php
Outdated
| default: return null; | ||
| $custom_types = []; | ||
|
|
||
| if (\Config::has('corcel.acf.custom_types')) { |
There was a problem hiding this comment.
You have to check if the Config class exists before using it. That's the first error when running phpunit. I suggest you to check if the class exists or if the config() function exists like I did on Model.php class. Please, take a look.
|
@jgrossi Thanks for your fast reaction and feedback. I made some small changes to the code so it would pass the tests. In the case of using a config file I think it's better to add a sample file in corcel/corcel repo, with a CorcelServiceProvider or something like that. Than the user can get a default file with the suggested options; |
|
What I mean is to create the Take a look: https://laravel.com/docs/5.3/packages#configuration |
|
@jgrossi Thanks for your response. I've add the CorcelAcfServiceProvider. Now my question is in what package are we gonna merge the 2 config files?
|
|
@jgrossi What do you think? |
|
Hi @kevinkiel sorry for being MIA but time is scarce for now :-) My goal is to have, at least for now, just one I was wondering if I can merge the config options you created (about acf) into the main I'll try to install a fresh Laravel here, install Corcel, copy the config file and then install Thanks! |
Example: corcel.config.php