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

Style the html manual, changing type and layout #1757

Merged
merged 6 commits into from May 5, 2018

Conversation

Projects
None yet
@charlesetc
Copy link
Contributor

charlesetc commented May 3, 2018

Hi!

So this commit makes major visual changes to the OCaml manual. I'm very happy to take any feedback and make changes.

I've got a preview up of these changes. This is compared to the current version.

The header font is Fira Sans. I checked it into this repository, so the font would be served from OCaml's servers. There is an argument to be made for using Google Fonts instead, since it can make things faster through caching. I'd be interested in hearing what people think about that.

The only "hack" here is the following snippet:

\newstyle{body >
          div:first-child >
          span:first-child >
          span:first-child}{
    font-family: "Fira Sans", sans-serif;
}

Which selects the "The OCaml System Release 4.08 ..." heading to make it a different font. Unfortunately there's no id or class by which to select this β€” if anyone knows how to add one that'd be awesome!

In reference to the recent change (49abafc) of the manual, this keeps the max-width for the body, but adds a margin: auto so that the body is centered. I think this looks slightly better and also more like the current manual. On small screens, when max-width doesn't apply, this also adds a percentage padding. You can go to preview_docs.narwhal.space on a phone to see how it looks.

Anyways, I'm glad to make any changes people see fit 🌡

Style the html manual, changing type and layout
Add a new font for the headers, change the link colors,
re-center it, and improve support for mobile viewing.
@xavierleroy

This comment has been minimized.

Copy link
Contributor

xavierleroy commented May 3, 2018

I'm glad to see efforts to improve the look and legibility of the online manual. However something is very wrong with the OCaml code snippets, e.g. the first example of section 1.1, 1+2*3. The original rendering has colors and a # prompt to distinguish input from output. The new style has none of that and is unreadable.

@charlesetc

This comment has been minimized.

Copy link
Contributor Author

charlesetc commented May 3, 2018

You're right! I'll see what I can do about that.

@charlesetc

This comment has been minimized.

Copy link
Contributor Author

charlesetc commented May 3, 2018

It's fixed now! I had the preview showing outdated code

\end{rawhtml}
}
\fi

\input{macros.tex}

This comment has been minimized.

@Octachron

Octachron May 3, 2018

Contributor

This \input{macros.tex} include should be before the redefinition of the \@meta (since it is the one that defines \ifouthtml).

This comment has been minimized.

@charlesetc

charlesetc May 3, 2018

Author Contributor

Thanks! It should be fixed now

@Drup

This comment has been minimized.

Copy link
Contributor

Drup commented May 3, 2018

I really like it, but as someone with terrible eyesight, I would prefer if the background was not pure white. While it looks good and modern, pure white background is very tiring for the eyes. I also tend to like when code blocks have specific background.
For example, RWO uses a slightly beige background and darker code blocks, which is far less aggressive.

Similarly, you might want to tone down the electric blue a bit.

Misc remarks:

The jump to the library documentation is very jarring now. I suppose that is your next step?

@bluddy

This comment has been minimized.

Copy link

bluddy commented May 3, 2018

That's some good css hacking. Adding classes isn't possible without modifying hevea, which isn't on github. I'm really impressed by what you've been able to accomplish just using this method.

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented May 4, 2018

I think that using Google's free Fonts CDN for Fira Sans will be better than serving it off of our own servers β€” it will load faster.

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented May 4, 2018

Unfortunately there's no id or class by which to select this β€” if anyone knows how to add one that'd be awesome!

So I've also been working on doing something similar, and Lambda Soup can be used to postprocess the manual quite nicely for this purpose. One of the changes I've made in my own hacking has been to replace the arrows at the tops and bottoms of the pages to include SVG arrows in their place. You can also nicely alter the CSS classes in the way you wish. Code to do this is quite compact and easy.

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented May 4, 2018

Oh, and in my own experiments I've used Fira Sans for the headers just as you did, and EB Garamond for the body text. EB Garamond is one of the nicest serif fonts available on Google Fonts. Try it, it's quite nice.

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented May 4, 2018

On the background, I find the beige a little too tinted. Perhaps F7F7F7 instead?

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented May 4, 2018

The small yellow arrows certainly have a vintage charm, but do not really correspond to the style anymore.

So I have two options to suggest there, both easily added by the use of Lambda Soup to edit the HTML.

  1. Just say Back, Up, Forward.
  2. Use Google's "Material Design" SVG arrow icons, which are very nice and scale perfectly to every resolution since they're vector art:

https://github.com/google/material-design-icons/blob/master/navigation/svg/design/ic_arrow_back_24px.svg

https://github.com/google/material-design-icons/blob/master/navigation/svg/design/ic_arrow_upward_24px.svg

https://github.com/google/material-design-icons/blob/master/navigation/svg/design/ic_arrow_forward_24px.svg

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented May 4, 2018

On style.css for stdlib, I was working on this yesterday and struggling to get everything quite right because the CSS classes are so eccentric, but it should be possible to get something quite nice and consistent with the rest of the manual by using a touch of Lambda Soup to postprocess and change the class structure.

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented May 4, 2018

BTW, @aantron was helping me with Lambda Soup assistance, so I'm tagging him here in case there are Lambda Soup questions.

@charlesetc

This comment has been minimized.

Copy link
Contributor Author

charlesetc commented May 4, 2018

Haha thanks @bluddy! @Drup thanks for the great suggestions! I've added a light grey background (@pmetzger F7F7F7, thanks!) to the body, and a beige background + light outline to code blocks. This code block change is another big visual change, so we should be sure about it. I also changed that light blue on visited links to a darker blue which should be better. I'm not sure what's wrong with the links to modules like Int32 β€” it looks to my like it's the same in the current version. Am I missing something?

Unfortunately there's already a tiny bit of definition overflow in the current manual. Bumping up the font size, and limiting the body adds to that. It's also arranged with tables so I haven't yet found a way to get them to wrap around or scroll... but I'll keep trying.

Also good point about the standard library formatting! I've just added styling that does it. See this Str module.

I'm actually kind of fond of the arrows, but they are obviously dated. I don't know anything about Lambda Soup but it does seem like it would take some kind of post-processing to do it. @pmetzger everything you're saying about lambda soup sounds awesome and is definitely what this needs!

I just tried EB Garamond and it's definitely a nice font but looks a little strange here so I left it out for now eb

@charlesetc

This comment has been minimized.

Copy link
Contributor Author

charlesetc commented May 4, 2018

Oh also I switched to the Google CDN :)

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented May 4, 2018

The reason it looked strange in the libref stuff is because of the horrid class problems in the generated ocamldoc. Try using EB Garamond only in the hevea generated manual for now and you'll be happier. We'll need Lambda Soup to clean up the ocamldoc classes in order to do real CSS font styling there.

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented May 4, 2018

So Lambda Soup may be found at: https://github.com/aantron/lambda-soup

You will note that the documentation for Lambda Soup is itself postprocessed with a Lambda Soup program that is given in the github repository.

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented May 4, 2018

(I'll be back tomorrow. Big kudos on doing this work!)

@charlesetc

This comment has been minimized.

Copy link
Contributor Author

charlesetc commented May 4, 2018

Wow Lambda soup looks awesome and it's ocamldoc site is beautiful!

Thanks πŸ˜„ I'll also be back tomorrow to:

  • Try to fix the overflowing BNF's
  • Add EB Garamond (having trouble atm)
@gasche

This comment has been minimized.

Copy link
Member

gasche commented May 4, 2018

For the record, I'm not completely excited at the idea of post-processing generated documentation. Using Lambda Soup is fine for experiments, but if we decide that we like a certain style, why not change the code generation instead to get it from the scratch? Post-processing would make the manual/documentation build harder to maintain.

@Octachron

This comment has been minimized.

Copy link
Contributor

Octachron commented May 4, 2018

@charlesetc :

Which selects the "The OCaml System Release 4.08 ..." heading to make it a different font. Unfortunately there's no id or class by which to select this β€” if anyone knows how to add one that'd be awesome!

Sorry, I missed this point on first reading. One could fix this problem by making the latex code more semantic and replacing the \begin{center}...\end{center} used in the main title by a new maintitle environment and then adding the right definition to macros.tex and macros.hva: example

@Drup

This comment has been minimized.

Copy link
Contributor

Drup commented May 4, 2018

@charlesetc Thanks, this is much nicer!

I agree with @gasche, in particular regarding the API documentation. We can tweak the output of ocamldoc whichever way we want, might as well avoid doing postprocessing. Also, improvements in ocamldoc styling can easily benefit odoc as well.

Notably, Having a side panel with an index for the manual would be really cool.

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented May 4, 2018

For the record, I'm not completely excited at the idea of post-processing generated documentation. Using Lambda Soup is fine for experiments, but if we decide that we like a certain style, why not change the code generation instead to get it from the scratch?

That requires a lot more learning to do. Fixing the HTML is easy. Fixing ocamldoc requires learning how the inside of ocamldoc works and that's much less obvious. I would recommend we not postpone fixing up the appearance until that work gets done. Instead, I would suggest we do this in two steps:

  1. We do some postprocessing now, just to figure out what we want and get things looking nice for the short term. That gets committed for now, because it will make the documentation more readable for people now. The nice thing about code is it isn't stone, we can get rid of it at will in a month or six months.
  2. A volunteer who understands ocamldoc and hevea fixes them up to generate better things, and then the postprocessor is thrown out, but they don't have to rush to do this because we have the postprocessor as a crutch for the moment.
@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented May 4, 2018

Notably, Having a side panel with an index for the manual would be really cool.

We can do that pretty quickly with Lambda Soup, and we can fix it "correctly" over time. @aantron has an example in his own documentation for Lambda Soup.

@Chris00

This comment has been minimized.

Copy link
Member

Chris00 commented May 4, 2018

We do some postprocessing now, just to figure out what we want and get things looking nice for the short term.

For the library documentation (as opposed to the manual itself), if there are hesitations to merge post-processing in the repo, an opam package with a small tool would be nice (and useful for other people as well).

@gasche

This comment has been minimized.

Copy link
Member

gasche commented May 4, 2018

I would suggest we do this in two steps:

  1. We do some postprocessing now, just to figure out what we want and get things looking nice for the short term. That gets committed for now, because it will make the documentation more readable for people now. The nice thing about code is it isn't stone, we can get rid of it at will in a month or six months.
  2. A volunteer who understands ocamldoc and hevea fixes them up to generate better things, and then the postprocessor is thrown out, but they don't have to rush to do this because we have the postprocessor as a crutch for the moment.

What will predictably happen in that scenario is that no one will ever do step (2), and we will end up with a build system that is even more baroque than it is today. We will still be there maintaining the manual build ten years from now, and I think it really is in our collective interest for me to look a bit more annoying right now, and try to get someone (not necessarily the PR author) motivated enough to make the rendering fixable at the source now and not in six renewable months.

@dra27

This comment has been minimized.

Copy link
Contributor

dra27 commented May 5, 2018

A gentle reminder not to ignore minorities - the 1% will include people like me who have to scale things because our eyesight is rather below average.

That said, you all seem to have agreed that absolute sizes are not great...

@dbuenzli

This comment has been minimized.

Copy link
Contributor

dbuenzli commented May 5, 2018

BTW, just to note again: 99% of users never adjust font sizes on their displays, and are unaware that they can even do it.

Maybe not you but people with accessiblility needs do however.

That said: it is absolutely a mistake to use the browser default for paragraph text sizing.

I don't exactly know where you got that from. It seems that you are trying to project your own tastes as facts. The browser default is absolutely reasonable and works for millions of people on thousands of websites.

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented May 5, 2018

@dra27 said:

A gentle reminder not to ignore minorities - the 1% will include people like me who have to scale things because our eyesight is rather below average.

As is mine. (I've got bad retina problems in my right eye, and issues with focus in the left. It doesn't prevent me from driving but reading can be a chore.)

I'm not against allowing people to scale their fonts. rem etc. seem fine. I'm against picking small ones by default.

@dbuenzli said:

Maybe not you but people with accessiblility needs do however.

I'm someone with visual problems, so yes, I'm a person with accessibility needs. I'm in favor of using CSS unit measurements like rem that are based on scaling off the basic browser font values, but I'm against picking small by default. Do you now understand my position?

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented May 5, 2018

BTW, just to address another one of @dbuenzli's points:

The browser default is absolutely reasonable and works for millions of people on thousands of websites.

I just looked at a half dozen newspaper and documentation web sites, to confirm what I thought was true, and all specified font sizing (though many with rem and % rather than px).

The default font selection and font size in browsers is a historical accident, not something that should be relied on to give good results.

BTW, you were speaking of people with accessibility needs β€” those of us with poor vision are people who don't appreciate small default font sizes, as you might guess, and the defaults are generally too small.

@gasche

This comment has been minimized.

Copy link
Member

gasche commented May 5, 2018

Would someone be kind enough to propose a new PR to use rem consistently instead of px settings for font sizes? We could still have a discussion about the font-size: X%; choice for the root element, but unfortunately I have absolutely no idea of how to resolve your fundamental disagreement? (Would the people that cared enough about the topic to comment on this PR thus far just vote? Do we have a Benevolent Style Dictator?)

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented May 5, 2018

Would someone be kind enough to propose a new PR to use rem consistently instead of px settings for font sizes?

If @charlesetc is willing I think that would be best, but if not I'll happily do that.

I'd suggest we think of the style as a work in progress. It will require a number of adjustments to make sure we account reasonably for different viewing devices, to adjust problems we find in particular pages, etc., and doing this over a series of Pull Requests will work best.

Meanwhile, @charlesetc did us all a service by showing us a much nicer looking page that seems like a good basis for future work and I greatly thank him for doing what he's done so far. Bravo.

@charlesetc

This comment has been minimized.

Copy link
Contributor Author

charlesetc commented May 6, 2018

I'm also in favor of some sort of relative font sizing β€” seems everyone is. I'm not sure how rem's work so @pmetzger do you think you could do the PR? Let me know if I can help!

Also thanks to all who helped review!

@charlesetc charlesetc deleted the charlesetc:manual-formatting branch May 6, 2018

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented May 6, 2018

@charlesetc So a rem is just like an em, except it remains constant throughout the document, while em changes in value to whatever the most recent font-size declaration stated. For most users, a rem is equivalent to 16 points throughout (unless you set a new root font-size in the style), while an em might shift up and down.

This explains it pretty well: https://zellwk.com/blog/rem-vs-em/

Want to give it a try?

@charlesetc

This comment has been minimized.

Copy link
Contributor Author

charlesetc commented May 7, 2018

Thanks! Seems pretty simple and I put up a PR ^^

@jasim

This comment has been minimized.

Copy link

jasim commented May 7, 2018

@charlesetc This is very good stuff. Thanks a ton!

@charlesetc

This comment has been minimized.

Copy link
Contributor Author

charlesetc commented Jul 30, 2018

So things look pretty good! (Also thanks @jasim :))

I noticed that the fonts are not being hosted. I would expect them to be at https://caml.inria.fr/pub/docs/manual-ocaml/fonts and that's indeed where it seems the css is looking. They still render if the computer has Fira cached or installed, but otherwise not. Maybe someone who knows the details of how the manual is hosted might be able to help?

@gasche

This comment has been minimized.

Copy link
Member

gasche commented Jul 30, 2018

I guess that the release target in manual/manual/Makefile needs to be fixed to also copy over fonts files to be included in the manual tarball.

@xavierleroy

This comment has been minimized.

Copy link
Contributor

xavierleroy commented Jul 30, 2018

Why host a copy of the fonts on caml.inria.fr or embed them in the manual distribution when we can fetch the fonts straight from Google? fonts.google.com recommends:

To embed your selected fonts into a webpage, copy this code into the of your HTML document.

<link href="https://fonts.googleapis.com/css?family=Fira+Sans" rel="stylesheet">

Use the following CSS rules to specify these families:

    font-family: 'Fira Sans', sans-serif;
@gasche

This comment has been minimized.

Copy link
Member

gasche commented Jul 30, 2018

@xavierleroy there are pros and cons, the main cons being that some people do not appreciate having to query a Google-owned webpage for non-Google-related errands, as they want to avoid Google web-tracking. There has been a lengthy debate on the Discuss thread related to the manual-change PR and the consensus was that hosting the fonts to make the manual self-contained is a reasonable compromise. (As far as I understand, there are no licensing issues with distributing the files ourselves.)

@xavierleroy

This comment has been minimized.

Copy link
Contributor

xavierleroy commented Jul 30, 2018

I must have missed that debate. Some other people may not agree to serve fonts from caml.inria.fr either, you know. That's not our job.

At any rate, the semi-absolute URLs /pub/docs/manual-ocaml/fonts/fira-sans-v8-latin-regular.* currently hard-coded in style.css are pessimal. On the one hand, they force the fonts to be embedded in the tarball for the HTML manual distribution, because http://caml.inria.fr/pub/docs/manual-ocaml is populated by untarring this distribution. On the other hand, since they are not relative URLs, they won't work for a local installation of the manual or an installation on a different Web server.

@alainfrisch

This comment has been minimized.

Copy link
Contributor

alainfrisch commented Jul 30, 2018

Re web-related tracking, @gasche perhaps referred to GDPR compliance issues ( google/fonts#1495 ).

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented Jul 30, 2018

At any rate, the semi-absolute URLs /pub/docs/manual-ocaml/fonts/fira-sans-v8-latin-regular.* currently hard-coded in style.css are pessimal. On the one hand, they force the fonts to be embedded in the tarball for the HTML manual distribution, because http://caml.inria.fr/pub/docs/manual-ocaml is populated by untarring this distribution. On the other hand, since they are not relative URLs, they won't work for a local installation of the manual or an installation on a different Web server.

Yah, the fonts should be handled as relative paths etc.

@gasche

This comment has been minimized.

Copy link
Member

gasche commented Jul 30, 2018

@pmetzger or @charlesetc: would one of you be kind enough to send a PR that uses relative paths in the right place, and fixes the manual/manual/Makefile release target so that the tarball contains the fonts in the right place? I can refresh the 4.07 documentation with a newly-created tarball from it.

@xavierleroy: Personally I don't see this as an issue, we serve CSS style sheets, HTML pages, and gif/svg images of navigation arrows, so why not (open source) font files. If you strongly oppose serving font files, personally I would suggest rolling back on the use of alternative fonts altogether. There is little point in binding the manual to content that only third-parties can serve.

@xavierleroy

This comment has been minimized.

Copy link
Contributor

xavierleroy commented Jul 30, 2018

Font files can be large (I haven't measured how large) and are copyrighted. For those two reasons I'm reluctant to put them in the distribution tarball for the HTML manual.

Serving a copy of the font files from caml.inria.fr is much less of an issue. However, local installations of the HTML manual will either miss the font files (if relative URLs are used), or fetch them from caml.inria.fr (if absolute URLs are used). In the latter case, tracking can occur, just not from Google.

CSS have fallbacks for font declarations, so I have no problems using Fira if it's in the browser and some other font(s) as fallbacks.

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented Jul 30, 2018

Just an aside: one of the nice things about using open source fonts is that we're allowed to use them any way we like. They can be served by google, or INRIA, or a CDN, or available locally, as we prefer. We can pick any method that suits us.

@jasim

This comment has been minimized.

Copy link

jasim commented Jul 30, 2018

This is a font-family stack that can work well across operating systems:

font-family: system-ui, BlinkMacSystemFont, -apple-system, Segoe UI, Roboto, Oxygen, 
             Ubuntu, Cantarell, Fira Sans, Droid Sans, Helvetica Neue, sans-serif;

It comes from https://tailwindcss.com/docs/fonts/#sans-serif

Happy to help with a PR if it will work.

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented Jul 30, 2018

@jasim We have no reason whatsoever not to use a font file. Webfonts have been a thing for a while, and we get to precisely control what the user sees by selecting one. Given that the fonts are open source and that we can supply them at will, there's no incentive not to. They're not even very large. (This has also been beaten to death.)

@gasche

This comment has been minimized.

Copy link
Member

gasche commented Jul 31, 2018

For the record, the Fira Sans files (I obtained them from this website) are 192Kio in total:

28K	fira-sans-v8-latin-regular.eot
56K	fira-sans-v8-latin-regular.svg
56K	fira-sans-v8-latin-regular.ttf
28K	fira-sans-v8-latin-regular.woff
24K	fira-sans-v8-latin-regular.woff2
192K	total

In comparison, the PDF version of the manual that we serve from caml.inria.fr is 1.8Mio, and the HTML source of the "language extension" chapter is 184Kio.

@gasche

This comment has been minimized.

Copy link
Member

gasche commented Aug 1, 2018

Report: Xavier (privately) agreed to look the other way while I install font files. We still need someone to rewrite the manual styles to use relative paths, and the release rule to (download and?) install the files in the right place, so that they are included in the archive we distribute. Then I can regenerate the archives and upload them on the website. I don't mind doing the path-rewriting and Makefile change myself, but in that case it will wait until the end of August.

Note: from the root of the repository, the better way to run the release script (after building the distribution with make world.opt -j5) is

  cd manual
  RELEASENAME=4.08.0+dev make release

This puts the manual files in $HOME/release, which is an inconvenient hardcoded path, and you might have to mkdir -p this directory beforehand, but this is how it is right now.

@dbuenzli

This comment has been minimized.

Copy link
Contributor

dbuenzli commented Aug 1, 2018

Just want to add that you might also just go with woff2 rather than the whole browser compat menagerie of font files, see here for current support. If you find that too scary (remember though that this will gracefully degrade if the browser doesn't support it) I would just go with woff+woff2, see support for woff here.

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented Aug 1, 2018

BTW, side note: https://github.com/BlackFoundry/InriaFonts

I'm amused that the Black Foundry refers to this in their documentation as "the Inria type system".

@pmetzger

This comment has been minimized.

Copy link
Member

pmetzger commented Aug 1, 2018

(Sadly, the Inria type system lacks a monospace font!)

@nojb nojb referenced this pull request Aug 2, 2018

Merged

Distribute fonts with manual #1964

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.