waynestate / nova-ckeditor4-field

A Laravel Nova CKEditor4 WYSIWYG Field
MIT License
61 stars 24 forks source link

Configuring toolbar in config and on field can cause toolbar duplication #87

Closed justindantzer closed 1 year ago

justindantzer commented 1 year ago

There appears to be an issue with merging of config options and passed options on a specific field, resulting in duplicated toolbar items.

To Reproduce

  1. Install package, leaving config as is.
  2. Add field via the CKEditor::make() method to a given Nova resource
  3. Using the field's options() method, apply toolbar modifications
    CKEditor::make('Some Field')
    ->options([
        'toolbar' => [
            ['Source', '-', 'Print', '-', 'Cut', 'Copy', 'Paste', 'PasteText', 'PasteFromWord', ],
            ['Undo', 'Redo', '-', 'Find', 'Replace', '-', 'SelectAll', 'RemoveFormat'],
            ['HorizontalRule', 'SpecialChar'],
            '/',
            ['Format', ],
            ['Bold', 'Italic', 'Strike', '-', 'Subscript', 'Superscript'],
            ['NumberedList', 'BulletedList', '-', 'Outdent', 'Indent', 'Blockquote', ],
            ['JustifyLeft', 'JustifyCenter', 'JustifyRight'],
            ['Link', 'Unlink'],
            ['Maximize', 'ShowBlocks'],
        ],
    ])
  4. View the create/edit page for the Nova resource, and see the duplicated toolbar items (screenshots below)

Expected behavior The toolbar should be overridden by the options passed at the field level

Screenshots Actual

Screenshot 2023-04-06 at 3 30 26 PM

Expected

Screenshot 2023-04-06 at 3 30 52 PM

The issue appears to be related to the array_merge_recursive within the options() method on the field:

public function options($options)
{
    $currentOptions = $this->meta['options'] ?? [];

    return $this->withMeta([
        'options' => array_merge_recursive($currentOptions, $options),
    ]);
}

Calling array_merge_recursive on the entirety of the options array causes the duplication issue within the non-associative toolbar option. I don't think array_merge_recursive is necessarily wrong, but a special case is needed for the toolbar option and any other non-associative options.

Generic recursive merge example:

$merged = array_merge_recursive(['somekey' => [1, 2, 3]], ['somekey' => [1, 2, 3]])
// results in ['somekey' => [1,2,3,1,2,3]]
chrispelzer commented 1 year ago

@justindantzer Thanks. This is now fixed with 1.2.3

Taking another look at it, you're right. Taking a look at the options for CKEditor, it should be replacing the 1st level of keys if they exist within the users config options. https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_config.html