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.

Sign up for GitHub

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

Jump to bottom

remove Raw block from GrapesJS sidebar for MJML emails #13363

Open
wants to merge 13 commits into
base: 5.x
Choose a base branch
from
Open

remove Raw block from GrapesJS sidebar for MJML emails #13363

wants to merge 13 commits into from

Conversation

LordRembo
Copy link
Contributor

@LordRembo LordRembo commented Feb 13, 2024 •
edited

Q A
Bug fix? (use the a.b branch) [ ]
New feature/enhancement? (use the a.x branch) [x ]
Deprecations? [ ]
BC breaks? (use the c.x branch) [ ]
Automated tests included? [ ]
Related user documentation PR URL mautic/mautic-documentation#...
Related developer documentation PR URL mautic/developer-documentation#...
Issue(s) addressed Fixes #...

Description:

Opened up some discussion on the arguably broken state of the Raw block. My suggestion would be to remove this block entirely, since it's unusable.
It doesn't do what users think it does: renders a gallery of images instead of outputting editable html.
This seems to be intentional behavior, as it behaves the same in GrapesJS' own demo. But since it makes no sense, we are better or removing it instead of leaving users confused.

Note: This PR just removes the block, it does not attempt to replace the suggested functionality. That is a new feature in a separate PR, with its own caveats

Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Make a new MJML template based Email (eg. Paprika)
  3. Notice the 'Raw' block is no longer present in the sidebar

@LordRembo LordRembo marked this pull request as ready for review February 13, 2024 16:36
@LordRembo LordRembo mentioned this pull request Feb 13, 2024
Open
1 task
@code5rick
Copy link
Contributor

@LordRembo, after testing your PR on GitPod, I can confirm that it does remove the code block from the builder, more specifically on the email builder.

One thing I noticed is that the general font of Mautic seems off compared to the original. I believe you made changes to the stylesheet that affected the general layout.

image

Other than that, it seems OK!

@LordRembo
Copy link
Contributor Author

@code5rick You need to test against the 5.x branch of the project, not the 5.0.3 release. There is nothing here that changes the body font.
It's probably something that was changed after that release.

@mollux
Copy link
Contributor

mollux commented Feb 15, 2024

@code5rick @LordRembo the font changes are related to #13317 which was merged into 5.x.

As this PR targets the same branch, those font changes are here too

@LordRembo LordRembo removed the request for review from annamunk February 16, 2024 09:01
@LordRembo LordRembo added builder-grapesjs Anything related to the GrapesJS email or landing page builders T1 Low difficulty to fix (issue) or test (PR) enhancement Any improvement to an existing feature or functionality labels Feb 16, 2024
@RCheesley
Copy link
Sponsor Member

@LordRembo some conflicts to address here :)

@RCheesley RCheesley added the has-conflicts Pull requests that cannot be merged until conflicts have been resolved label Apr 19, 2024
@LordRembo
Merge branch '5.x' into remove-raw-block
e82182a 
# Conflicts:
#	plugins/GrapesJsBuilderBundle/Assets/library/js/builder.service.js
#	plugins/GrapesJsBuilderBundle/Assets/library/js/dist/builder.js
#	plugins/GrapesJsBuilderBundle/Assets/library/js/dist/builder.js.map
@LordRembo
rebuild grapesjs dist files
c106474
@LordRembo LordRembo removed the has-conflicts Pull requests that cannot be merged until conflicts have been resolved label Apr 22, 2024
@LordRembo
Copy link
Contributor Author

Should be okay now

@LordRembo
Copy link
Contributor Author

fixed the new merge conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
builder-grapesjs Anything related to the GrapesJS email or landing page builders enhancement Any improvement to an existing feature or functionality T1 Low difficulty to fix (issue) or test (PR)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@LordRembo @code5rick @mollux @RCheesley