Skip to content
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

Upgrade CMS5 to use TinyMCE6 #10369

Closed
5 tasks done
maxime-rainville opened this issue Jun 25, 2022 · 7 comments
Closed
5 tasks done

Upgrade CMS5 to use TinyMCE6 #10369

maxime-rainville opened this issue Jun 25, 2022 · 7 comments

Comments

@maxime-rainville
Copy link
Contributor

maxime-rainville commented Jun 25, 2022

Acceptance criteria

  • Silverstripe CMS 5 uses TinyMCE6
  • All existing WYSIWYG functionality is retained.
  • Skin is replicated See Create tinymce6 skin to match the old tinymce4 one silverstripe-admin#1366
  • Our current plugins are upgraded to work
  • Upgrade docs are updated to reflect any changes developers will need in their own projects (within reason - we can just link to tinymce's notes where appropriate)
  • Documentation that mentions tinymce generally is updated

Note

  • TinyMCE has a react integration. There might be some value in converting our HTMLEditor field to be a pure react component. But that's a secondary concern.
  • "Paste from word" and "spell check" plugins are now premium plugins. We might need to validate how much of a pain loosing those will be.

PRs

Fix PRs

@maxime-rainville
Copy link
Contributor Author

@GuySartorelli will have an initial look at to what has changed between TinyMCE 4 and 6.

@GuySartorelli
Copy link
Member

GuySartorelli commented Jul 11, 2022

Had a bit of a look around and a play.

Download options

  1. Download .zip file from https://www.tiny.cloud/get-tiny/ and include in thirdparty dir
    • This is how we do it atm but it's not ideal. Makes it harder to update.
  2. Install via yarn: https://www.tiny.cloud/docs/tinymce/6/npm-projects/#install-tinymce-using-npm-or-yarn
    • Just the regular JS without the official react component.
  3. Official react component: https://github.com/tinymce/tinymce-react
    • I haven't look much into this - we already have our own react component and this one doesn't seem to add much. If anything it would probably be more work to rip out our one and re-do our code to use this one.
    • I was able to get a POC tinymce6 editor running fine without changing anything in our existing react component.

Upgrade path

There is docs for upgrading from 4 to 5, and from 5 to 6. We can use those to figure out what has been deprecated or removed from 4 to 6.
Docs for upgrading from 4 to 5
Docs for upgrading from 5 to 6

The following are things I know we'll have to make changes for:

  • The paste, spellchecker and contextmenu plugins have been removed.
    • Spellchecker functionality is now premium, though the browser_spellcheck option still works.
    • Context menus are now part of core instead of a separate plugin
    • Paste is now part of core instead of a separate plugin
      • Word paste functionality is explicitly not supported - there is a call for maintainers if we think this is worth supporting (which IMO it is)
      • There is a premium powerpaste plugin that includes pasting from word
  • The mode option has been deprecated and is no longer used
  • The 'modern' theme has been replaced with the 'silver' theme.
  • Third-party skins (i.e. our skin) should be applied using the skin_url option: https://www.tiny.cloud/docs/tinymce/6/editor-skin/#skin_url
  • Various plugin API have changed: https://www.tiny.cloud/docs/migration-from-4x/#plugins
    • This will mean we need to change our sslink, ssemail, and other custom plugins
  • There are a few changes to events firing that we may need to look into
  • Our changetracker js needs a slight modification.
    - var serializer = new tinymce.html.Serializer(config);
    - var parser = new tinymce.html.DomParser(config);
    + var serializer = tinymce.html.Serializer(config);
    + var parser = tinymce.html.DomParser(config);

@GuySartorelli
Copy link
Member

GuySartorelli commented Sep 20, 2022

@silverstripe/core-team
While upgrading I've run into a scenario where it will be easier to make a change than to retain the existing behaviour, and I'd like some feedback to decide whether to make the change or not.

Existing behaviour

  • When saving content from the visual editor into the underlying textarea element, a conversion takes place where content added through the embed or media plugins (i.e. embed a youtube video or insert an image) is converted from its HTML markup to shortcode markup.
  • When loading content from the textarea to the visual editor, those shortcodes are converted back to their HTML markup
  • This conversion is different from the conversion in PHP - i.e. the HTML markup in the visual editor is not identical to the markup that will be displayed on the front end.
  • This HTML markup is not only used in the visual editor but also displayed in the code editor.

code editor showing the temporary html markup

Proposed new behaviour

  • This conversion happens when getting content from and setting content into the visual editor. The difference here is that the HTML markup will only be used inside the visual editor as a visual placeholder - but the code editor will show the shortcode markup as it will be stored in the database

code editor showing the shortcode markup

Reasoning

  • The SaveContent event has changed in tinymce6. I can't see any documentation about this change, but although the event fires the new content (with the shortcode markup) doesn't actually end up in the textarea which means it gets stored in the database with the HTML markup which was only intended as a placeholder for the WYSIWYG visual editor
    • The event is listed as supported in this list but is completely absent from this list which leads me to believe the documentation in that first list is simply outdated and it's not actually supported anymore.
  • The temporary HTML markup displayed in the visual editor is misleading anyway - if you modify that markup for the embed, it may result in the embedded shortcode not actually working as expected. If we instead display the actual shortcode inside the code editor it is clearer what will actually be affected
  • Link shortcode and custom shortcodes already display in the code editor as shortcodes - doing this with embeds and images will be more consistent

@emteknetnz
Copy link
Member

This seems like a good change to me.

As I understand it, this would only have an effect on end users when clicking the "view source" button, which is itself is probably a fairly narrow use case for a content editor. The fact they're seeing shortcodes instead of html tag does seem like it's that big of a deal. They can still see what's going on.

As a developer this is improvement as what's I see when I click "view source" is closer to what's in the database.

I also like that fact we'd be removing some "duplicate yet different" conversion logic

@GuySartorelli
Copy link
Member

To be clear, the conversion logic is still going to be there - it's necessary for the visual WYSIWYG editor to display the image or embed placeholder. It's just that the dummy HTML markup will only be used inside the visual editor where before it was used anywhere tinymce used the markup.

@kinglozzer
Copy link
Member

I’m fine with this change. In TinyMCE 4, if you view source and edit the “HTML” for a shortcode (that is, the placeholder HTML) then presumably you’ll lose any edits anyway?

As an aside, how flexible/“pluggable” is the placeholder conversion logic? We sometimes override shortcodes to change how they’re rendered on the frontend, it’d be great to be able to update the placeholder conversion logic so the visual representation is closer to the rendered content.

@GuySartorelli
Copy link
Member

GuySartorelli commented Sep 27, 2022

In TinyMCE 4, if you view source and edit the “HTML” for a shortcode (that is, the placeholder HTML) then presumably you’ll lose any edits anyway?

Depends what the edits are. The image shortcode will accept any attributes you add to it - that will be the case in its shortcode syntax form as well. The embed shortcode won't, though, so edits on the temp-markup for that would be mostly lost.

how flexible/“pluggable” is the placeholder conversion logic?

I don't think it would be possible to extend or replace it with your own conversion logic with the way it's written at the moment. Check out the SaveContent and BeforeSetContent event handlers in TinyMCE_ssembed.js for an example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants