Provide markup for the mail display mockup

First, early try. Added css, divided for each view mode.
Teaser:

Full:

Thx for helping to move forward.

We need much less boldness in the teaser.
To make a difference between the subject and the body, you can grey out the body text a bit. Google also does it this way. And i think we can even make it a bit smaller.

Might be my installation but when changing the font weight from 0 to 500 the change is none.

1. +++ b/templates/inmail-message.html.twig
   @@ -19,53 +19,60 @@
   +                >
   

   This bracket needs encoding!

2. +++ b/templates/inmail-message.html.twig
   @@ -19,53 +19,60 @@
   -                <div class="inmail-message--header-label">From</div>
   +                <div class="inmail-message--header-label">From:</div>
   

   Not sure we really need the colons? With labels we usually try to avoid colons when possible.

3. +++ b/templates/inmail-message.html.twig
   @@ -73,6 +80,7 @@
   +                <div class="inmail-message--header-label">Attachments:</div>
   

   This is footer, not header.

And yeah, you still display them bold. Why not simply regular?

Please consider my proposal to always print the semantic labels and hide them in CSS.

Why is the attachment label and the "all header" different indicated?

Can we quickly make this "All header" thing collapsed, reduced? It steals so much attention. We could also simply hide it in CSS and make it visible (CSS click handler) in a follow-up again.
I'm highly confused that the mail body is also present there. It should be headers only.
And the header should display preformatted (pre tags). A real line should stay a line.

Now should be better. Also hidden the all headers by default. :)

Sorry, fixed tests and fixed indentation. :)

1. --- /dev/null
   +++ b/css/message-teaser.css
   
   @@ -0,0 +1,51 @@
   diff --git a/css/message.css b/css/message.css
   

   I don't know of any other component that offers a CSS file per view mode. This contains many risks, see below.

2. +++ b/css/message-teaser.css
   @@ -0,0 +1,51 @@
   +.inmail-message--body,
   ...
   +  display: flex;
   
   +++ b/css/message.css
   @@ -2,30 +2,25 @@
   +.inmail-message--body,
   ...
   +  margin: 10px;
   

   Super dangerous and bogus. The same class receives different definitions depending if it is a teaser display or a full one.
   That's why these things should stay in one file per component. And that's why they need to be fully qualified.
   If some definition in CSS only applies the teaser, you need to prefix it with that selector too.

Sorry, true. Now all the css is in just one file. Added prefix for teaser elements.

This comment is originally writen for as a feedback for #12, sorry i don't have time to preview patch in #14, but i guess still a lot of stuff applies for new patch also.

There is lots of stuff here that can/should be improved for frontend. Here are couple of things for now.
Do note that none of this code is tested - and there are probably errros ;)
Also sorry for a bigger comment, i've tried to explain more bigger picture and not just specific stuff related to this ;)

BEM syntax

D8 themes are using BEM syntax for naming convention.

For example instead of

  <section class="inmail-message inmail--view-mode-teaser">
    <header>
      <div class="inmail-message--header">
        <div class="inmail-message--header-to"></div>
        <div class="inmail-message--header-cc"></div>
      </div>
    </header>
    <div class="inmail-message--body">
      <div class="inmail-message--body-subject"></div>
      <div class="inmail-message--body-plain"></div>
    </div>
  </section>

If we apply correct BEM conventions we could have something like

  <section class="inmail-message inmail-message--teaser">
    <header>
      <div class="inmail-message__header">
        <div class="inmail-message__header__to"></div>
        <div class="inmail-message__header__cc"></div>
      </div>
    </header>
    <div class="inmail-message__body">
      <div class="inmail-message__body__subject"></div>
      <div class="inmail-message__body__plain"></div>
    </div>
  </section>

Note the modifier variation for original inmail--view-mode-teaser. I've used inmail-message--teaser, but its also perfectly valid to use inmail-message--view-mode-teaser or inmail-message-view-mode--teaser. It's more a personal preference, generally i like to describe the class name with the less characters possible and in this case just inmail-message--teaser is probably enough.

Code re-usability

Template level

All frontend stuff starts from template and it's important to try as much as possible to write clean and well structured HTML because this will affect later CSS rules and JS code.

First thing, i know that its very tempting and easy to use base level if command and completly separate different view modes, in this case teaser and full view modes. However the problem is that with this approach developer is less enforced to write reusable code because its easy to just write completely different stuff for each view mode. So i would always try to start with only one template file and one structure that covers all needed cases.

Later if template code starts to be more complex and you are starting to get too many sub-component level if commands then it make sense to separate stuff but still try to reuse code - for example have base template inmail-message.html.twig and then use twig blocks and extends for inmail-message--teaser.html.twig. Check http://twig.sensiolabs.org/doc/tags/extends.html and https://ffwagency.com/blog/power-of-extending-twig-templates about this.

Here maybe something like this could work (mix of both template extend and if statements just for example - for sure you could define block body_content also and override it in teaser:

inmail-message.html.twig

{{ attach_library('inmail/message') }}
<section class="inmail-message inmail-message--{{ view_mode }}">
  <header>
    <div class="inmail-message__header">
      {% block header_content %}
        <div class="inmail-message__element inmail-message__header__from">
          <label>From</label>
          {{ message.from }}
        </div>
        {% if message.getTo is not empty %}
        <div class="inmail-message__element inmail-message__header__to">
          <label>To</label>
          {{ message.getTo | join(', ') }}
        </div>
        {% endif %}
        {% if message.getCc is not empty %}
        <div class="inmail-message__element inmail-message__header__cc">
          <label>CC</label>
          {{ message.getCc | join(', ') }}
        </div>
        {% endif %}
        <div class="inmail-message__element inmail-message__header__received-date">
          <label>Received</label>
          {{ message.getReceivedDate() }}
        </div>
        <div class="inmail-message__element inmail-message__header__subject">
          <label>Subject</label>
          {{ message.subject }}
        </div>
        <div class="inmail-message__element inmail-message__header__all">
          <label>All Headers</label>
          {{ message.header.toString() }}
        </div>
      {% endblock %}
    </div>
  </header>
  <div class="inmail-message__body">
    {# @note - if message.subject exist only for teaser then use that for if instead of view_mode#}
    {% if view_mode == 'teaser' %}
      <div class="inmail-message__element inmail-message__body__subject">{{ message.subject }}</div>
    {% endif %}
    <div class="inmail-message__element inmail-message--body__content">
      {% if view_mode == 'teaser' %}
        {{ message.getPlainText() | slice(0, 1000) }}
      {% else %}
        {{ message.getDecodedBody() }}
      {% endif %}
    </div>
    {% if attachments %}
    <footer>
      <div class="inmail-message__footer">
        <label>Attachments</label>
        <ul>
          {% for attachment in attachments %}
          <li>
            {% if attachment.url %}
            <a href="{{ attachment.url }}" title="{{ attachment.filename }}"
               target="_blank">{{ attachment.filename }}</a> ({{ attachment.filesize }})
            {% else %}
            {{ attachment.filename }} ({{ attachment.filesize }})
            {% endif %}
          </li>
          {% endfor %}
        </ul>
      </div>
    </footer>
    {% endif %}
  </div>

</section>

inmail-message--teaser.html.twig

{% extends "inmail-message.html.twig" %}
{% block header_content %}
  <div class="inmail-message__element inmail-message__header__from">{{ message.from }}</div>
    {% if message.getTo is not empty %}
    <div class="inmail-message__element inmail-message__header__to">{{ message.getTo | join(', ') }}</div>
  {% endif %}
  {% if message.getCc is not empty %}
    <div class="inmail-message__element inmail-message__header__cc">{{ message.getCc | join(', ') }}</div>
  {% endif %}
{% endblock %}

Couple of notes

- For sure for this to work we need additional template suggestions based on view mode.

- Converted 'inmail-message--header-label' div to just label tag.

- Instead of inmail-message__body-plain use inmail-message__body__content - you want to do variations in CSS with selector when possible and not pollute HTML markup with various CSS classes which are used in just some specific cases.

- For separator like > and | consider using CSS before/after pseudo elements for this because this are more visual separators.
This is just suggestion and CSS rules for this needs tweaking.

- Consider using label tag for labels without any CSS class.

CSS level

With template in place we can now write more efficent CSS in only one file

/**
* @file
* Stylesheet for inmail.
*/

/* General rules */

.inmail-message {
  margin: 10px;
}

.inline-message label {
  font-weight: 700;
  display: inline-block;
  width: 100px;
}

/* @todo - check this CSS selector because it seems it's not used. */
.inmail-message__header header {
  display: -webkit-flex;
  display: flex;
  flex-wrap: wrap;
}

/* Full view */

/* @note - if there are additional header child tags here instead of header
and footer selector here we could use more specific selectors or use
css classes for them also (inmail-message__header and inmail-message__footer). */
.inmail-message--full header,
.inmail-message--full .inmail-message__body,
.inmail-message--full footer {
  margin: 10px;
}

.inmail-message--full .inmail-message__header__all {
  display: none;
}

/* Teaser view */

.inmail-message--teaser header,
.inmail-message--teaser .inmail-message__body {
  display: flex;
  display: -webkit-flex;
}

.inmail-message--teaser .inmail-message__element {
  overflow: hidden;
  text-overflow: ellipsis;
  white-space: nowrap;
}

.inmail-message--teaser .inmail-message__header__from {
  margin: 0 1em 0 0;
}
.inmail-message--teaser .inmail-message__header__from:after {
/* @todo - this rule probably need more tweakings to look good. */
  content: '>';
}

.inmail-message--teaser .inmail-message__header__to {
  margin: 0 1em 0 1em;
}

.inmail-message--teaser .inmail-message__header__cc {
display: inline-block;
  flex-basis: auto;
  margin-left: 1em;
  color: #777;
}
.inmail-message--teaser .inmail-message__header__cc:before {
  /* @todo - this rule probably need more tweakings to look good. */
  content: '|';
}

.inmail-message--teaser .inmail-message__header__plain {
  display: inline-block;
  flex-basis: 50%;
  margin-left: 1em;
  color: #777;
  font-size: 88%;
}

.inmail-message--teaser .inmail-message__body__subject {
  flex-basis: auto;
  margin-right: 1em;
}

Other notes

1.

.inmail-message--header header {
  display: -webkit-flex;
  display: flex;
  flex-wrap: wrap;
}

Checking html and i dont see that this CSS selector is used anywhere?

If we are covering webkit with prefixes then we shoudd do also prefixes then check should we use

-webkit-flex-wrap: wrap; /* Safari 6.1+ */

2.

Code intend for everything in Drupal, including twig is 2 spaces. Currently twig template is using 4 spaces.

Thanks for the feedback, will upload a new patch soon.

> Thanks for the feedback, will upload a new patch soon.

Cool, just be sure to check everything, none of the code that is written is tested so for sure there are bugs.

:-)

Interdiff too big. I wasn't able to make the extends work, not really sure why. Also, I was wondering if it might be better (if extends works) to create a "base" twig that defines the main areas of the mail (header/body/footer) and then override blocks on different twig templates to add the content that is needed. We would still use the selectors provided by @pivica and thus the code would be even more readable. This is an early patch with the new changes. Full view mode works nice but still thinking that creating a "base", "teaser", "full" twig templates is IMHO better.

To make extends / suggestions work, checkout:
block_theme_suggestions_block()
With twig files like:
core/themes/stable/templates/block/block--system-branding-block.html.twig
core/themes/stable/templates/block/block.html.twig

Please keep refactoring limited in a way that this issue can be completed quickly. Otherwise it blocks all other issues as it creates more and more overlap.

Added everything in a twig, changed selectors, added css, addressed @todo in twig file. :)

Removing the @todo change. That better to do it in another issue. :)

1. +++ b/templates/inmail-message.html.twig
   @@ -22,69 +22,81 @@
   +  <section class="inmail-message inmail-message--teaser">
   ...
   +          <div class="inmail-message__element inmail-message__header__to">{{ message.getTo | join(', ') }}</div>
   ...
   +<section class="inmail-message inmail-message--full">
   ...
   +        <div class="inmail-message__element inmail-message__header__to">
   +          <label>To</label>
   +          {{ message.getTo | join(', ') }}
   +        </div>
   

   Teaser and full HTML could share the same markup and use CSS to hide labels and other parts.
   We now have a 100% split inside a single file, that's still full duplication.
   As ivica pointed out, the arrow and other indications could be easily done with CSS ::before and ::after and that would improve semantics.
   Keep in mind that with making each address an element, each of them will have another div/span container. Keeping the html artificially flat in the heaser doesn't help.
   We can move some improvements on that in a follow-up though. Let's complete this fast as it overlaps with lots of issues improving header and body part.

2. +++ b/templates/inmail-message.html.twig
   @@ -22,69 +22,81 @@
   +  </header>
   +  <div class="inmail-message__body">
   ...
   +    {% if attachments %}
   +      <footer>
   +        <div class="inmail-message__footer">
   

   I think the footer should be on same level as header and the body should be a separate container from the footer area.

Ohhhh true, this will be better then, the display is the same as the last screenshot. :)

1. +++ b/templates/inmail-message.html.twig
   @@ -21,70 +21,68 @@
   +  <header>
   +    <div class="inmail-message__header">
   

   Why not add the class to the header element?

2. +++ b/templates/inmail-message.html.twig
   @@ -21,70 +21,68 @@
   +  {% if attachments %}
   

   This should definitively only be output in full view mode.

3. +++ b/templates/inmail-message.html.twig
   @@ -21,70 +21,68 @@
   +    <footer>
   +      <div class="inmail-message__footer">
   

   Why not add the class to the footer element?

Made small changes as suggested in comment #26

You still need to fix the indentation.

+++ b/css/message.css
@@ -1,31 +1,88 @@
+.inmail-message--teaser .inmail-message__footer {
+  display: none;
+}

And then you can remove this.

Removed block of code as suggested in comment #28.

Sorry, this have been taking too much time, this should be OK. btw Thanks @ModernMantra for the patch :)

1. +++ b/css/message.css
   @@ -1,31 +1,85 @@
   +.inmail-message--full .inmail-message__header__to,
   +.inmail-message--full .inmail-message__header__cc,
   +.inmail-message--full .inmail-message__header__received-date,
   +.inmail-message--full .inmail-message__header__subject {
   +  display: flex;
   +}
   

   Instead of all this can we just write next stuff?

   .inmail-message--full .inmail-message__header .inmail-message__element {
     display: flex;
   }
   

2. +++ b/css/message.css
   @@ -1,31 +1,85 @@
   +  margin: 0 0 0.1em;
   

   Just checking can we maybe just use margin-bottom: 0.1em if there is no need to override other values?

3. +++ b/css/message.css
   @@ -1,31 +1,85 @@
   +  margin: 10px 0;
   

   Similar to previous one check can we just do

   margin-top: 10px;
   margin-bottom: 10px;
   

   Usually this pattern is used because it is easier to override this kind of CSS rules then the one that use all values and shorter syntax. But if wee need to override left and right and set it to 0 then it is fine.

Thank you so much, yes, makes so much sense. Addressing changes on comment above. :)

Now this reads nice.

Committed, i guess lots of issues to catch up with the markup.