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

DPMMA-2881 fix Grapesjs-Mjml self-closing mj-spacer tag issue #14142

Open
wants to merge 1 commit into
base: 5.x
Choose a base branch
from

Conversation

patrykgruszka
Copy link
Member

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

Description

This pull request resolves an issue where self-closing tag <mj-spacer /> were causing rendering problems in Grapesjs-Mjml.

Fix for other self-closing tags was introduced in #13431, this PR just adds mj-spacer, to list of void tags.


📋 Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
    1: Go to Channels > Emails
    2: Create new
    3: Select "New template email"
    4: Fill in a name for "Subject" and "Internal name"
    5: Select the "Brienz" theme - because it uses MJML
    6: Go to "Builder"
    7: Click on the pencil icon that corresponds to "Edit code"
    8: Delete all the code and paste this code:
<mjml>
  <mj-body>
    <mj-section>
      <mj-column>
        <mj-text font-size="20px">top text</mj-text>
        <mj-spacer/>
        <mj-text font-size="20px">bottom text</mj-text>
      </mj-column>
    </mj-section>
  </mj-body>
</mjml>

10: Click save
11: Check if it's rendered correctly: top and bottom text should have a spacer element between
image

Copy link
Contributor

@tomekkowalczyk tomekkowalczyk left a comment

Choose a reason for hiding this comment

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

Change looks good ;)

@patrykgruszka patrykgruszka added bug Issues or PR's relating to bugs ready-to-test PR's that are ready to test email Anything related to email builder-grapesjs Anything related to the GrapesJS email or landing page builders code-review-passed PRs which have passed code review labels Sep 24, 2024
Copy link

@Aleks0129 Aleks0129 left a comment

Choose a reason for hiding this comment

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

Does what it says
image

@patrykgruszka patrykgruszka added pending-test-confirmation PR's that require one test before they can be merged and removed ready-to-test PR's that are ready to test labels Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues or PR's relating to bugs builder-grapesjs Anything related to the GrapesJS email or landing page builders code-review-passed PRs which have passed code review email Anything related to email pending-test-confirmation PR's that require one test before they can be merged
Projects
Status: ⏳︎ Needs 1 more test
Development

Successfully merging this pull request may close these issues.

3 participants