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.

Sign up for GitHub

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

Jump to bottom

CSS adjustments in post-processed HTML manual #10497

Merged
merged 19 commits into from
Aug 18, 2021
Merged

CSS adjustments in post-processed HTML manual #10497

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
f8390bc
webman: Remove colored background for inline code
wikku Jul 9, 2021
a194d25
webman: Follow inline code styling of htmlman
wikku Jul 9, 2021
ef6d050
webman: Italicize grammar symbol names
wikku Jul 9, 2021
d5fe429
webman: Make display style blocks centered
wikku Jul 9, 2021
61764cc
webman: Increase heading font-weight
wikku Jul 9, 2021
8900ac5
webman: Use SCSS variables for font-family
wikku Jul 14, 2021
b4eb567
webman: Set command-line arguments in monospace
wikku Jul 14, 2021
a50a4d4
webman: Set (mathematical) variables in italics
wikku Jul 14, 2021
cfd3c03
webman: Remove styling for the ordinal "th"
wikku Jul 14, 2021
7c1fd96
webman/api: Improve constructor color contrast
wikku Jul 14, 2021
e311dab
webman: Center tables
wikku Jul 20, 2021
afe24ba
webman: Make table headers bold
wikku Jul 20, 2021
f6355cf
webman: Fix variables being bolded, not italicized
wikku Jul 20, 2021
cfb2545
webman: Add spacing to tables for aligned exprs
wikku Jul 20, 2021
321a227
webman: Add padding to .cellpadding1
wikku Jul 20, 2021
27ae0f5
webman: Styling for Unix/Windows specific blocks
wikku Jul 20, 2021
d56cbd0
webman: Italicize command-line argument variables
wikku Jul 20, 2021
49b9cd3
webman: Center BNF syntax bars `|`
wikku Jul 20, 2021
9671c87
Add Changes entry
wikku Aug 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions Changes
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,9 @@ OCaml 4.13.0
- #9632: Document incremental build solutions with opam
(Vincent Laviron, review by Daniel Bünzli and Gabriel Scherer)

- #10497: Styling changes in the post-processed HTML manual (webman)
(Wiktor Kuchta, review by Florian Angeletti)

### Compiler user-interface and warnings:

- #1737, #2092, #7852, #7859, #10405, #10417: Update locations during
Expand Down
16 changes: 10 additions & 6 deletions manual/src/html_processing/scss/_common.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ $logo_height:67px;
@import url(https://fonts.googleapis.com/css?family=Noticia+Text:400,400i,700);
@import url(https://fonts.googleapis.com/css?family=Fira+Sans:400,400i,500,500i,600,600i,700,700i);

$font-sans: "Fira Sans", Helvetica, Arial, sans-serif;
$font-mono: "Fira Mono", courier, monospace;
$font-serif: "Noticia Text", Georgia, serif;

/* Reset */
.pre,a,b,body,code,div,em,form,h1,h2,h3,h4,h5,h6,header,html,i,img,li,mark,menu,nav,object,output,p,pre,s,section,span,time,ul,td,var{
margin:0;
Expand Down Expand Up @@ -47,7 +51,7 @@ html.smooth-scroll {
}

body{
font-family:"Fira Sans",Helvetica,Arial,sans-serif;
font-family: $font-sans;
text-align:left;
color:#333;
background:#fff
Expand Down Expand Up @@ -76,7 +80,7 @@ html {
&>header {
margin-bottom: 30px;
nav {
font-family: "Fira Sans", Helvetica, Arial, sans-serif;
font-family: $font-sans;
}
}
}
Expand All @@ -87,7 +91,7 @@ html {
margin-right:4ex;
margin-top:20px;
margin-bottom:50px;
font-family:"Noticia Text",Georgia,serif;
font-family: $font-serif;
line-height:1.5
}

Expand Down Expand Up @@ -131,7 +135,7 @@ html {
padding-left:12px;
}
a {
font-family:"Fira Sans",sans-serif;
font-family: $font-sans;
font-size:.95em;
color:#333;
font-weight:400;
Expand Down Expand Up @@ -250,7 +254,7 @@ html {
color:$logocolor;
margin-right:4px;
margin-left:-1em;
font-family:"Fira Sans",Helvetica,Arial,sans-serif;
font-family: $font-sans;
font-size:13px;
vertical-align:1px;
}
Expand All @@ -260,7 +264,7 @@ html {
color:$logocolor;
margin-right:4px;
margin-left:-1em;
font-family:"Fira Sans",Helvetica,Arial,sans-serif;
font-family: $font-sans;
font-size:14px;
vertical-align:1px;
}
69 changes: 54 additions & 15 deletions manual/src/html_processing/scss/manual.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
float:left;
color:#777;
cursor: context-menu;
font-family:"Fira Sans",Helvetica,Arial,sans-serif;
font-family: $font-sans;
span{ /* menu icon */
font-size:22px;
margin-right:1ex;
Expand Down Expand Up @@ -63,7 +63,7 @@
font-size:smaller;
}
section>ul>li>a{ /* for Parts title */
font-family:"Fira Sans",Helvetica,Arial,sans-serif;
font-family: $font-sans;
font-size:larger;
background:linear-gradient(to left,#fff 0,#ede8e5 100%);
}
Expand Down Expand Up @@ -143,7 +143,7 @@ a.section-anchor{
color:#d5d5d5
}
.h10,.h7,.h8,.h9,h1,h2,h3,h4,h5,h6{
font-family:"Fira Sans",Helvetica,Arial,sans-serif;
font-family: $font-sans;
font-weight:400;
margin:.5em 0 .5em 0;
padding-top:.1em;
Expand Down Expand Up @@ -185,8 +185,11 @@ h3 code{
h4{
font-size:1.12em
}
h2, h3, h4, h5 {
font-weight: 500;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some higher-level description of the change would be great here too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is non-essential and purely subjective. I think it made it easier to skim for subsection headings and adds some visual variation to the whole thing, gives less of a "wall of text" feeling.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point and that seems potential nicer to me. It might be worthwhile to get a third advice (maybe on a larger text fragment to see how multiple titles mesh with the rest of the text?).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, this change is not without precedent, as both the old htmlman and LaTeX use a slightly higher font weight for those headings.

.ocaml,.pre,code,pre,tt{
font-family:"Fira Mono",courier;
font-family: $font-mono;
font-weight:400
}
.pre,pre{
Expand Down Expand Up @@ -263,14 +266,13 @@ h1 span{
color: #d28853;
}
blockquote.quote{
margin:0;
/*font-size: smaller;*/
hr{
display:none;
}
}
#part-menu{
font-family:"Fira Sans";
font-family: $font-sans;
text-align:right;
list-style:none;
overflow-y:hidden;
Expand All @@ -280,10 +282,21 @@ blockquote.quote{
color:#000;
&::before{@include diamond}
}
.center {
text-align: center;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Horizontally aligning text on the center seems like a mistake outside of tables?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is necessary to center the displayed syntax fragments, as in the screenshot. These rules are just copied from the old HTML manual CSS.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that having some indenting would be better than centering texts.

margin-left: auto;
margin-right: auto;
}
.display {
margin: 0 auto;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the old manual css this styling was on .dcenter which seems a little bit clearer in term of intents.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The classes .display and .dcenter seem to always coexist, so it doesn't really matter. I don't think it's clearer, they're just different approaches ("semantic" vs "non-semantic" CSS)

}
.c001 {
border-spacing: 6px;
border-collapse: separate;
}
span.c003{
color:#564233;
font-family:"Fira Mono",courier;
background-color:#f3ece6;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the motivation behind the change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PDF and old HTML manuals don't color the background of inline code.

In cases where you have syntax fragments interleaved with eg. grammar rules, this kind of styling can make the whole thing look incoherent, especially when they are inside a paragraph. You can take look at "Local modules". Maybe it would be nice if we could make the whole construct "let module module-name =  module-expr in  expr" have a colored background with mixed typewriter and italic font inside, but we can't do it now since it's not a single HTML element.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The incoherence is a good point, thanks for pointing that out.

font-family: $font-mono;
border-radius:6px
}
div.caml-example.toplevel code.caml-input::before,
Expand All @@ -294,25 +307,51 @@ div.caml-example.toplevel div.caml-input::before{
span.number{
padding-right: 1ex;
}
span.c004, span.c002{
span.c004, span.c005, span.c007 {
font-family: $font-mono;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a monospace fonts for command-line arguments make sense.

}
span.c003, span.c005 {
color: rgba(91, 33, 6, 0.87);
}
span.c002{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the motivation for the color change on c0003, c0005 ? The font change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The font change for .c004 and .c005 sets respectively the Match_failure and let in the screenshot in monospace font. I tried to follow the CSS rules from the old HTML manual. I took the font-family from other parts of the file, but I added the monospace generic fallback at the end, as is good practice. Should I fix that in other places of the file while I'm here?

The color #888888 that was used for (some) monospace fragments has simply too low contrast on #ffffff, fails accessibility guidelines (even more on darker backgrounds like in tables) and looks grayed out and unimportant (to me, identifiers and syntax fragments are pretty important and should not blend into the background).

Instead I chose a safe, non-offending color by simply darkening the color currently used for links (and adding a bit of transparency, to make it work better on darker backgrounds like in tables). Maybe a later PR will pick something better, but I wouldn't want to discuss this choice now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that switching the font for inline code to monospace is the right thing to do (we should get ride of those hevea generated classes at some points).

Adding a monospace failback to the CSS sounds like a good idea (in a separate commit probably).

I agree that the previous color has too little contrast. Having a color that is close from the grammatical symbol color seems like a sensible idea.

color:#888
}
span.c006{
font-weight:700;
color:#564233;
font-family:"Fira Mono",courier;
font-family: $font-mono;
}
span.c009{
font-style:italic;
background-color:#f3ece6;
border-radius:6px
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This style was supposed to be a duplicate of c010 (due to a change of the hevea-generated html). I agree that simpler is better here.

.c008 {
font-family: $font-sans;
}
span.c010 {
font-style: italic;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in line with the simple html manual and the pdf one. Good.

}
span.authors{
font-style:italic;
background-color:inherit
}
span.c011 {
font-style: italic;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aligning the graphical style with the description is great indeed.

.c012 {
font-style: italic;
}
span.c013{
font-weight:700
font-style: italic;
}
.center table {
margin-left: inherit;
margin-right: inherit;
}
td .c014 {
font-weight: bold;
}
.c016 {
text-align: center;
}
.cellpadding1 tr td {
padding: 1px 4px;
}
.caml-input{
span.ocamlkeyword{
Expand Down
15 changes: 7 additions & 8 deletions manual/src/html_processing/scss/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@

.api {
// font-size: 16px;
// font-family: "Fira Sans", Helvetica, Arial, sans-serif;
// font-family: $font-sans;
// text-align: left;
// color: #333;
// background: #FFFFFF;
Expand Down Expand Up @@ -259,7 +259,7 @@
we restart the sequence there like h2 */

h1, h2, h3, h4, h5, h6, .h7, .h8, .h9, .h10 {
font-family: "Fira Sans", Helvetica, Arial, sans-serif;
font-family: $font-sans;
font-weight: 400;
margin: 0.5em 0 0.5em 0;
padding-top: 0.1em;
Expand Down Expand Up @@ -316,7 +316,7 @@
/* Preformatted and code */

tt, code, pre {
font-family: "Fira Mono", courier;
font-family: $font-mono;
font-weight: 400;
}

Expand Down Expand Up @@ -705,7 +705,7 @@
span.arrow {
font-size: 20px;
line-height: 8pt;
font-family: "Fira Mono";
font-family: $font-mono;
}
header dl dd, header dl dt {
display: inline-block;
Expand Down Expand Up @@ -742,7 +742,7 @@
}

ul.tutos_menu {
font-family: "Fira Sans";
font-family: $font-sans;
text-align: right;
list-style: none;
}
Expand All @@ -756,7 +756,7 @@
}

span.c003 {
font-family: "Fira Mono", courier;
font-family: $font-mono;
background-color: #f3ece6;
border-radius: 6px;
}
Expand Down Expand Up @@ -793,8 +793,7 @@

code span.constructor,
.caml-input span.kw2 {
font-weight: 500;
color: #a28867;
color: #8d543c;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reducing the number of colors seems fine, but what is the reasoning behind reducing the font weight?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may have had another reason I can't remember, but I don't see why it would be bolded in the first place and it's not bolded in htmlman. I think now only keywords like let and while are bolded, which I think is standard when displaying code.

It's also annoying that .constructor in the HTML applies to both data constructors and modules, having a different style for modules (maybe just bolded) would be nice.

}

.caml-input span.numeric {
Expand Down