-
Notifications
You must be signed in to change notification settings - Fork 967
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
Add support for Included files to have mj-head #1585
Comments
It should be already the case if you define a mjml root tag and a mj-head inside any mj included file
… On 23 Apr 2019, at 21:16, Jon McLaren ***@***.***> wrote:
Is your feature request related to a problem? Please describe.
For a client I have to modularize all of our common Email components, so I'm separating them into includes. Right now I have to have the relevant mj-styles and mj-attributes in my parent file. I'd like to be able to put the styles relevant to a specific included file, within that child file itself, to keep the code clean and easy to maintain.
Describe alternatives you've considered
Someone had told me you can do mj-include inside of an mj-attributes which is great but requires me to have separate files for the module and it's styles.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
If that is true then perhaps it's an issue with the MJML App and not MJML itself. The app Bombs when you try it. so I assumed it was an issue with MJML itself. |
Please share some code, it should be handled properly within the app too. We just don’t look for infinite loop
… On 23 Apr 2019, at 21:25, Jon McLaren ***@***.***> wrote:
If that is true then perhaps it's an issue with the MJML App and not MJML itself. The app Bombs when you try it. so I assumed it was an issue with MJML itself.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Working on a reduced test case, I am seeing that when there's nothing else in the email it's okay with the mj-head being in the child file. Same exact code for how I'm including it, and in the included file, just without the rest of the contents of the email. Will post when I figure out what thing it doesn't like. |
Okay I was able to get a really simplified reduced test case going, It appears to be related to having other includes in the parent file.
REDUCED-TEST-INCLUDE.MJML (produces a red rectangle)
REDUCED-TEST-PROBLEM.MJML
|
Updated the files to remove everything that was not necessary to reproduce/tell if you're seeing the issue. Unless I'm missing something insanely obvious - there appears to be a bug in the way it's compiling. When i try the copy html button in the mjml app it doesn't copy anything so that makes me think it's not compiling and is likely an issue with MJML itself, not the app. I'm using MJML app version 2.11.0 on Mac OS X 10.14.4 Oh and to confirm I am including the files properly, if I add text to the reduced-test-problem.mjml file and comment out the include to |
FYI, I tested this with MJML 4.4.0-beta.1 without using the MJML app, and the multiple/duplicate includes looks to functioning correctly. |
Duplicate? |
Sorry, I misread your previous example. Without using the app, I was able to include an MJML partial in my index with its styles contained in its index.mjml
testPartial.mjml
I did notice that if I try to include another MJML partial before the closing Example code that creates the error is below. index.mjml
testPartial.mjml
testPartialTwo.mjml
|
We'll fix this in the next beta then if you can reproduce on cli good job !
…On Wed, Apr 24, 2019 at 8:31 PM stephenrust ***@***.***> wrote:
Sorry, I misread your previous example.
Without using the app, I was able to include an MJML partial in my index
with its styles contained in mj-head. It compiles as expected.
*index.mjml*
<mjml>
<mj-head>
<mj-attributes>
<mj-class name="green" color="green" />
</mj-attributes>
</mj-head>
<mj-body>
<mj-section>
<mj-column width="100%">
<mj-text mj-class="green">
Basic column here. It's green.
</mj-text>
</mj-column>
</mj-section>
<mj-include path="./partials/testPartial.mjml" />
</mj-body>
</mjml>
*testPartial.mjml*
<mjml>
<mj-head>
<mj-attributes>
<mj-class name="red" color="red" />
</mj-attributes>
</mj-head>
<mj-body>
<mj-section>
<mj-column width="100%">
<mj-text mj-class="red">
Include 1 is here. It's red.
</mj-text>
</mj-column>
</mj-section>
</mj-body>
</mjml>
I did notice that if I try to include another MJML partial before the
closing mj-body tag, the app errors out with a Error while rendering the
file : TypeError: addMediaQuery is not a function at MjColumn.getColumnClass
.
Example code that creates the error is below.
*index.mjml*
<mjml>
<mj-head>
<mj-attributes>
<mj-class name="basic-column" color="green" />
</mj-attributes>
</mj-head>
<mj-body>
<mj-section>
<mj-column width="100%">
<mj-text mj-class="basic-column">
Basic column here. It's green.
</mj-text>
</mj-column>
</mj-section>
<mj-include path="./partials/testPartial.mjml" />
<!-- Including the second partial below causes the error -->
<mj-include path="./partials/testPartialTwo.mjml" />
</mj-body>
</mjml>
*testPartial.mjml*
<mjml>
<mj-head>
<mj-attributes>
<mj-class name="red" color="red" />
</mj-attributes>
</mj-head>
<mj-body>
<mj-section>
<mj-column width="100%">
<mj-text mj-class="red">
Include 1 is here. It's red.
</mj-text>
</mj-column>
</mj-section>
</mj-body>
</mjml>
*testPartialTwo.mjml*
<mjml>
<mj-head>
<mj-attributes>
<mj-class name="blue" color="blue" />
</mj-attributes>
</mj-head>
<mj-body>
<mj-section>
<mj-column width="100%">
<mj-text mj-class="blue">
Test include 2 is here. It's blue.
</mj-text>
</mj-column>
</mj-section>
</mj-body>
</mjml>
``
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1585 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAELHTI6LBADPJYD6U7SAOLPSCROTANCNFSM4HH5UU3Q>
.
|
Thank you for working on a fix! I'll be honest the fix goes over my head. Glad you guys who have more experience in this project are active. I don't know that I could have figured that fix out on my own. |
fix #1585 fix handling of mj-head and includedIn in mj-includes
Is your feature request related to a problem? Please describe.
For a client I have to modularize all of our common Email components, so I'm separating them into includes. Right now I have to have the relevant
mj-styles
andmj-attribute
s in my parent file. I'd like to be able to put the styles relevant to a specific included file, within that child file itself, to keep the code clean and easy to maintain.Describe alternatives you've considered
Someone had told me you can do
mj-include
inside of anmj-attributes
which is great but requires me to have separate files for the module and it's styles.The text was updated successfully, but these errors were encountered: