HTML: Match rendering of polymorphic variant with normal variants #971

panglesd · 2023-06-08T08:17:09Z

Mostly CSS, I just needed to add one class.
Quite cumbersome due to the differences in document on how polymorphic variants and normal variants are turned into a document value. But modifying the CSS seemed still less work (and with less unexpected consequences in other backend) than modifying the document generator to "unify" normal and polymorphic variants.

Signed-off-by: Paul-Elliot <[email protected]>

trefis

Also, silly question sorry but related to Jon's issue: why do we <ol>?

trefis · 2023-06-08T11:44:14Z

src/html_support_files/odoc.css

+.spec.type .poly-variant > code {
+  min-width: 2ch;
+}
+.spec.type .poly-variant > code + code {
+  min-width: calc(40% - 2ch - 0.6ex);
+  /* In polymorphic variants, the | is not included in the first code block, so we
+   need to accound for its size */
+}
+


That seems fragile.
Why don't we just wrap the whole left/1st column and set the min-width on that? (I assume it's for regularity with other type declarations ... but I also don't quite understand why | is handled differently here then).

Yes, this is fragile (and was already).

The reason the html generated in case of polymorphic variant is structured differently from the one for variants, is that it is already the case at the intermediate document representation.

I was afraid of changing the intermediate representation, as that would require me to check that it does not break anything in other backends. But maybe that is a better solution.

dbuenzli · 2023-06-08T12:13:28Z

Also, silly question sorry but related to Jon's issue: why do we <ol>?

Let's add my own silly question: what else would you suggest to use ?

dbuenzli · 2023-06-08T12:15:40Z

src/document/generator.ml

-          let attrs = [ "def"; Url.Anchor.string_of_kind url.kind ] in
+          let attrs =
+            [ "def"; "poly-variant"; Url.Anchor.string_of_kind url.kind ]
+          in


I'm not sure exactly why you had to change the generator to fix this issue (odig's stylesheet have no problem with this without that) but, why not…

Thinking more about this I'm not sure it's really a good idea to add more to the markup to distinguish polymorphic variants from regular ones. I don't really see a good use case for it (except for people making stylesheets more complex than they could be).

I think it is related with odoc doing something more complex, when the size of the comment is small, the comment is in the same line and takes 50% of the space, otherwise it takes more space and is wrapped below (see the screenshot or some file where it happens).
I don't know if that's worth the trouble, but I think that's the difference with odig, which displays comments in a simpler way (from a quick look at the stylesheet, I might be wrong).

There might be a way to do it without, but since variant and fields had their classes, it felt natural to add one for polymorphic variants

dbuenzli · 2023-06-08T12:30:50Z

I think you should add the example with a short comment and a long comment for the case to the ocamlary, it's useful for people developing stylesheets, since that's a case they will have to handle. Also it would be nice to provide a link to the ocamlary module from the .mld docs. I can't see by starting from odig doc odoc but it's clearly there.

trefis · 2023-06-08T12:35:19Z

I think you should add the example with a short comment and a long comment for the case to the ocamlary

👍

Also, silly question sorry but related to Jon's issue: why do we <ol>?

Let's add my own silly question: what else would you suggest to use ?

I would have thougth <ul> ... but I don't know what I'm talking about, so really, don't mind me.

panglesd · 2023-06-08T12:42:36Z

I think you should add the example with a short comment and a long comment for the case to the ocamlary

I'll do that

I would have thougth <ul> ... but I don't know what I'm talking about, so really, don't mind me.

The html generator has to use ol for ordered lists: otherwise, user-input {ol {li bli}} would not give numbers.
So, we would need to change the document generator, from ordered lists to unordered lists, when rendering variant lists.
But that means that all backends would be modified, and I guess that latex uses the fact that its ordered.
(Otherwise, it would be very strange to use ordered lists and then remove the numbers!)

dbuenzli · 2023-06-08T13:48:45Z

I would have thougth
... but I don't know what I'm talking about, so really, don't mind me.

If one believes in semantic markup (no one does :-) then I'd say at least for variants it's more "correct". There is a notion of order between case (but for polymorphic ones it won't match, the li order).

I think it is related with odoc doing something more complex, when the size of the comment is small, the comment is in the same line and takes 50% of the space, otherwise it takes more space and is wrapped below (see the screenshot or some file where it happens).

As far as I see odig works the same way¹.

There might be a way to do it without, but since variant and fields had their classes, it felt natural to add one for polymorphic variants

I think it would be more natural not to distinguish variants and polymorphic variants. These are just list of cases, it's unlikely you want to style them differently. Adding one more class is just adding more selectors to handle and makes your stylesheets more complex, which, trust me, you definitively do not want.

Basically I think odoc just just render polymorphic variants exactly the way variants are. Unless I'm mistaken that just means, in the current state, to add variant to the class of the li generated for them. If your stylesheet already handles variants correctly then nothing else must be changed.

Except not flush right, but I would advise against doing that. If you have many small comments, the large blank make it difficult for the eye to catch the right comment on the right. The same kind of problem you get when you typeset a table of contents with page number flushed right and which people often solve with horrible dot leaders, rather than simply mention the page right after the entry. Obsessing with alignment may makes things look better from a distance, but often makes it less usable for the reader. ↩

panglesd · 2023-07-05T09:30:54Z

Thanks for the advice.
I tried to simplify the css. The fact that, in odoc, the background for text and for code is not the same is making things more complex than in odig. For instance, using odig's way of having the "long comment/short comment" behaviour ends up with white blocks of every dimension; which in my opinion is disturbing:

So, I went with the even simpler approach of having only "long comments":

This was also requested in #847 (comment), so at least it will please one person.

Let me know what you think!

I also added a link to the ocamlary module from the index page.

dbuenzli · 2023-07-05T13:29:05Z

Let me know what you think!

If you need to keep the blocks it's certainly better the way you suggest.

I also added a link to the ocamlary module from the index page.

Thanks!

wikku · 2023-08-03T14:30:02Z

The same problem remains with extensions of extensible variants, so you would need to add more selectors for .extension to the CSS. But by then I think it becomes clear that the CSS rules are wrong, and should be selecting actual lists elements if you want to disable the numbering (#987).

panglesd · 2023-09-12T08:24:34Z

I cherry picked @wikku's commit that makes the generated html for polymorphic variants match the one for normal variants.

I also added the variant class to extension constructor so that the same css is applied to extension constructors and normal constructors.

So, the rendering of normal constructors, extension constructors and polymorphic variants should be exactly the same, and stay the same no matter how we modify the css.

The rest of this PR is:

adding the ocamlary module to the pages of odoc's doc
simplifying the css so that docstrings attached to constructors are always displayed on the next line

- Consider poly-variants as variants - Always go to line to display a doc-comment Signed-off-by: Paul-Elliot <[email protected]>

and add an example of comments associated to polymorphic variants Signed-off-by: Paul-Elliot <[email protected]>

For consistency with variants. In the HTML backend, this merges two <code> elements into one.

Signed-off-by: Paul-Elliot <[email protected]>

@panglesd

CHANGES: ### Added - Add support for external search engines (@panglesd, @EmileTrotignon, ocaml/odoc#972) This includes the generation of an index and the display of the results in the UI (HTML only). - Display 'private' keyword for private type extensions (@gpetiot, ocaml/odoc#1019) - Allow to omit parent type in constructor reference (@panglesd, @EmileTrotignon, ocaml/odoc#933) ### Fixed - Warn and exit when table(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Hint when list(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Fix crash on functors returning an alias (@Julow, ocaml/odoc#1046) - Fix rendering of polymorphic variants (@wikku, @panglesd, ocaml/odoc#971) - Add references to extension declarations (@gpetiot, @panglesd, ocaml/odoc#949) ### Changed - Style: Adjusted line height in the TOC to improve readability (@sorawee, ocaml/odoc#1045) - Style: Remove font fallback to Helvetica, Arial (@Julow, ocaml/odoc#1028) - Style: Preformatted elements fallback to UA monospace (@toastal, ocaml/odoc#967) - Style: Sidebar is now stuck to the left of the content instead of the left of the viewport (@EmileTrotignon, ocaml/odoc#999)

@panglesd

CHANGES: ### Added - Add support for external search engines (@panglesd, @EmileTrotignon, ocaml/odoc#972) This includes the generation of an index and the display of the results in the UI (HTML only). - Display 'private' keyword for private type extensions (@gpetiot, ocaml/odoc#1019) - Allow to omit parent type in constructor reference (@panglesd, @EmileTrotignon, ocaml/odoc#933) ### Fixed - Warn and exit when table(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Hint when list(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Fix crash on functors returning an alias (@Julow, ocaml/odoc#1046) - Fix rendering of polymorphic variants (@wikku, @panglesd, ocaml/odoc#971) - Add references to extension declarations (@gpetiot, @panglesd, ocaml/odoc#949) ### Changed - Style: Adjusted line height in the TOC to improve readability (@sorawee, ocaml/odoc#1045) - Style: Remove font fallback to Helvetica, Arial (@Julow, ocaml/odoc#1028) - Style: Preformatted elements fallback to UA monospace (@toastal, ocaml/odoc#967) - Style: Sidebar is now stuck to the left of the content instead of the left of the viewport (@EmileTrotignon, ocaml/odoc#999)

@panglesd

CHANGES: ### Added - Add support for external search engines (@panglesd, @EmileTrotignon, ocaml/odoc#972) This includes the generation of an index and the display of the results in the UI (HTML only). - Display 'private' keyword for private type extensions (@gpetiot, ocaml/odoc#1019) - Allow to omit parent type in constructor reference (@panglesd, @EmileTrotignon, ocaml/odoc#933) ### Fixed - Warn and exit when table(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Hint when list(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Fix crash on functors returning an alias (@Julow, ocaml/odoc#1046) - Fix rendering of polymorphic variants (@wikku, @panglesd, ocaml/odoc#971) - Add references to extension declarations (@gpetiot, @panglesd, ocaml/odoc#949) ### Changed - Style: Adjusted line height in the TOC to improve readability (@sorawee, ocaml/odoc#1045) - Style: Remove font fallback to Helvetica, Arial (@Julow, ocaml/odoc#1028) - Style: Preformatted elements fallback to UA monospace (@toastal, ocaml/odoc#967) - Style: Sidebar is now stuck to the left of the content instead of the left of the viewport (@EmileTrotignon, ocaml/odoc#999)

@panglesd

CHANGES: ### Added - Add support for external search engines (@panglesd, @EmileTrotignon, ocaml/odoc#972) This includes the generation of an index and the display of the results in the UI (HTML only). - Display 'private' keyword for private type extensions (@gpetiot, ocaml/odoc#1019) - Allow to omit parent type in constructor reference (@panglesd, @EmileTrotignon, ocaml/odoc#933) ### Fixed - Warn and exit when table(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Hint when list(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Fix crash on functors returning an alias (@Julow, ocaml/odoc#1046) - Fix rendering of polymorphic variants (@wikku, @panglesd, ocaml/odoc#971) - Add references to extension declarations (@gpetiot, @panglesd, ocaml/odoc#949) ### Changed - Style: Adjusted line height in the TOC to improve readability (@sorawee, ocaml/odoc#1045) - Style: Remove font fallback to Helvetica, Arial (@Julow, ocaml/odoc#1028) - Style: Preformatted elements fallback to UA monospace (@toastal, ocaml/odoc#967) - Style: Sidebar is now stuck to the left of the content instead of the left of the viewport (@EmileTrotignon, ocaml/odoc#999)

@panglesd

CHANGES: ### Added - Add support for external search engines (@panglesd, @EmileTrotignon, ocaml/odoc#972) This includes the generation of an index and the display of the results in the UI (HTML only). - Display 'private' keyword for private type extensions (@gpetiot, ocaml/odoc#1019) - Allow to omit parent type in constructor reference (@panglesd, @EmileTrotignon, ocaml/odoc#933) ### Fixed - Warn and exit when table(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Hint when list(s) is not closed (@lubegasimon, ocaml/odoc#1050) - Fix crash on functors returning an alias (@Julow, ocaml/odoc#1046) - Fix rendering of polymorphic variants (@wikku, @panglesd, ocaml/odoc#971) - Add references to extension declarations (@gpetiot, @panglesd, ocaml/odoc#949) ### Changed - Style: Adjusted line height in the TOC to improve readability (@sorawee, ocaml/odoc#1045) - Style: Remove font fallback to Helvetica, Arial (@Julow, ocaml/odoc#1028) - Style: Preformatted elements fallback to UA monospace (@toastal, ocaml/odoc#967) - Style: Sidebar is now stuck to the left of the content instead of the left of the viewport (@EmileTrotignon, ocaml/odoc#999)

panglesd force-pushed the ol-to-ul branch from 36f639e to 5d56924 Compare June 8, 2023 08:23

panglesd added a commit to panglesd/odoc that referenced this pull request Jun 8, 2023

Added changelog entry for ocaml#971

a19217a

Signed-off-by: Paul-Elliot <[email protected]>

trefis reviewed Jun 8, 2023

View reviewed changes

dbuenzli reviewed Jun 8, 2023

View reviewed changes

panglesd force-pushed the ol-to-ul branch 3 times, most recently from 17e133c to fa22aea Compare July 5, 2023 09:17

panglesd force-pushed the ol-to-ul branch from fa22aea to 3604b06 Compare July 5, 2023 09:33

wikku mentioned this pull request Aug 3, 2023

Unnumbered lists of constructors for polyvariants and extensible variants #987

Closed

panglesd force-pushed the ol-to-ul branch from 3604b06 to d110239 Compare September 12, 2023 08:10

panglesd force-pushed the ol-to-ul branch 2 times, most recently from 2ac9780 to 701f2e9 Compare September 12, 2023 13:48

Simplify css for docs on variants and poly-variants

ce1df23

- Consider poly-variants as variants - Always go to line to display a doc-comment Signed-off-by: Paul-Elliot <[email protected]>

panglesd force-pushed the ol-to-ul branch from 701f2e9 to 61666b2 Compare October 26, 2023 13:25

panglesd and others added 6 commits October 26, 2023 15:25

Link to Ocamlary from index of pages

c78ffab

and add an example of comments associated to polymorphic variants Signed-off-by: Paul-Elliot <[email protected]>

Use documentedSrc instead of code in polyvariants

e4c5cea

For consistency with variants. In the HTML backend, this merges two <code> elements into one.

Consider extension constructor as constructors in css

1cae830

css: use list-specific selector for list-specific properties

a4dff22

Signed-off-by: Paul-Elliot <[email protected]>

compatibility with 4.02

61666b2

Signed-off-by: Paul-Elliot <[email protected]>

Add changelog entry for ocaml#971

1f24c7c

Signed-off-by: Paul-Elliot <[email protected]>

panglesd merged commit 8737ad8 into ocaml:master Oct 26, 2023
3 of 4 checks passed

Julow mentioned this pull request Dec 11, 2023

[new release] odoc and odoc-parser (2.4.0) ocaml/opam-repository#24927

Closed

Julow mentioned this pull request Dec 12, 2023

[new release] odoc and odoc-parser (2.4.0) ocaml/opam-repository#24930

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HTML: Match rendering of polymorphic variant with normal variants #971

HTML: Match rendering of polymorphic variant with normal variants #971

panglesd commented Jun 8, 2023

trefis left a comment

trefis Jun 8, 2023

panglesd Jun 8, 2023

dbuenzli commented Jun 8, 2023

dbuenzli Jun 8, 2023

dbuenzli Jun 8, 2023 •

edited

Loading

panglesd Jun 8, 2023

dbuenzli commented Jun 8, 2023 •

edited

Loading

trefis commented Jun 8, 2023

panglesd commented Jun 8, 2023

dbuenzli commented Jun 8, 2023

panglesd commented Jul 5, 2023

dbuenzli commented Jul 5, 2023

wikku commented Aug 3, 2023

panglesd commented Sep 12, 2023

HTML: Match rendering of polymorphic variant with normal variants #971

HTML: Match rendering of polymorphic variant with normal variants #971

Conversation

panglesd commented Jun 8, 2023

trefis left a comment

Choose a reason for hiding this comment

trefis Jun 8, 2023

Choose a reason for hiding this comment

panglesd Jun 8, 2023

Choose a reason for hiding this comment

dbuenzli commented Jun 8, 2023

dbuenzli Jun 8, 2023

Choose a reason for hiding this comment

dbuenzli Jun 8, 2023 • edited Loading

Choose a reason for hiding this comment

panglesd Jun 8, 2023

Choose a reason for hiding this comment

dbuenzli commented Jun 8, 2023 • edited Loading

trefis commented Jun 8, 2023

panglesd commented Jun 8, 2023

dbuenzli commented Jun 8, 2023

Footnotes

panglesd commented Jul 5, 2023

dbuenzli commented Jul 5, 2023

wikku commented Aug 3, 2023

panglesd commented Sep 12, 2023

dbuenzli Jun 8, 2023 •

edited

Loading

dbuenzli commented Jun 8, 2023 •

edited

Loading