-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
CSS adjustments in post-processed HTML manual #10497
Changes from 8 commits
f8390bc
a194d25
ef6d050
d5fe429
61764cc
8900ac5
b4eb567
a50a4d4
cfd3c03
7c1fd96
e311dab
afe24ba
f6355cf
cfb2545
321a227
27ae0f5
d56cbd0
49b9cd3
9671c87
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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%); | ||
} | ||
|
@@ -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; | ||
|
@@ -185,8 +185,11 @@ h3 code{ | |
h4{ | ||
font-size:1.12em | ||
} | ||
h2, h3, h4, h5 { | ||
font-weight: 500; | ||
} | ||
.ocaml,.pre,code,pre,tt{ | ||
font-family:"Fira Mono",courier; | ||
font-family: $font-mono; | ||
font-weight:400 | ||
} | ||
.pre,pre{ | ||
|
@@ -270,7 +273,7 @@ blockquote.quote{ | |
} | ||
} | ||
#part-menu{ | ||
font-family:"Fira Sans"; | ||
font-family: $font-sans; | ||
text-align:right; | ||
list-style:none; | ||
overflow-y:hidden; | ||
|
@@ -280,10 +283,17 @@ blockquote.quote{ | |
color:#000; | ||
&::before{@include diamond} | ||
} | ||
.center { | ||
text-align: center; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the old manual css this styling was on There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The classes |
||
} | ||
span.c003{ | ||
color:#564233; | ||
font-family:"Fira Mono",courier; | ||
background-color:#f3ece6; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the motivation behind the change? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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, | ||
|
@@ -294,23 +304,35 @@ 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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The font change for The color 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
} | ||
span.c010 { | ||
font-style: italic; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aligning the graphical style with the description is great indeed. |
||
span.c013{ | ||
font-weight:700 | ||
} | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?).
There was a problem hiding this comment.
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.