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

Please use a little more width for crate pages on screens #970

Closed
joshtriplett opened this Issue Aug 19, 2017 · 3 comments

Comments

Projects
None yet
3 participants
@joshtriplett
Copy link
Member

joshtriplett commented Aug 19, 2017

When rendering on a full-size screen (rather than mobile), the crates page still renders content in a very narrow column. The width used on almost every other page seems fine, but crate pages have a sidebar, which narrows the region available for the README content.

Take a look at https://crates.io/crates/colorparse, and compare it to https://github.com/joshtriplett/colorparse/. Notice how the code in the latter doesn't wrap, while it does in the former.

I think it would be reasonable for the main (non-sidebar) crate content on crate pages to have the same width that the overall (non-columnar) content has on other pages. That would make the README much more readable, without sacrificing the critical property of avoiding excessively wide lines.

Does that seem reasonable?


Implementation instructions added by @carols10cents:

  • Increase the max-width set on .crate-info .docs (I think) so that it's about as wide as readmes appear on GitHub
  • Test that change at different screen widths and make adjustments if anything looks weird
  • Send in a PR and include some screenshots! npm run start:live will come in handy for this :)

@carols10cents carols10cents added the A-CSS label Sep 4, 2017

@carols10cents carols10cents added this to the impl period milestone Sep 15, 2017

@carols10cents carols10cents removed this from the impl period milestone Sep 15, 2017

@tcbyrd

This comment has been minimized.

Copy link
Contributor

tcbyrd commented Sep 18, 2017

Hey all! Just found this after reading through the roadmap for the rest of the year. Happy to help out and submit my first PR to the project, but first wanted to just discuss what I found.

If the docs div is increased to 640px, the overflow goes away in this example without any major side effects:
screenflow

This change would make it so code shouldn't hit the overflow up to about 65 characters of line-length. Making it as wide as GitHub's README would mean having to move the authorship div somewhere else or making everything wider, which is something that may require a bit more discussion beyond the scope of this Issue. I'd be willing to open a PR with some suggestions on the latter if needed.

@joshtriplett

This comment has been minimized.

Copy link
Member Author

joshtriplett commented Sep 18, 2017

@tcbyrd I think the right solution would be to widen the containing div as well (the beige area) and leave the authorship sidebar the same size. Some other pages already have a wider content area.

@tcbyrd

This comment has been minimized.

Copy link
Contributor

tcbyrd commented Sep 18, 2017

I agree. 960px is a pretty decent width for the whole thing. I pulled down the source and noticed this doesn't change the docs because they are hosted on GitHub Pages. To make it consistent we'd also need to change this line in rust-lang/cargo/tree/gh-pages/stylesheets/all.css

Since I already setup the dev environment, I'll open a PR tomorrow unless I hear otherwise.

bors-voyager bot added a commit that referenced this issue Sep 21, 2017

Merge #1064
1064: Make the page wider to give more room for README on desktop r=carols10cents

Fixes #970

**Summary**
This is my first PR to this repo, but I saw the Issue above and thought I'd try and tackle it since it's fairly minimal, but is a nice change to help with readability. This is a suggestion to widen the width of the main div to 960px so that the `.crate-info .docs` div can be increased a bit without having to decrease the size of the `.crate-info .authorship` div.

**Reasoning**
I mainly chose 960px since it's a fairly widely accepted standard for content of this type. This allows the documentation to be 640px, which increases the line-length of the `pre` elements to 65 to help minimize horizontal scrolling.

**Before/After**
![screenflow](https://user-images.githubusercontent.com/13207348/30617484-440b40ee-9d64-11e7-9d8a-9c0c9e74a4ac.gif)

**Other Changes**
I also changed the responsive screen width option to match the 960px width. The only major side effect here is the authorship div gets a little narrow until it switches to 100% at 900px, but that was already happening at 1040px and it's still readable. The other option is to leave it at 960px and just let the whole page horizontally scroll from 900-960px.

_Example of the narrow authorship div @ 915px_
![image](https://user-images.githubusercontent.com/13207348/30618133-17211a2e-9d67-11e7-9cef-ac13b2b8306a.png)

**Docs Updates**
If the team wants to accept this change, for the docs to be consistent, [this line](https://github.com/rust-lang/cargo/blob/73eb8c424dc3732cea23769982d0e4e261d5399e/stylesheets/all.css#L6) in the [GitHub pages branch](https://github.com/rust-lang/cargo/tree/gh-pages) of the Cargo repo will also need to be updated.

I saw there were some discussions about a much bigger redesign, but I thought this may help in the interim since it's a fairly minimal change. I'm open to any feedback! 😄

@bors-voyager bors-voyager bot closed this in #1064 Sep 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.