search

mostafaznv / nova-ckeditor

CkEditor 5 Field for Laravel Nova with Media & Snippet Browsers
https://mostafaznv.gitbook.io/nova-ckeditor/
MIT License
51 stars 21 forks source link

Allow multiple toolbar configurations #51

Closed mennovanhout closed 1 year ago

mennovanhout commented 1 year ago

A simple pull request that will add support for multiple toolbar configurations. This PR is backwards compatible and won't break existing intergrations.

It's a simple PR but will add a lot of value if you use CKEditor for different types of fields and want to provide different toolbar options:

image

mennovanhout commented 1 year ago

I just found out that the pullrequest that has been merged yesterday makes the headeroptions configurable. But this is part of the toolbar and now added as a general setting. So I strongly belief this should be nesteded within the toolbar. Let me change this PR to addapt that aswell.

I'll still make it backwards compatible but I do believe the other MR is wrongly merged without thinking enough about it.

mennovanhout commented 1 year ago

I don't really like the "$this->headings = " part but it's the only way to make it backwards compatible since Laravels "mergeConfigFrom" doesn't do nested merging.

mostafaznv commented 1 year ago

Thank you for submitting this pull request. Your suggested feature is impressive, and I appreciate your contribution to the project. I am thrilled to merge it soon.

However, I would like to make some minor changes to it, which I will do tonight or maybe tomorrow.

mennovanhout commented 1 year ago

Thank you for submitting this pull request. Your suggested feature is impressive, and I appreciate your contribution to the project. I am thrilled to merge it soon.

However, I would like to make some minor changes to it, which I will do tonight or maybe tomorrow.

Hi, Thanks for your response. Could you inform me about the changes? I would also more then love to do them.

mostafaznv commented 1 year ago

I want to change two things:

1- Enhance $this->headers. and I would like to do part of it in a Vue component. 2- I have an idea regarding the multi-toolbar feature, but I know it might be challenging and could potentially cause some backward compatibility issues. However, I believe that having a config file like this would be nice:

'toolbars' => [
    'default' => 'toolbar-1',

    'toolbar-1' => [...],
    'toolbar-2' => [...],
]
mennovanhout commented 1 year ago

I want to change two things:

1- Enhance $this->headers. and I would like to do part of it in a Vue component. 2- I have an idea regarding the multi-toolbar feature, but I know it might be challenging and could potentially cause some backward compatibility issues. However, I believe that having a config file like this would be nice:

'toolbars' => [
    'default' => 'toolbar-1',

    'toolbar-1' => [...],
    'toolbar-2' => [...],
]

I agree, I just wanted to have some backwards compatibility because i didn't know if you'd wanted my PR to be merged. But I agree to rather have a config gile like that. Maybe it's time for a version 5 release ;). Then we can also remove the $this->heading code and make the project more maintainable.

mostafaznv commented 1 year ago

Yes. Now I'm thinking about releasing v5.0.0 :D

mostafaznv commented 1 year ago

Thank you for your contribution. It has been published in version 5.0.0 Please check the release page for details.

mennovanhout commented 1 year ago

@mostafaznv Thanks, I'd love to contribute more to this package. I'll star and watch it aswell. Maybe I can resolve more issues or add functions later :D

mostafaznv commented 1 year ago

@mennovanhout I am glad to hear this. thank you