Update grapesjs compatibility (Mautic 5)

[LordRembo]

Registering your custom components using the legacy method breaks when updating GrapesJS to release 0.20.1. Need this to be able to upgrade GrapesJS in the GrapesJSBuilderBundle.

Here is a PR for Mautic 4

To test:

• have a Mautic 5.x running locally
• replace the package with my forked branch in plugins/GrapesJsBuilderBundle, eg. by running npm i grapesjs-preset- mautic@github:LordRembo/grapesjs-preset-mautic#update-grapesjs-m5 inside that bundle
• rebuilding GrapesJs bundle by running npm run build
• Make or edit an email and see that you can still use drag & drop to add/move/edit a Dynamic content Block.
• Check that there are no new JS errors
• You should still be able to save that edited mail

[LordRembo]

It would be helpful if someone testing this, can put special attention to any JS errors in GrapesJs and if there are any unexpected font differences with the 5.x branch.

[LordRembo]

About the changes:

• The code changes outside the dist folder were done by hand, the rest is done by running the build script
• The most important change is, some syntax that was already deprecated becomes invalid when updating to GrapesJs 0.21.1 so I replaced that
• I had to make a second change, and that is to check for the 'options' property if 'list' doesn't exist, when checking the list of fonts in the Editor. 'options' is how it is listed in the documentation of current GrapesJs, I think 'list' is older syntax perhaps. In any case, I made both possible now.

[patrykgruszka]

The code updates seem good. But I can't check the mail builder because there's an issue with the 5.x branch: mautic/mautic#13510.

[RCheesley]

I think the description should maybe say you need a local copy of 5.x rather than 4.x?

[LordRembo]

@RCheesley ah, right, sorry. Changed it.

[LordRembo]

It looks a bit like the Dynamic Content block is simply not properly rendered (shows the mj-text tag but should render a table row and cell with the same attributes).
Looking in src/dynamicContent/dynamicContent.domcomponents.js and related files to see how the component is built up, there doesn't seem to be anything major broken. A couple of things I can tweak that was already wrong in the original (eg. isComponent check should be moved 'cause it doesn't trigger here, some other checks that could be better) but they don't seem to be the root cause of the render issue.
Will try finding time to look at it a bit deeper next week.

[LordRembo]

@adiux I see that you have worked extensively on the Dynamic content block. I could use a bit of help figuring out why this is broken, all of a sudden:

• the output is all of a sudden no longer rendered as a tr/td but just an mj-text tag
• any added Dynamic content blocks are no longer stored. You can't edit them anymore after closing and reopening a mail (No Dynamic Content editor found for decId )

All I updated for it, was change the use of the legacy API with the new one (as per these release notes) but I don't understand enough about the Dynamic content block to see where/why these issues occur.
I've cleaned up the code here and there, hoping to find some syntax issues but that doesn't seem to be the case.

[adiux]

Hi @LordRembo as far as I understand it the DC is stored in the DOM (#dynamicContentContainer) of the editor pages HTML. The decId is used to link it to a DC editor (where you actually edit the DC). The data-param-dec-id is persisted to the db and can be used to link to the item store in the html. The store id starts with 0. So decId is always + 1.

There is a GJS command that transforms the token ({dynamiccontent="Dynamic Content 2"}) to the representation in the UI.

Not sure why there is only mj-text, but I would start fixing this error. Hope this helps. Let me know if you have questions.

[LordRembo]

@adiux Seems like the 'remove' event is fired whenever the Builder is opened, and that removes the item. That should not be the case I think? You have any idea if this code can be safely removed?

  // @todo merge events and listeners. or move this to the component itself as a
  // local listener. see create-new-dynamic-content-store-item
  onComponentRemove() {
    this.editor.on('component:remove', (component) => {
      // Delete dynamic-content on Mautic side
      if (component.get('type') === 'dynamic-content') {
        console.debug('dynamic content remove');
        this.editor.runCommand('preset-mautic:dynamic-content-delete-store-item', { component });
      }
    });
  }

[adiux]

@LordRembo just from this snipped, this seems to be needed. Otherwise, how would a DC be removed?

Why is it fired on the opening of the builder. That sounds strange to me.

[LordRembo]

@adiux Without it, the blocks do get removed in code but it's only an empty reference that is kept in storage. Which I would agree is at least good practice to clean up.
I figured out the problem over the weekend: there's an extra condition needed to make sure only the block that is selected for removal, is actually removed. Doesn't make any sense that this isn't filtered beforehand but that's GrapesJS I guess. Found references to this in a GrapesJS ticket from 2022.

[LordRembo]

Alright, issues fixed, updated the grapesjs update PR to use the latest changes. Refer to that one for QA.

[LordRembo]

Just practically, I think we need a 5.1 branch to merge this thing, because it is used for the GrapesJS upgrade PR and that one is planned for a 5.1 release.
@escopecz or @mollux Do you think this is a good solution? If so, maybe one of you could make one?

[escopecz]

I created the 5.1 branch

[LordRembo]

Alright! The PR that depends on this update, has its required 2 approval reviews. So this preset should be good to merge (against the 5.1 branch).
Afterwards, I'll update the reference in that other PR.