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

Include fixes. #178

Merged
merged 5 commits into from
Aug 26, 2023
Merged

Include fixes. #178

merged 5 commits into from
Aug 26, 2023

Conversation

SebastianStehle
Copy link
Owner

This PR contains a wide range of fixes, basically to address issues with include as mentioned or discovered in #177

1. mj-attribute can be inside mj-body

mj-attribute can be inside a mj-body with mj-include. Therefore the child elements can actually change the parent elements as well but because of attribute inheritance parents can also affect the children. Therefore we need 2 passes:

  1. Read the whole MJML and store all attributes in binders
  2. Make the binding process.

Therefore we cannot reuse the binder anymore, which will reduce the performance a little bit.

2. mj-attribute affects all components

Before we have only handled binded attributes but in a few cases we get the child attributes from a component (see column or section).

This is also resolved because we had to store the binder now where we can solve that easily.

3. Child components of an include must be the child of the parent.

Because of parent-child inheritance we must just ignore that the mj-include even exists. This has been fixed by using the parent of the mj-include when parsing the included content.

I am not sure if mjml and mjml-body in an included file are also dummies that need to be flattened.

4. Simplify validators

We have the full component tree anyway.

5. Allow multiple heads

Copy link
Collaborator

@LiamRiddell LiamRiddell left a comment

Choose a reason for hiding this comment

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

Quite the handful of fixes, it's interesting how many edge cases there is with this project.

@SebastianStehle SebastianStehle merged commit c5bbdb7 into main Aug 26, 2023
@SebastianStehle SebastianStehle deleted the include-fixes branch August 26, 2023 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants