Skip to content

Resolves issue 2143 #2155

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

Merged
merged 3 commits into from
Jan 25, 2020
Merged

Resolves issue 2143 #2155

merged 3 commits into from
Jan 25, 2020

Conversation

Zexbe
Copy link
Contributor

@Zexbe Zexbe commented Jan 23, 2020

I made three changes to improve the experience.

The homepage links go to the overview instead of a specific version
I added the version next to the stats, so you can tell what they are for.
I added a link back to the overview page next to the stats.

On the overview page it looks like this "Stats Overview"
On a specific version it looks like this "Stats Overview for 0.2.4 (see all)"

Resolves issue #2143

Remaking to run job again

Zexbe added 3 commits January 20, 2020 21:13
Change the newest list on the front page to go to the overview of a crate, instead of a specific version.
Added a version next to stats, so you can tell what it is giving stats for. Also added a link back to the  crate overview page, if you want to see all the stats.
I updated the tests that expected the page to go to the specific version of the crate, and added a new test for the stats overview text. I also removed an extra variable in the template
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @carols10cents (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

Copy link
Member

@jtgeibel jtgeibel left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @Zexbe, this looks great to me! I've approved the visual change in Precy, so this should merge fine (don't worry about any of the remaining red Xs).

I really like the "(show all)" link. My only suggestion there is that maybe we should add an anchor and link directly to ...#stats-overview. That way if there is a long readme the user doesn't have to scroll back to the bottom of the page.

This is already a big improvement, so I'm fine merging as-is as well.

@Zexbe
Copy link
Contributor Author

Zexbe commented Jan 25, 2020

I tried to add an anchor, it didn't work. LinkTo does not have anything to add anchor. I also tried to create a link with an anchor without LinkTo, but because the page needs to be rendered, the anchor doesn't exist yet, so the browser does not scroll to it.

@Zexbe Zexbe requested a review from jtgeibel January 25, 2020 08:03
@jtgeibel
Copy link
Member

Thanks for digging into that @Zexbe! There's probably a way to check for the fragment and scroll from JS once the page renders, but I'm not too worried about it.

@bors r+

@bors
Copy link
Contributor

bors commented Jan 25, 2020

📌 Commit c7bf3fe has been approved by jtgeibel

@bors
Copy link
Contributor

bors commented Jan 25, 2020

⌛ Testing commit c7bf3fe with merge 39b4e76...

bors added a commit that referenced this pull request Jan 25, 2020
Resolves issue 2143

I made three changes to improve the experience.

    The homepage links go to the overview instead of a specific version
    I added the version next to the stats, so you can tell what they are for.
    I added a link back to the overview page next to the stats.

On the overview page it looks like this "Stats Overview"
On a specific version it looks like this "Stats Overview for 0.2.4 (see all)"

Resolves issue #2143

Remaking to run job again
@bors
Copy link
Contributor

bors commented Jan 25, 2020

☀️ Test successful - checks-travis
Approved by: jtgeibel
Pushing 39b4e76 to master...

@bors bors merged commit c7bf3fe into rust-lang:master Jan 25, 2020
@Zexbe Zexbe deleted the pull-request-1 branch January 25, 2020 17:15
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.

5 participants