Added preference center blocks to GrapesJS builder #29
Conversation
You need put that strings to https://github.com/mautic/mautic/blob/4.x/plugins/GrapesJsBuilderBundle/Translations/en_US/javascript.ini |
|
@julienWebmecanik can you push also PR to official mautic repo with build assets? We need get it to ready to test state. Then I can ask community for testing and merge it |
|
This pull request has been mentioned on Mautic Community Forums. There might be relevant details there: https://forum.mautic.org/t/changing-style-of-default-unsubscribe-page/21341/6 |
| * Blocks for Preference Center Category | ||
| */ | ||
| //check if page is for preference center | ||
| if (mQuery('#page_isPreferenceCenter_1').is(':checked')) { |
There was a problem hiding this comment.
ContentService.isPreferenceCenter should be used here too
| @@ -54,8 +53,6 @@ export default class DynamicContentCommands { | |||
| convertDynamicContentComponentsToTokens(editor) { | |||
| // get all dynamic content elements loaded in the editor | |||
| const dynamicContents = editor.DomComponents.getWrapper().find('[data-slot="dynamicContent"]'); | |||
| this.logger.debug(`DC: ${dynamicContents.length} components found`, { dynamicContents }); | |||
There was a problem hiding this comment.
maybe this was removed by accident?
| ) { | ||
| if (typeof jQuery == 'undefined') { | ||
| var jquery = document.createElement('script'); | ||
| jquery.src = '//cdnjs.cloudflare.com/ajax/libs/jquery/3.4.1/jquery.min.js'; |
There was a problem hiding this comment.
I don't think it is a good idea to load the JavaScript dependencies inside the code. We should start using npm to manage our dependencies. I guess the idea of this is to only load it when needed?
Possible option:
Require it using npm and then load it from our codebase.
I guess things like jquery and bootstrap should be loaded anyway already in our codebase.
| style: { padding: '50px' }, | ||
| }); | ||
|
|
||
| const style = `<style> |
There was a problem hiding this comment.
According to grapesjs doc we should avoid putting styles in blocks. https://grapesjs.com/docs/modules/Blocks.html#important-caveats
would it work if we add it here? https://github.com/mautic/mautic/blob/5.x/plugins/GrapesJsBuilderBundle/Assets/library/js/grapesjs-custom.css
Also not ideal but it might allow the user and theme author to overwrite it
|
@julienWebmecanik thanks for this PR. It looks good to me so far. I found a couple of improvements that you could look at. Let me know if I can help. |
|
@julienWebmecanik please can you take a look at the feedback in the PR and address the conflicts? Thanks! |
|
@julienWebmecanik @npracht @kuzmany it would be great to get the conflicts and feedback addressed on this PR so we can test and merge it, any chance this can get some love? |
|
We vote for this to get some love, too ❤️ 🙏🏼 |
|
@julienWebmecanik please could you address the conflicts? We have testers waiting to help get this merged! Thank you! |
|
@RCheesley we'll not carry this anymore. We are on the way to use a different page builder we're building. This one is not good enough for marketeer. |
|
@RCheesley we are interested in completing this functionality. What remains to be done? |
|
Hey there, I think we need to address the conflicts, and then see were at in terms of the actual PR. I don't know how much was needing to be done because I can't test it with the conflicts. Would you be up for that @annamunk ? |
|
@RCheesley thank you! If that's okay, we'll discuss it with the team this week and I'll come back with an answer. |
|
Sure, sounds good! Thanks for being willing to consider it! |
|
Close favor to #46 |
Preference center finished based on #8.
A few notes :