-
Notifications
You must be signed in to change notification settings - Fork 13.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
Tweaked Docs Design #12996
Tweaked Docs Design #12996
Conversation
Signed-off-by: Daniel Fagnan <[email protected]>
Looks cool, although it would be useful if you could upload a rendered copy somewhere, so that we could browse through and get a broader sense of what it looks like. (I wonder if "Type Definitions" would be better as "Type Aliases".) Also, does this design become responsive easily? (Is being mobile friendly a goal we should have?) |
@huonw Cool. I'm currently uploading it to S3 for people to see/use. Good question about being responsive.
Code examples (not the short signature snippets) often doesn't work very well on a small screens. I'll update the PR to make the search/header part more responsive. |
@bjz Agreed. |
I have the beginning of a responsive branch for the old impl here.
EDIT: I see that you raised it. I'll look at it when it's online. |
transition: border 500ms ease-out; | ||
-webkit-transition: border 500ms ease-out; | ||
-moz-transition: border 500ms ease-out; | ||
-o-transition: border 500ms ease-out; |
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.
All major browsers support these attributes, we shouldn't be using vendor prefixes.
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.
@adrientetar Ahh yes.
Signed-off-by: Daniel Fagnan <[email protected]>
Fixed some issues, making the overall design a little bit more responsive. (480px width, minimally useable size) Demo: http://titan-rust.s3-website-us-east-1.amazonaws.com/doc/std/index.html /cc @huonw @adrientetar Also, I removed the search button because it doesn't do anything (from what I can tell), so it's quite useless. |
@cmr Agreed. I've made the link colors more visible. Ahh, yeah. I see that about the search button. |
@@ -155,12 +184,31 @@ nav.sub { | |||
|
|||
.docblock h1, .docblock h2, .docblock h3, .docblock h4, .docblock h5 { |
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.
Where is this used? I can't seem to find the docblock
class.
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 is used for the H* within the docblock div which is in the #main.content block. Some pages might not have any of the headers, but many do.
For full responsivity, I think this will need to adjust the HTML to set the viewport width/default zoom (or whatever needs to happen); when I tested on my 480x800 phone (in vertical mode, i.e. width 480px) it still defaulted to being very zoomed out. That said, it's probably fastest to navigate the docs when more of the page can be viewed at once; so I'm not 100% sure about our API docs making it hard to zoom out on a phone. Of course the tutorial and guides should definitely be made as a usable as possible. |
@huonw Yeah, I'm testing that now. It's actually not that bad right now on mobile with the viewport width set correctly. The only major issues are font sizes being super inconsistent (thanks to the use of different units). The other issue is that the sidebar might not be best on mobile because of the limited width. Whenever the sidebar is displayed, the content has an even more limited width. |
Viewport should be set via HTML. |
@thehydroimpulse I am done reviewing. I have pushed a commit on top of your work, ada599255b5f6fd03d8c3a9e08374c1d61f7cbb7. It is only janitorial work, the design itself is unchanged. Branch is adrientetar/rust/tree/docs_design. |
@adrientetar Ok awesome. I'll pull your changes. |
Signed-off-by: Daniel Fagnan <[email protected]>
Signed-off-by: Daniel Fagnan <[email protected]>
Signed-off-by: Daniel Fagnan <[email protected]>
@adrientetar @cmr @huonw @bjz Updated S3 with the newest copy. The colors (like the green) might be a tad too bright, so let me know if that's the case. Ideally, in the future, the sidebar can be compacted into a drop-down right before the search box. Then this can be used on mobile or smaller screens comfortably. |
Nice. cc @brson. (Could you double check that the tutorial, manual and the guides are still rendered correctly? If they've changed at all maybe you could upload one of them?) |
@huonw http://titan-rust.s3-website-us-east-1.amazonaws.com/doc/tutorial.html (All the other guides also work correctly and haven't changed.) |
This does not affect tutorial and so on because they have a separate CSS file. |
Generally speaking, I think that the flat design of before was better. A bit too rounded here in my opinion. |
@adrientetar I'm not really looking to implement anything "flat" per say, just stuff that makes sense and simple (although that makes it a flat design sometimes). I'd argue the current iteration has a much more polished look, especially with a more visible crate header, instead of a generic header with an underline. |
@@ -51,7 +54,7 @@ pub fn render<T: fmt::Show, S: fmt::Show>( | |||
|
|||
<section class=\"sidebar\"> | |||
{logo, select, none{} other{ | |||
<a href='{root_path}{krate}/index.html'><img src='#' alt=''/></a> | |||
<a href='{root_path}{krate}/index.html'><img src='#' alt='' width='100px' /></a> |
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.
Shouldn't have 'px', attribute is expected without unit here.
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.
Done.
This is not just CSS changes. (But, now I'm in a position to look at the code, it's obvious that they have no effect either.) |
Taking a look at this now. |
This looks like a solid improvement to me, and I especially like that it brings the style more inline with the rest of our docs. Stylistically, I like the simplicity; I'm not crazy about the very rounded corners on headers. Please do keep trying to unify all our different HTML styles. If others' points are addressed then I'm happy to merge this. |
@brson Cool. Do you have any thoughts on the search button (i.e., should we keep it)? Yay or Nay? I can also make the corners slightly less rounded, if that makes things better. |
Let's leave the search button at least for now since others' want it. Do as you please with the corners. Thanks. |
Signed-off-by: Daniel Fagnan <[email protected]>
@brson Sounds good. I addressed the previous comments. If @adrientetar is done reviewing the code then I'll squash my commits. |
Will look at it later today (if you can squash commits since my last one it would be cool) |
@@ -155,20 +197,41 @@ nav.sub { | |||
|
|||
.docblock h1, .docblock h2, .docblock h3, .docblock h4, .docblock h5 { | |||
margin-left: 0; | |||
margin: 40px 0 10px 0; |
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.
Can you then cleanup the margin-left
that is right before it?
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.
Done.
@thehydroimpulse ping? This looks like a nice redesign! |
@alexcrichton Gonna finish this up tomorrow. |
Closing this in favor of #13485 |
- Cherry-pick from #12996 - Use Fira Sans for headlines and sidebar (Light), Heuristica for the body (Adobe Utopia derivative). Both are licensed under the SIL OFL license. - A few tweaks Two examples: [modified `std`](http://adrientetar.legtux.org/cached/rust-docs/std.htm) and [modified `std::io`](http://adrientetar.legtux.org/cached/rust-docs/io.htm). cc #13484 **Blocked on rust-lang/prev.rust-lang.org#25 (for hosting of the fonts), that's showcased [here](http://adrientetar.github.io/rust-www/).** cc @brson, @thehydroimpulse
Thoughts?
/cc @bjz @cmr