mitydigital / statamic-tinymce-cloud

A Statamic fieldtype for TinyMCE using Tiny Cloud for Statamic 3.3+ and Statamic 4.0
https://docs.mity.com.au/tinymce-cloud
MIT License
2 stars 1 forks source link

JSON Configuration Issues #5

Closed BuchyOne closed 1 year ago

BuchyOne commented 1 year ago

Bug description

v1.0.3 Cannot add valid JSON to Configurations Default and have it pass.

Workaround has been to comment out validation in ConfigurationsDefault.php.

The editor appears as assigned in Blueprints.

v1 screenshot of workaround

Screen Shot 2022-09-06 at 5 45 14 pm

...

v2.0.1 Cannot add valid JSON to Configurations Default and have it pass. Comment out validation in ConfigurationsDefault.php, then JSON passes. Then, no editing is possible.

Editing pages report exceptions:

ErrorException: Illegal string offset 'configuration'

Cannot access offset of type string on string {"userId":"95c8d6d5-... /vendor/mitydigital/statamic-tinymce-cloud/src/Fieldtypes/TinymceCloud.php:38

Added logging there to check default variables : {"cloud_channel":"6","defaults":[{"name":"Simple","configuration":"{ \"height\":500, \t\"plugins\":[\"image\", \"table\", \"wordcount\"], \t \"toolbar\":\"undo redo | blocks | bold italic backcolor | alignleft aligncenter alignright alignjustify | bullist numlist outdent indent | remove format | help\" }","type":"configuration","enabled":true}]}

Further logging suggested the json decoded $config is an empty object.

v2 screenshot of workaround

Screen Shot 2022-09-06 at 6 45 11 pm

Steps to reproduce

Install TinyMCE via composer.json mod and upgrade. Clear all caches. Log in to statamic cp. Go to TinyMCE Add valid JSON to Configuration form and submit.


Repeat with validation removed.

Screenshots show success / fail when validation off / on

Screen Shot 2022-09-06 at 6 13 59 pm Screen Shot 2022-09-06 at 6 10 19 pm

Environment and versions

Laravel Framework 9.27.0
Statamic 3.3.34
Apache/2.4.53 (Unix) LibreSSL/3.3.6 PHP/8.1.9 
TinyMCE 1.0.3 & 2.0.1
---

Additional details

I am using v1.0.3 successfully with the workaround, however I continue to receive 'domain not registered ... ' message.

Have account, domain is registered, REQUEST referer header is in the domain.

Have KEY correctly set in .env

Nuked everything, new site, used the : composer require mitydigital/statamic-tinymce-cloud install method. Same JSON issues.

Same probs installing on different machine / enviro - Ubuntu / NGINX

For v2 did wipe 'init' from relevant page yamls

First time using Statamic, may not be a bug, may be my dimness ...

martyf commented 1 year ago

Thanks for all of the detail here...

The JSON validation was removed from v1 and reworked in v2 but clearly still causing issues.

To workaround this, making sure everything is strictly valid is needed. Run your JSON through https://jsonlint.com/ to test. This also includes double-quoting your keys in the JSON object, which is getting strict, I know, such as:

{
    "selector": "textarea",
    "height": 500,
    "plugins": [
        "advlist", "autolink", "lists", "link", "image", "charmap", "preview",
        "anchor", "searchreplace", "visualblocks", "code", "fullscreen",
        "insertdatetime", "media", "table", "help", "wordcount"
    ]
}

So let me ask you this: what makes more sense here? Trying to do some validation, or would you expect that whatever you put in the field will simply just be used? Seriously curious to hear what you would expect this to do.

I feel it is not right the way it is... it is too strict and too fiddly to be changing config between what the JS version allows and what this addon allows. So it may just be easier to remove the validation altogether. Thoughts?

I'm no longer supporting the v1 branch, but does your key work in the v2 branch?

martyf commented 1 year ago

So I've done a bit of playing and switched to YAML validation instead of JSON, and it is a lot more flexible when compared to the way you'd write config in JavaScript.

Can you please update to 2.1.0 and let me know how you go?

BuchyOne commented 1 year ago

Thanks for the prompt response.

I did use a linter and it did pass. I think my screenshot shows properly formatted JSON.

Perhaps the inclusion of the pipe character for the "toolbar" key is causing some issue. I will try a few more tests and get back.


In response to your question, if you have a JSON form field, then validation is important.

The approach might improve if the interface offered different editor setups (e.g. 'simple', 'standard', 'complete' ), and the related object assigned to the user's choices in the background.

e.g. user selects 'simple', sets this as default, names it and saves.

The last of those different setups could the the 'custom' setup - and would show the current DIY configuration textarea.


Lastly, v2 did not work when I went to edit a page, so I'm not sure if the key worked.

BuchyOne commented 1 year ago

hah! simultaneity ...

right, the yaml format would be a very good way to go

martyf commented 1 year ago

Cool cool so update to 2.1.0 and see how you go - the YAML validating is more forgiving.

I get what you're saying (to essentially bypass the validation), which could be a good addition too. But also validating is useful to help highlight and troubleshoot errors before getting to the editor screen - if that's where errors start appearing, it may be more confusing to find.

I can't replicate the key issue - I put my key in the .env file, and when I load an editor page which uses the Tiny editor, it loads the editor as expected. If you look at the URL for the first tinymce.min.js call, your API key should be in the URL.

BuchyOne commented 1 year ago

Will do update and test today and get back.

And check for the API key in the js url.

Thanks for your time and effort.

BuchyOne commented 1 year ago

Hi,

Tested and no issues saving the JSON. So parsing to yaml has solved that.

Pages are editable (after resaving blueprints to update config), and I see no errors in logs.

My issue is completely solved.

Thank you very much.

martyf commented 1 year ago

Hoorah I'm glad!

I'll close off this issue.