ocaml / odoc

Documentation compiler for OCaml and Reason
Other
322 stars 89 forks source link

Distinction between `{i text}` and `{e text}` #420

Closed voodoos closed 4 years ago

voodoos commented 4 years ago

I played a bit with italics and emphasis, but I wasn't able to infer the difference between the two. (And the documentation is not very explicit.)

As a LaTex user I was expecting {e text} to behave in a similar way than \emph{text}. emph has the logical meaning "I want to emphasize that" and this does not always translate to the physical task of writing in italics. For example, adding emphasis to a word in the middle of an italic sentence will put the word straight in LaTex.

Expression LaTex-based expectation odoc 1.5.0 behaviour
{i foo} foo foo
{e foo} foo foo
{e bar {e foo} bar} bar foo bar bar foo bar
{i bar {e foo} bar} bar foo bar bar foo bar

That lead to several questions:

Edit: Actually I found #188 that seems to show that nested emphasis should behave as described in this issue, but they are not in my test setup.

voodoos commented 4 years ago

In fact, the html code generated by odoc is the correct one:

<em>blah <em>blih</em> blah</em>

It is my browser (firefox) which does not render it correctly. This is not an odoc issue.

aantron commented 4 years ago

Indeed, the default visual effect of {e ...} should be to toggle italicization of the nested text, with respect to the surrounding text, whereas {i ...} forces the text to be italic no matter the context.

voodoos commented 4 years ago

The standard says:

The level of stress that a particular piece of content has is given by its number of ancestor em elements.

But this is not happening in either Firefox, Chrome or Edge.

Maybe odoc should output the precise <i>bar</i>foo<i>bar</i> html ? It would force a correct render at the cost of loosing the semantical meaning of the <em> tag.

aantron commented 4 years ago

It's probably a CSS bug here:

https://github.com/ocaml/odoc/blob/8245c022e13f102eb4e510514d1e07833bcfc015/src/odoc/etc/odoc.css#L71-L73

we should check and fix that.

voodoos commented 4 years ago

This CSS is effectively enforcing the bug. However it looks like reverting i and em tag to their browser-default behaviour won't change the output.

According to https://www.w3schools.com/cssref/css_default_values.asp the default value for em is font-style: italic;.

We probably cannot solve that problem with css only.

aantron commented 4 years ago

Looks like I was wrong in my belief that the display should be toggled, according to HTML.

I think if we choose to display nested em tags differently, it should be done with CSS, either by providing extra selectors to handle certain levels of nesting, or by having the HTML emitter emit extra classes on em tags. Changing the tag markup would have a negative impact on accessibility.

voodoos commented 4 years ago

The second solution is probably better, it feels less hacky and could handle any level of nesting.

I can try to look at it if you want.

aantron commented 4 years ago

I would be happy to review a PR.

I suggest to add classes only to nested <em> elements, i.e. outer <em> elements would be emitted plain, as now.

voodoos commented 4 years ago

Yes, I was tinkering with the css a little and that is also the path I chose.

My proposal is:

em.odd, i em { font-style: normal; }


With that setup, the following expression:
```html
<em>
  em(
  <em class="odd">
    em2(
    <i>
      i(
      <em>
        em(
        <em class="odd">
          em2(
          <em>
            em3(
            <i>
              i(
              <em>em</em>
              )i
            </i>
            )em3
          </em>
          )em2
        </em>
        )em
      </em>
      )i
    </i>
    )em2
  </em>
  )em
</em>

Is rendered as expected: em( em2( i( em( em2( em3( i( em )i )em3 )em2 )em )i )em2 )em

aantron commented 4 years ago

The first code block mentions em.even, which I think is perhaps based on an earlier, private version of your approach, so I have a hard time evaluating it. However, I suggest you open a PR once you have a working or likely-working approach, and it will be easier to review there.

voodoos commented 4 years ago

The first code block mentions em.even, which I think is perhaps based on an earlier, private version of your approach, so I have a hard time evaluating it. However, I suggest you open a PR once you have a working or likely-working approach, and it will be easier to review there.

Yes I faced the standard initial-value dilemma while numbering the levels :-)