jdrouet/mrml

Default fonts does not seems to behave as a fallback

Closed this issue · 3 comments

I just discover this awesome lib, thanks for the great work!

Redefining in template one of the default -or configured- font seems to duplicate definition instead of replacing it. for instance with Roboto

<mjml>
  <mj-head>
    <mj-font name="Roboto" href="https://fonts.googleapis.com/css?family=Roboto:100,100i,300,300i,400,400i,500,500i,700,700i,900,900i" />
  </mj-head>
  <mj-body>
    <mj-section>
      <mj-column>
        <mj-text font-family="Roboto, Arial">
          Hello World!
        </mj-text>
      </mj-column>
    </mj-section>
  </mj-body>
</mjml>

produces

<link href="https://fonts.googleapis.com/css?family=Roboto:100,100i,300,300i,400,400i,500,500i,700,700i,900,900i" rel="stylesheet" type="text/css">
<style type="text/css">
  @import url(https://fonts.googleapis.com/css?family=Roboto:100,100i,300,300i,400,400i,500,500i,700,700i,900,900i);
</style>

using MJML,
but duplicates the definitions with MRML:

<link href="https://fonts.googleapis.com/css?family=Roboto:300,400,500,700" rel="stylesheet" type="text/css">
<link href="https://fonts.googleapis.com/css?family=Roboto:100,100i,300,300i,400,400i,500,500i,700,700i,900,900i" rel="stylesheet" type="text/css">
<style type="text/css">
  @import url(https://fonts.googleapis.com/css?family=Roboto:300,400,500,700);
  @import url(https://fonts.googleapis.com/css?family=Roboto:100,100i,300,300i,400,400i,500,500i,700,700i,900,900i);
</style>

I am not a rust developer, but a simple fix could look like

diff --git a/packages/mrml-core/src/mj_head/render.rs b/packages/mrml-core/src/mj_head/render.rs
index d377bdc..9b2f617 100644
--- a/packages/mrml-core/src/mj_head/render.rs
+++ b/packages/mrml-core/src/mj_head/render.rs
@@ -145,23 +145,25 @@ impl<'e, 'h> MjHeadRender<'e, 'h> {
         header
             .used_font_families()
             .iter()
-            .filter_map(|name| opts.fonts.get(name))
+            .filter_map(|name| header.font_families().get(name.as_str()))
             .for_each(|href| links.push_str(&self.render_font_link(href)));
         header
             .used_font_families()
             .iter()
-            .filter_map(|name| header.font_families().get(name.as_str()))
+            .filter(|name| !header.font_families().get(name.as_str()).is_some())
+            .filter_map(|name| opts.fonts.get(name))
             .for_each(|href| links.push_str(&self.render_font_link(href)));
         let mut imports = String::default();
         header
             .used_font_families()
             .iter()
-            .filter_map(|name| opts.fonts.get(name))
+            .filter_map(|name| header.font_families().get(name.as_str()))
             .for_each(|href| imports.push_str(&self.render_font_import(href)));
         header
             .used_font_families()
             .iter()
-            .filter_map(|name| header.font_families().get(name.as_str()))
+            .filter(|name| !header.font_families().get(name.as_str()).is_some())
+            .filter_map(|name| opts.fonts.get(name))
             .for_each(|href| imports.push_str(&self.render_font_import(href)));
         if links.is_empty() && imports.is_empty() {
             String::default()

Thanks for this 👍 Will fix it asap

could you tell me how you use it? you might just have to update the default fonts with an empty map

I am planning to use ruby/rails integration but I did use the cli for reproduction by comparing html outputs between mrml and mjml :

cat font.mjml | mrml render > font.mrml.html
cat font.mjml | mjml -is > font.mjml.html
# e.g. npm install -g html-differ && html-differ font.mjml.html font.mrml.html