-
Notifications
You must be signed in to change notification settings - Fork 649
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
Resolves issue 2143 #2155
Conversation
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
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. |
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.
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 X
s).
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.
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. |
📌 Commit c7bf3fe has been approved by |
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
☀️ Test successful - checks-travis |
I made three changes to improve the experience.
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