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

Multiple themes for rustdoc #47620

Merged
merged 6 commits into from
Jan 23, 2018
Merged

Multiple themes for rustdoc #47620

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
003b2bc
Add possibility to have multiple themes
GuillaumeGomez Jan 20, 2018
9aee164
Add themes option
GuillaumeGomez Jan 20, 2018
aae6dc4
tweak colors on dark theme
QuietMisdreavus Jan 22, 2018
3c52acd
Change theme icon
GuillaumeGomez Jan 22, 2018
5f93159
Fasten up theme loading
GuillaumeGomez Jan 22, 2018
5b85044
Fasten even more theme switch
GuillaumeGomez Jan 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/librustdoc/externalfiles.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use std::str;
use html::markdown::{Markdown, RenderType};

#[derive(Clone)]
pub struct ExternalHtml{
pub struct ExternalHtml {
/// Content that will be included inline in the <head> section of a
/// rendered Markdown file or generated documentation
pub in_header: String,
Expand Down
8 changes: 6 additions & 2 deletions src/librustdoc/html/layout.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ r##"<!DOCTYPE html>
<title>{title}</title>

<link rel="stylesheet" type="text/css" href="{root_path}normalize.css">
<link rel="stylesheet" type="text/css" href="{root_path}rustdoc.css">
<link rel="stylesheet" type="text/css" href="{root_path}main.css">
<link rel="stylesheet" type="text/css" href="{root_path}rustdoc.css" id="mainThemeStyle">
<link rel="stylesheet" type="text/css" href="{root_path}main.css" id="themeStyle">
{css_extension}

{favicon}
Expand All @@ -70,6 +70,10 @@ r##"<!DOCTYPE html>
{sidebar}
</nav>

<div id="theme-picker">&#x1f58c;
Copy link
Member

Choose a reason for hiding this comment

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

This icon is not present in the fonts we bundle with rustdoc, nor in the system fonts in Windows 7:

image

Copy link

Choose a reason for hiding this comment

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

theme-picker and theme-choices need some ARIA attributes (see this for how it's handled in mdBook).

Copy link
Member Author

Choose a reason for hiding this comment

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

In mdbook it breaks elements alignment so I'd prefer not use the same mechanisms.

Copy link

Choose a reason for hiding this comment

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

How does it break elements alignment? Also, this should be a button.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't see the point of this being a button... :-/

Copy link

Choose a reason for hiding this comment

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

Can you click it? Then it should be a button or a link.

<div id="theme-choices"></div>
</div>
<script src="{root_path}theme.js"></script>
<nav class="sub">
<form class="search-form js-only">
<div class="search-container">
Expand Down
95 changes: 91 additions & 4 deletions src/librustdoc/html/render.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ pub struct SharedContext {
/// This flag indicates whether listings of modules (in the side bar and documentation itself)
/// should be ordered alphabetically or in order of appearance (in the source code).
pub sort_modules_alphabetically: bool,
/// Additional themes to be added to the generated docs.
pub themes: Vec<PathBuf>,
}

impl SharedContext {
Expand Down Expand Up @@ -219,6 +221,17 @@ impl Error {
}
}

macro_rules! try_none {
($e:expr, $file:expr) => ({
use std::io;
match $e {
Some(e) => e,
None => return Err(Error::new(io::Error::new(io::ErrorKind::Other, "not found"),
$file))
}
})
}
Copy link
Member

Choose a reason for hiding this comment

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

since rustdoc is on nightly anyway, why not just use ?

Copy link
Member Author

Choose a reason for hiding this comment

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

We need to make a conversion. I intended to use ? but it cannot be used directly unfortunately...

Copy link
Member

Choose a reason for hiding this comment

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

This doesn't do the same as ? - it returns an Err instead of None.

Copy link
Member

Choose a reason for hiding this comment

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

isn't that how ? works on an Option inside something returning Result? You get https://doc.rust-lang.org/stable/std/option/struct.NoneError.html, and you can do the appropriate From impl if you want a custom thing

Copy link
Member

Choose a reason for hiding this comment

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

It also takes the path of the file that caused the failure, which won't be there in the NoneError.

(I also didn't realize that there was a conversion available between NoneError and custom error types >_>)


macro_rules! try_err {
($e:expr, $file:expr) => ({
match $e {
Expand Down Expand Up @@ -489,7 +502,8 @@ pub fn run(mut krate: clean::Crate,
renderinfo: RenderInfo,
render_type: RenderType,
sort_modules_alphabetically: bool,
deny_render_differences: bool) -> Result<(), Error> {
deny_render_differences: bool,
themes: Vec<PathBuf>) -> Result<(), Error> {
let src_root = match krate.src {
FileName::Real(ref p) => match p.parent() {
Some(p) => p.to_path_buf(),
Expand All @@ -513,6 +527,7 @@ pub fn run(mut krate: clean::Crate,
markdown_warnings: RefCell::new(vec![]),
created_dirs: RefCell::new(FxHashSet()),
sort_modules_alphabetically,
themes,
};

// If user passed in `--playground-url` arg, we fill in crate name here
Expand Down Expand Up @@ -859,12 +874,84 @@ fn write_shared(cx: &Context,
// Add all the static files. These may already exist, but we just
// overwrite them anyway to make sure that they're fresh and up-to-date.

write(cx.dst.join("main.js"),
include_bytes!("static/main.js"))?;
write(cx.dst.join("rustdoc.css"),
include_bytes!("static/rustdoc.css"))?;

// To avoid "main.css" to be overwritten, we'll first run over the received themes and only
// then we'll run over the "official" styles.
let mut themes: HashSet<String> = HashSet::new();

for entry in &cx.shared.themes {
let mut content = Vec::with_capacity(100000);

let mut f = try_err!(File::open(&entry), &entry);
try_err!(f.read_to_end(&mut content), &entry);
write(cx.dst.join(try_none!(entry.file_name(), &entry)), content.as_slice())?;
themes.insert(try_none!(try_none!(entry.file_stem(), &entry).to_str(), &entry).to_owned());
}

write(cx.dst.join("main.css"),
include_bytes!("static/styles/main.css"))?;
include_bytes!("static/themes/main.css"))?;
themes.insert("main".to_owned());
write(cx.dst.join("dark.css"),
include_bytes!("static/themes/dark.css"))?;
themes.insert("dark".to_owned());

let mut themes: Vec<&String> = themes.iter().collect();
themes.sort();
// To avoid theme switch latencies as much as possible, we put everything theme related
// at the beginning of the html files into another js file.
write(cx.dst.join("theme.js"), format!(
r#"var themes = document.getElementById("theme-choices");
Copy link
Member

Choose a reason for hiding this comment

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

this might be signficantly nicer as an external file that's include_bytes'd.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes but I need to generate the list of themes which can't be know at compile time. However, I intend to move most of this code into a js file later.

var themePicker = document.getElementById("theme-picker");
themePicker.onclick = function() {{
if (themes.style.display === "block") {{
Copy link

Choose a reason for hiding this comment

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

Both branches should update aria-expanded (see mdBook).

themes.style.display = "none";
themePicker.style.borderBottomRightRadius = "3px";
themePicker.style.borderBottomLeftRadius = "3px";
}} else {{
themes.style.display = "block";
themePicker.style.borderBottomRightRadius = "0";
themePicker.style.borderBottomLeftRadius = "0";
}}
}};
var currentTheme = document.getElementById("themeStyle");
var mainTheme = document.getElementById("mainThemeStyle");
[{}].forEach(function(item) {{
var div = document.createElement('div');
Copy link

Choose a reason for hiding this comment

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

button, please.

div.innerHTML = item;
div.onclick = function(el) {{
Copy link

Choose a reason for hiding this comment

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

This will create an event listener for each theme if I understand correctly. What about just one (on theme-choices)?

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer to avoid js code as much as possible. Like this, no need to try to find which child sent the event.

Copy link

Choose a reason for hiding this comment

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

Yeah, but that comes at a performance cost.

Copy link
Member Author

Choose a reason for hiding this comment

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

What performance cost? O.o The bottleneck is clearly not in js events.

Copy link

Choose a reason for hiding this comment

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

True, not a bottleneck, but why not avoid extra event listeners on a page? It is one or two extra lines of code.

switchTheme(currentTheme, mainTheme, item);
}};
themes.appendChild(div);
}});

function updateLocalStorage(theme) {{
if (typeof(Storage) !== "undefined") {{
localStorage.theme = theme;
}} else {{
// No Web Storage support so we do nothing
}}
}}
function switchTheme(styleElem, mainStyleElem, newTheme) {{
styleElem.href = mainStyleElem.href.replace("rustdoc.css", newTheme + ".css");
updateLocalStorage(newTheme);
}}
function getCurrentTheme() {{
if (typeof(Storage) !== "undefined" && localStorage.theme !== undefined) {{
return localStorage.theme;
}}
return "main";
}}

switchTheme(currentTheme, mainTheme, getCurrentTheme());
"#, themes.iter()
.map(|s| format!("\"{}\"", s))
.collect::<Vec<String>>()
.join(",")).as_bytes())?;

write(cx.dst.join("main.js"), include_bytes!("static/main.js"))?;

if let Some(ref css) = cx.shared.css_file_extension {
let out = cx.dst.join("theme.css");
try_err!(fs::copy(css, out), css);
Expand Down
12 changes: 11 additions & 1 deletion src/librustdoc/html/static/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@
}
}
document.getElementsByTagName("body")[0].style.marginTop = '45px';
var themePicker = document.getElementById("theme-picker");
if (themePicker) {
themePicker.style.position = "fixed";
}
}

function hideSidebar() {
Expand All @@ -136,6 +140,10 @@
filler.remove();
}
document.getElementsByTagName("body")[0].style.marginTop = '';
var themePicker = document.getElementById("theme-picker");
if (themePicker) {
themePicker.style.position = "absolute";
}
}

// used for special search precedence
Expand Down Expand Up @@ -1532,7 +1540,9 @@
ul.appendChild(li);
}
div.appendChild(ul);
sidebar.appendChild(div);
if (sidebar) {
sidebar.appendChild(div);
}
}

block("primitive", "Primitive Types");
Expand Down
38 changes: 37 additions & 1 deletion src/librustdoc/html/static/rustdoc.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,8 @@ ul.item-list > li > .out-of-band {
}

h4 > code, h3 > code, .invisible > code {
position: inherit;
max-width: calc(100% - 41px);
display: block;
}

.in-band, code {
Expand All @@ -376,6 +377,7 @@ h4 > code, h3 > code, .invisible > code {
margin: 0px;
padding: 0px;
display: inline-block;
max-width: calc(100% - 43px);
}

.in-band > code {
Expand Down Expand Up @@ -1140,3 +1142,37 @@ kbd {
border-radius: 3px;
box-shadow: inset 0 -1px 0;
}

#theme-picker {
position: absolute;
left: 211px;
top: 17px;
padding: 4px;
border: 1px solid;
border-radius: 3px;
cursor: pointer;
}

#theme-choices {
display: none;
position: absolute;
left: -1px;
top: 30px;
border: 1px solid;
border-radius: 3px;
z-index: 1;
}

#theme-choices > div {
border-top: 1px solid;
padding: 4px;
text-align: center;
}

@media (max-width: 700px) {
#theme-picker {
left: 109px;
top: 7px;
z-index: 1;
}
}
Loading