-
Notifications
You must be signed in to change notification settings - Fork 73
DOC Document new tinymce module #691
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DOC Document new tinymce module #691
Conversation
770930f to
e2da7d9
Compare
en/02_Developer_Guides/15_Customising_the_Admin_Interface/08_Typography.md
Show resolved
Hide resolved
|
|
||
| See [defining HTML editor configurations](/developer_guides/forms/field_types/htmleditorfield/#defining-html-editor-configurations) for details about the new generalised API, and [optional features > TinyMCE HTML editor](/optional_features/htmleditor-tinymce/) for any TinyMCE-specific documentation. | ||
|
|
||
| #### If you want TinyMCE 7 anyway |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't support this, we haven't tested this, so I don't see why we're providing any instructions for how to do this. It's feels like it sending mixed messages, as well as needlessly making people read and comprehend more.
This should only be a single sentence saying that's it's unsupported, untested, though theoretically possible, however you're entirely on your own.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to give people a jump start if they want to do it. I don't think there's mixed messages - I've made it very clear this is untested and unsupported, but that at least these steps will be required if someone wants to attempt it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah ... this new license really does complicate things and people may not realise what that entails until after they've upgraded. I don't think this is something we should really be encouraging. Also we do intend to replace the editor which would be what we people should gravitate towards instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there another place you'd prefer I put this information for people who may want to upgrade to TinyMCE 7?
Or is there a way we can reduce the risk you perceive in providing it here? Additional warnings, hide it in a collapsible element, etc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now I've added a note into the warning about seeking legal advice before using GPL licensed code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha "seek legal advise" is a bit much
Is there another place you'd prefer I put this information
I can't really think of anywhere official. I just don't think we should be including this information. Even with warnings, it seems odd documenting how to do something we don't stand behind and could simply break at any point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just don't think we should be including this information.
Like I've said before, this is a question people are going to have. I don't want to be answering this question 20 different times for 20 different people - and I also don't want people floundering in the dark when we can at least give them a head start, because we do know that at least these two things will need to be done.
Your initial concern seemed to be that there would be confusion about it because we're saying we don't support it - so I added a really clear warning to remove that confusion.
Now your concern seems to be "it's not supported so don't say it" but I think it's sensible in a changelog (note specifically a changelog - I didn't put this in the main docs because that would be weird) to say "here's some help for how to migrate away from this if you want to" when a change is made (or in this case when a change would normally be made, but wasn't).
Ha "seek legal advise" is a bit much
I thought your concern was around confusion about the license, which is why I added that.
The language is based on the language TinyMCE use, to be fair. e.g tinymce/tinymce#9453 (comment)
The interpretation of this can vary based on your individual use case, so we recommend consulting with a legal expert to ensure compliance.
and https://www.tiny.cloud/blog/tinymce-7-release-highlights/
You should consult your legal team for compliance assurance around all your software usage.
5ad2645 to
5af1969
Compare
en/08_Changelogs/6.0.0.md
Outdated
| If you want to use TinyMCE 7 in your own projects, and have done your due diligence to validate that the new license won't cause you any legal problems, you can theoretically do that. | ||
|
|
||
| > [!WARNING] | ||
| > You should seek legal advice before using `GPL` licensed code in your project. | ||
| > | ||
| > Using TinyMCE 7 with Silverstripe CMS isn't supported and has not been tested. | ||
| > | ||
| > The plugins, skin, and configuration that come with `silverstripe/htmleditor-tinymce` are not guaranteed to work with TinyMCE 7. | ||
|
|
||
| Configure the [`TinyMCEConfig`](api:SilverStripe\TinyMCE\TinyMCEConfig) class to use your own copy of TinyMCE by setting the [`base_dir`](api:api:SilverStripe\TinyMCE\TinyMCEConfig->base_dir) configuration property to the directory which holds your TinyMCE JavaScript distribution files. | ||
|
|
||
| ```yml | ||
| SilverStripe\TinyMCE\TinyMCEConfig: | ||
| base_dir: 'path/to/tinymce7/directory' | ||
| ``` | ||
|
|
||
| You will also need to explicitly define the URL for the silverstripe skin if you want to use it: | ||
|
|
||
| ```php | ||
| // app/_config.php | ||
| use SilverStripe\Core\Manifest\ModuleLoader; | ||
| use SilverStripe\TinyMCE\TinyMCEConfig; | ||
| $moduleManifest = ModuleLoader::inst()->getManifest(); | ||
| $module = $moduleManifest->getModule('silverstripe/htmleditor-tinymce'); | ||
| $editorConfig = TinyMCEConfig::get('cms'); | ||
| $editorConfig->setOption('skin_url', $module->getResource('client/dist/tinymce/skins/ui/silverstripe')->getURL()); | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| If you want to use TinyMCE 7 in your own projects, and have done your due diligence to validate that the new license won't cause you any legal problems, you can theoretically do that. | |
| > [!WARNING] | |
| > You should seek legal advice before using `GPL` licensed code in your project. | |
| > | |
| > Using TinyMCE 7 with Silverstripe CMS isn't supported and has not been tested. | |
| > | |
| > The plugins, skin, and configuration that come with `silverstripe/htmleditor-tinymce` are not guaranteed to work with TinyMCE 7. | |
| Configure the [`TinyMCEConfig`](api:SilverStripe\TinyMCE\TinyMCEConfig) class to use your own copy of TinyMCE by setting the [`base_dir`](api:api:SilverStripe\TinyMCE\TinyMCEConfig->base_dir) configuration property to the directory which holds your TinyMCE JavaScript distribution files. | |
| ```yml | |
| SilverStripe\TinyMCE\TinyMCEConfig: | |
| base_dir: 'path/to/tinymce7/directory' | |
| ``` | |
| You will also need to explicitly define the URL for the silverstripe skin if you want to use it: | |
| ```php | |
| // app/_config.php | |
| use SilverStripe\Core\Manifest\ModuleLoader; | |
| use SilverStripe\TinyMCE\TinyMCEConfig; | |
| $moduleManifest = ModuleLoader::inst()->getManifest(); | |
| $module = $moduleManifest->getModule('silverstripe/htmleditor-tinymce'); | |
| $editorConfig = TinyMCEConfig::get('cms'); | |
| $editorConfig->setOption('skin_url', $module->getResource('client/dist/tinymce/skins/ui/silverstripe')->getURL()); | |
| ``` | |
| If you want to use TinyMCE 7 in your own projects, and have done your due diligence to validate that its [new license](https://www.tiny.cloud/docs/tinymce/latest/license-key/) won't cause you any legal problems, you can theoretically do that. | |
| Configure [`TinyMCEConfig`](api:SilverStripe\TinyMCE\TinyMCEConfig) class to use your own copy of TinyMCE by setting the [`base_dir`](api:api:SilverStripe\TinyMCE\TinyMCEConfig->base_dir) configuration property to the directory which holds your TinyMCE JavaScript distribution files e.g. `path/to/tinymce7/directory`. Then define the URL for the silverstripe skin if you want to use it: | |
| ```php | |
| // app/_config.php | |
| use SilverStripe\Core\Manifest\ModuleLoader; | |
| use SilverStripe\TinyMCE\TinyMCEConfig; | |
| $url = ModuleLoader::getModule('silverstripe/htmleditor-tinymce')->getResource('client/dist/tinymce/skins/ui/silverstripe')->getURL(); | |
| TinyMCEConfig::get('cms')->setOption('skin_url', $url); |
Compromise here is probably keep the information mostly infact, just condense it down
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried for something inbetween what I had and what you have.
- I want to explicitly keep the note that this isn't supported and may not work (because I don't want people asking us to make it work if it doesn't). I've moved it out of the callout block to compromise with your desire not to draw too much attention to this section.
- I removed the "e.g.
path/to/tinymce7/directory" reference because that's not meaningful without the yaml example. - I was going to keep that
$urlassignment to two lines but that's just not worth arguing about, the information is there even if it maybe wouldn't pass a peer review of a code PR.
I didn't realise ModuleLoader::getModule() existed, that's a nice find.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to keep that
$urlassignment to two lines but that's just not worth arguing about, the information is there even if it maybe wouldn't pass a peer review of a code PR.
lol nevermind, that doesn't pass linting. Made it two lines.
https://github.com/silverstripe/developer-docs/actions/runs/13847698077/job/38749338946?pr=691
5af1969 to
584d71b
Compare
|
Rebased |
ba12754 to
53c8327
Compare
53c8327 to
f02f1dd
Compare
|
Lint failure will be fixed by silverstripe/documentation-lint#19 |
Docs related to TinyMCE specifically have been either generalised or moved into the TinyMCE module.
A lot of the docs in the
HTMLEditorFieldpage have just been rearranged, so check if content already exists but just moved before suggesting changes.Issue