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

[FEATURE] Adding custom GrapeJS plugins to customize the editor #12429

Merged
merged 5 commits into from Apr 25, 2024

Conversation

Moongazer
Copy link
Contributor

@Moongazer Moongazer commented Jun 5, 2023

Problem description

Currently it is very difficult to customize Mautic's editor (GrapesJS) to fit the requirements of a customer-project. The only way is to create a fork of grapesjs-preset-mautic, make the adjustments, re-compile the assets, and overwrite them inside the GrapesJsBuilderBundle. Especially for automatic deployments this is pretty much a huge hassle.

Solution proposal

The GrapesJS library has a plugin API (which grapesjs-preset-mautic uses also). We just need a way to allow other plugins being loaded additionally beside Mautic's changes of the editor. This PR does exactly that: other custom GrapesJS plugins (e.g. distributed through a custom plugin-bundle) can register itself in a place where Mautic's builder.service finds and adds them during initialization of the editor.

Technical implementation

My main target was to find a simple and most-flexible solution for this problem with a very loose coupling. As inspiration I took the way of how GoogleAnalytics or other cliendside tracking-software using the so called Data-Layer to communicate with 3rd party scripts. This solution is basically bullet-proof easy because there is just one global window object through which all the communication runs. Its very loose coupled, only the loading-order matters (meaning, all plugins have loaded first before the actual editor is initialized). But using the AssetsSubscriber, this is already the case.

This PR modifies the builder.service.js to look for a window.MauticGrapesJsPlugins object (which holds an array of plugin-objects), and adding the plugins with its options to the editor on initializsation (if there are any).

How to create and add a custom GrapesJS plugin

  1. Create a new Mautic plugin-bundle, e.g. GrapesJsCustomPluginBundle
  2. Create a GrapesJS plugin (./Assets/src/index.ts) like this. I'm using TypeScript here, but also Vanilla JS works:
import grapesjs from 'grapesjs';

// declare type for window so TS will not complain during compiling
declare global {
    interface Window {
        MauticGrapesJsPlugins: object[];
    }
}

export type PluginOptions = {
};

export type RequiredPluginOptions = Required<PluginOptions>;

const GrapesJsCustomPlugin: grapesjs.Plugin<PluginOptions> = (editor, opts: Partial<PluginOptions> = {}) => {
    const options: RequiredPluginOptions = {
        ...opts
    };
    console.log('Run GrapesJsCustomPlugin...')
    console.log('Options passed to GrapesJsCustomPlugin:', options)
    editor.on('load', () => {
        console.log('GrapesJsCustomPlugin: editor.onLoad()')
    });
}

// export the plugin in case someone wants to use it as source module
export default GrapesJsCustomPlugin;

// create a global window-object which holds the information about GrapesJS plugins
if (!window.MauticGrapesJsPlugins) window.MauticGrapesJsPlugins = [];
// add the plugin-function with a name to the window-object
window.MauticGrapesJsPlugins.push({
    name: 'GrapesJsCustomPlugin', // required
    plugin: GrapesJsCustomPlugin, // required
    context: ['page', 'email-mjml'], // optional. default is none/empty, so the plugin is always added; options: [page|email-mjml|email-html]
    pluginOptions: { options: { test: true, hello: 'world'} } // optional
})

Because of the export default, this plugin can still be used in a fork customizing the source files with import GrapesJsCustomPlugin from 'path'. But this is not required! One can also write a plain vanialla JS function like described here.

  1. Add the JavaScript file (compiled or source) to the AssetSubscriber of your plugin-bundle:
    public function injectAssets(CustomAssetsEvent $assetsEvent): void
    {
        if ($this->config->isPublished()) {
            $assetsEvent->addScript('plugins/GrapesJsCustomPluginBundle/Assets/dist/index.js');
        }
    }

The resulting HTML source looks like the following (the plugin code is loaded before builder.js, so its data is registered in the global window object):

<script src="/plugins/GrapesJsCustomPluginBundle/Assets/dist/index.js?v6e9fccee" data-source="mautic"></script>
<script src="/plugins/GrapesJsBuilderBundle/Assets/library/js/dist/builder.js?v6e9fccee" data-source="mautic"></script>

You can download this GrapesJsCustomPluginBundle for testing the PR here: https://github.com/Moongazer/GrapesJsCustomPluginBundle

Summary

Just to sum up the main pros and cons of this solution:

Pros:

  • Full backward compatible
  • Loose coupling of custom plugin-bundles and Mautic's core
  • Full flexible: it really doesnt matter HOW the custom plugin was written/transpiled/compiled (ES5, ES6, AMD, UMD, TypeScript, Vanilla JS,...). It just needs to be a function which is added to the global window.MauticGrapesJsPlugins object
  • JavaScript's module system can still be used

Cons (not really):

  • No real interface for the plugin-registration in window.MauticGrapesJsPlugins (needs to be documented)
  • For modifications of grapesjs-preset-mautic still a fork is required

Anything else? Please feel free to discuss or improve the solution! Apprechiate any feedback.

Note:

If you want this feature in Mautic 4, you will need to apply this patch to your instance: #12692

@RCheesley RCheesley added T3 Hard difficulty to fix (issue) or test (PR) enhancement Any improvement to an existing feature or functionality builder-grapesjs Anything related to the GrapesJS email or landing page builders ready-to-test PR's that are ready to test code-review-needed PR's that require a code review before merging labels Jun 5, 2023
Copy link
Member

@kuzmany kuzmany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this idea and would like to see it implemented in M5.
But I fetch it, run composer install, install the plugin, and enable it.

But I see no logs in the browser console.
Can you help me with make proper tests?

@Moongazer
Copy link
Contributor Author

I like this idea and would like to see it implemented in M5. But I fetch it, run composer install, install the plugin, and enable it.

But I see no logs in the browser console. Can you help me with make proper tests?

@kuzmany
Probably you didn't compile the JavaScript sources to create the patched builder.js file. I was not sure if I should attach the compiled file to this PR, because I think this will result in conflicts during a possible merge (IMO the builder.js file should be fresh compiled from sources for any release).

@kuzmany
Copy link
Member

kuzmany commented Sep 22, 2023

@Moongazer ah, understand. I think it was built during the composer install, like others npm packages.
You can include it in PR, If there will be conflicts later, you can recompile it again.

@Moongazer
Copy link
Contributor Author

Moongazer commented Sep 22, 2023

I think there was another reason why I didn't add the compiled builder.js: I just tried again to compile it via npm run build, but it failed with

⚠️  Could not load existing sourcemap of "../../../node_modules/grapesjs-parser-postcss/dist/grapesjs-parser-postcss.min.js".
🚨  /Users/moongazer/workspace/mautic-moongazer/plugins/GrapesJsBuilderBundle/Assets/library/js/builder.service.js:9:31: Cannot resolve dependency 'grapesjs-preset-mautic/dist/editorFonts/editorFonts.service'
   7 | import grapesjsmautic from 'grapesjs-preset-mautic';
   8 | import mjmlService from 'grapesjs-preset-mautic/dist/mjml/mjml.service';
>  9 | import editorFontsService from 'grapesjs-preset-mautic/dist/editorFonts/editorFonts.service';

So I think the M5 repo from where I branched off for this PR was broken already. The node package grapesjs-preset-mautic in my node_modules directory does not have a folder editorFonts/.

Edit:
@kuzmany I rebased the PR with the current 5.x and it seems the previous issue was solved. You will find the compiled builder.js now being included here.

@santiant
Copy link

It would be so cool to have this functionality. I have some customers who require the development of special GrapesJS blocks to provide integration with e-commerce catalogs/products and allow them to easily compose emails with the products...

The problem is I found there is no documentation supporting this kind of extensibility and seeing this pull request means there was not. I really hope it gets merged because I don't feel comfortable by modifying the core files.

kuzmany
kuzmany previously approved these changes Sep 29, 2023
Copy link
Member

@kuzmany kuzmany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works. I vote to merge it to M5 Beta

@mollux
Copy link
Contributor

mollux commented Sep 29, 2023

@LordRembo probably related to investigation you're doing :)

@kuzmany
Copy link
Member

kuzmany commented Sep 29, 2023

@escopecz consider it to merge M5 beta. It break nothing and bring way to extend builder

@mollux
Copy link
Contributor

mollux commented Sep 29, 2023

@kuzmany I'll check, indeed an nice addition!

Copy link
Sponsor Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this!

  1. How come it's removing so much lines from package-lock.json?
  2. Could you copy the documentation from this PR description to https://github.com/mautic/developer-documentation-new?

@escopecz escopecz added pending-test-confirmation PR's that require one test before they can be merged needs-documentation PR's that need documentation before they can be merged and removed ready-to-test PR's that are ready to test labels Oct 2, 2023
@Moongazer
Copy link
Contributor Author

@escopecz Regarding to 1): Not sure, but maybe it has to do with the rebasing? At the time I branched off, the NPM stuff was broken (that's why I couldn't compile the builder.js in the first place). Two weeks ago I rebased to the current 5.x branch and than it was working, so I guess other PRs have modified and fixed the package.json. For 2): I will see what I can do.

@RCheesley
Copy link
Sponsor Member

Hi folks, if we can resolve the conflicts and get the docs together, we should be able to get this moving again.

@RCheesley RCheesley added the has-conflicts Pull requests that cannot be merged until conflicts have been resolved label Apr 19, 2024
@Moongazer Moongazer force-pushed the feat/grapesjs-custom-plugins branch from a7bf405 to d37dec8 Compare April 19, 2024 19:16
@Moongazer
Copy link
Contributor Author

@RCheesley @escopecz I rebased to the current 5.x branch, so all conflicts should be resolved (not sure why the message above still showing "This branch has conflicts that must be resolved").

Unfortunately I don't have time to copy the description to docs (precisely: don't have time to setup the repo, figure out how to init and add everything, especially messing around with Python 3 installation on my Linux etc...). I'd appreciate if someone else could do this who has set up the docs repo already, thx 🙏🏼

@LordRembo
Copy link
Contributor

@Moongazer Depends on what files are giving the conflicts. I had a similar problem and I saw the conflicts locally only when checking out the repo again in a new folder and pulled in all the 5.x changes, then did a git merge of 5.x on top of my branch. You'll probably have to do a similar thing.
At the very least, you'll have to install the node modules (npm i) and rebuild the GrapesJS dist folder files (npm run rebuild), because there was a big update added to 5.x today. Your dist changes won't be properly merged without it.

@Moongazer Moongazer force-pushed the feat/grapesjs-custom-plugins branch from d37dec8 to 54943f4 Compare April 22, 2024 15:33
@Moongazer
Copy link
Contributor Author

@LordRembo OK I've rebased to current 5.x and built the assets again, please merge ASAP!!

@LordRembo LordRembo removed the has-conflicts Pull requests that cannot be merged until conflicts have been resolved label Apr 23, 2024
escopecz
escopecz previously approved these changes Apr 23, 2024
Copy link
Sponsor Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it OK to upgrade all dependencies in this PR? It would be easier to merge if the dependencies would be upgraded in a separate PR. But if the GrapesJS team decides this is OK then I'm also fine with it.

@RCheesley
Copy link
Sponsor Member

I've written the docs here so it doesn't block merging if this can be tested in time for the release: mautic/developer-documentation-new#187 - please review it as I've just written what was here plus grammar fixes to align with the styleguide. Thanks!

@RCheesley RCheesley removed the needs-documentation PR's that need documentation before they can be merged label Apr 23, 2024
@LordRembo
Copy link
Contributor

@escopecz The dependency updates are all minor updates (and of non-grapesjs dependent) node modules, so it should be okay. Maybe we should lock the versions in package json to only bugfix updates in future, I'll post the question in Slack and see what everyone thinks.

@Moongazer
Copy link
Contributor Author

@escopecz @LordRembo I had to delete the package-lock.json because the npm i failed (first I had to upgrade my node from v16.13 to v16.14, than the installation failed again because of some Python compile error. Deleting the file fixed it).

Copy link
Sponsor Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion implementation! Re-approving 👍

@Moongazer Moongazer force-pushed the feat/grapesjs-custom-plugins branch from 2ca301e to 5b2e2c9 Compare April 23, 2024 20:09
Copy link

codecov bot commented Apr 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.39%. Comparing base (bfb2e7f) to head (4c6714a).
Report is 9 commits behind head on 5.x.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                5.x   #12429      +/-   ##
============================================
- Coverage     61.39%   61.39%   -0.01%     
  Complexity    34024    34024              
============================================
  Files          2238     2238              
  Lines        101686   101686              
============================================
- Hits          62431    62430       -1     
- Misses        39255    39256       +1     

see 1 file with indirect coverage changes

@escopecz
Copy link
Sponsor Member

@LordRembo from your comment I don't understand whether you approve or want some changes. Can you make it clear?

@LordRembo
Copy link
Contributor

@escopecz The comment was just to remark that the updates to the lock file should not be a blocker for the ticket. Haven't had time to do any testing, so I don't have a review for it. If it works for you and Kuzmany, that's good for me.

@escopecz escopecz added this to the 5.1.0 milestone Apr 25, 2024
@escopecz escopecz added code-review-passed PRs which have passed code review and removed code-review-needed PR's that require a code review before merging pending-test-confirmation PR's that require one test before they can be merged labels Apr 25, 2024
@escopecz escopecz merged commit 993ec0e into mautic:5.x Apr 25, 2024
15 of 16 checks passed
@Moongazer Moongazer deleted the feat/grapesjs-custom-plugins branch April 25, 2024 12:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builder-grapesjs Anything related to the GrapesJS email or landing page builders code-review-passed PRs which have passed code review enhancement Any improvement to an existing feature or functionality T3 Hard difficulty to fix (issue) or test (PR)
Projects
Status: 🥳 Done
Development

Successfully merging this pull request may close these issues.

None yet

7 participants