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

Make the page wider to give more room for README on desktop #1064

Merged
merged 2 commits into from Sep 21, 2017

Conversation

Projects
None yet
3 participants
@tcbyrd
Copy link
Contributor

tcbyrd commented Sep 19, 2017

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

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

Docs Updates
If the team wants to accept this change, for the docs to be consistent, this line in the GitHub pages branch 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! 😄

@carols10cents

This comment has been minimized.

Copy link
Member

carols10cents commented Sep 20, 2017

Welcome!! This looks great!! I love the gifs showing the difference too ❤️ ❤️ ❤️ Thank you!!

bors: r+

@carols10cents

This comment has been minimized.

Copy link
Member

carols10cents commented Sep 21, 2017

bors: r+

let's try this again

bors-voyager bot added a commit that referenced this pull request 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

This comment has been minimized.

Copy link
Contributor

bors-voyager bot commented Sep 21, 2017

Build succeeded

@bors-voyager bors-voyager bot merged commit 835868c into rust-lang:master Sep 21, 2017

2 checks passed

bors Build succeeded
continuous-integration/travis-ci/pr The Travis CI build passed
Details
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.