FriendsOfSymfony / FOSCKEditorBundle

Provides a CKEditor integration for your Symfony project.
Other
522 stars 84 forks source link

Default configuration overwriting #172

Closed cpoint-eu closed 5 years ago

cpoint-eu commented 5 years ago

Hi, I encountered a configuration problem with "default_config" value and custom toolbar settings.

my config file:

`fos_ck_editor:

default_config: simple_toolbar
configs:
    simple_toolbar:
        toolbar:
            - ['Undo', 'Redo']
            - ['Format']
            - ['Bold', 'Italic', 'Strike', 'Subscript', 'Superscript' ]
            - ['Link', 'Unlink']
            - ['RemoveFormat']
            - ['JustifyLeft', 'JustifyCenter', 'JustifyRight', 'JustifyBlock']
            - ['BulletedList', 'NumberedList', '-', 'Outdent', 'Indent']
            - ['Image', 'Table', 'CodeSnippet']
            - ['Copy']
            - ['Source']`

When I call the FOS CK Editor type without "config_name" attribute, the toolbar is always overwritten by default toolbar. The problem is, probably, with config array merge in FOS\CKEditorBundle\Form\Type\CKEditorType on line 72:

$config = array_merge($this->configuration->getConfig($options['config_name']), $config);

and should be:

$config = array_merge($config, $this->configuration->getConfig($options['config_name']));

kunicmarko20 commented 5 years ago

Hi @hasakm. If I am correct, isn't the point that your form configuration overwrites global configuration? Based on that I would say that merge is fine, but maybe something else can be done.

x1oJ0 commented 5 years ago

Hi @kunicmarko20. I think @hasakm's config should overwrite the default config of fos_ck_editor to setup toolbar items as he wants them. Looks like that that config merge overwrites his custom settings by default once. I tried to simulate the situation and his point is right, options in merge should be swap. When I swap the values in array_merge then it works fine for me. If I keep the merge as it is my custom settings for toolbar doesn't work. Or am I wrong?

kunicmarko20 commented 5 years ago

Ah, I confused it. The second will overwrite the first one, correct? Then it makes sense to swap it, are you able to provide a PR, please?

x1oJ0 commented 5 years ago

Sure @kunicmarko20 I just opened the PR.

kunicmarko20 commented 5 years ago

Closing as no response.