emilianotisato / nova-tinymce

Laravel Nova TinyMCE editor (with images upload capabilities!)
MIT License
116 stars 39 forks source link

Adds full support of self hosted functionality #17

Closed y-martini closed 4 years ago

y-martini commented 5 years ago

Note: This PR considers use of TinyMCE v5. To make images upload work, see: https://github.com/UniSharp/laravel-filemanager/issues/759#issuecomment-487258426

emilianotisato commented 5 years ago

@yuriy-martini Thaks for your contribution!

Just to understand, with this implementation you don't need tinyMCE api key? If that is true, please do not just remove the key from the vue file resources/js/components/FormField.vue but also remove the instruction in the readme, the call in the Provider src/FieldServiceProvider.php / etc.

And we need to compile the assets (npm run prod) so the dist/js/field.js is generated again and new users installing this package can use it right away.

Again, super thanks for your contribution!

y-martini commented 4 years ago

Just to understand, with this implementation you don't need tinyMCE api key? If that is true, please do not just remove the key from the vue file resources/js/components/FormField.vue but also remove the instruction in the readme, the call in the Provider src/FieldServiceProvider.php / etc.

Yes, you don't need tinyMCE api key. Removed other api key info and updated readme.md

And we need to compile the assets (npm run prod) so the dist/js/field.js is generated again and new users installing this package can use it right away.

Yes, of course. Will be added some files and folders in /dist.

emilianotisato commented 4 years ago

Perfect, thanks!

But just last thing is missing:

Yes, of course. Will be added some files and folders in /dist.

I do not see in this PR the new compiled assets replacing the old ones in dist folder, so when I do a fresh install I am sill using the old version.

Please compile and add the new dist files in this PR.

y-martini commented 4 years ago

Perfect, thanks!

But just last thing is missing:

Yes, of course. Will be added some files and folders in /dist.

I do not see in this PR the new compiled assets replacing the old ones in dist folder, so when I do a fresh install I am sill using the old version.

Please compile and add the new dist files in this PR.

You need to compile, of course. It's because dist files shouldn't be in PR

emilianotisato commented 4 years ago

@yuriy-martini I understand your confusion. It is that in a normal project of course the compiled assets are not versioned. But in a package that is ready to push the assets, of course you need to give the end user already compiled!

Look at any other nova package, we all compile and version the dist folder always! it so that when you install you already have the assets that are injected into the ServiceProvider

In this case: src/FieldServiceProvider.php lines 19 and 20

So, trust me and add the compiled version please. You did a good work here, I will merge this PR immediately after you add the compiled assets.

Thanks!

y-martini commented 4 years ago

Yes, sure :D