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

HTML head customisation with head.hbs #1206

Merged
merged 5 commits into from
May 10, 2020

Conversation

GillesRasigade
Copy link

Allow to define custom attributes within the HTML tag thanks to a head.hbs template file.

A typical use case is the addition of mobile capabilities for PWA deployment for example to render the doc in fullscreen once added to the home screen (Android or iOS):

<meta name="apple-mobile-web-app-capable" content="yes" />
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<meta name="mobile-web-app-capable" content="yes">

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Could you also update the documentation in the theme docs for this and header.hbs?

@@ -75,6 +75,7 @@ impl HelperDef for RenderToc {
("", 1)
};

#[warn(clippy::if_same_then_else)]
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure why this is added.

Copy link
Author

Choose a reason for hiding this comment

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

I added this to avoid having a error after executing clippy:
Screenshot from 2020-05-10 10-17-35

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see. I just pushed a change to rewrite it: #1214. I usually prefer unrelated changes like this to be separate.

}
html.classList.remove('sidebar-visible');
html.classList.add("sidebar-" + sidebar);
</script>

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer to avoid unrelated formatting changes. Can you remove all the whitespace changes and formatting changes?

Copy link
Author

Choose a reason for hiding this comment

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

Code auto-formatting has been reverted.

@ehuss
Copy link
Contributor

ehuss commented May 7, 2020

@Dylan-DPC does this seem like a reasonable idea to you? I suspect there might be some longer term issues with making it easier to extend the templates (#353 ?), but this seems like a simple, incremental improvement.

@Dylan-DPC-zz
Copy link

i don't see any issue with this

@GillesRasigade
Copy link
Author

Documentation added. Comments answered. Thanks for your reviews.

@ehuss ehuss force-pushed the html-head-custom branch from 11eee71 to 50bcf67 Compare May 10, 2020 15:43
Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks!

@ehuss ehuss merged commit 7e11d37 into rust-lang:master May 10, 2020
@code-ink-space
Copy link

The documentation might be confusing.
https://rust-lang.github.io/mdBook/format/theme/index.html

It says:

The theme is totally customizable, you can selectively replace every file from the theme by your own by adding a theme directory next to src folder in your project root.

I think it should say "under" or "inside" the src folder because that's what worked for me. Not having theme next to src but inside it.
Doing mdbook init --theme made me see the correct directory structure.

Ruin0x11 pushed a commit to Ruin0x11/mdBook that referenced this pull request Aug 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants