-
-
Notifications
You must be signed in to change notification settings - Fork 922
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
Providing some context to how long ago a version was released #4352
Providing some context to how long ago a version was released #4352
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4352 +/- ##
=======================================
Coverage 97.02% 97.02%
=======================================
Files 420 420
Lines 8726 8726
=======================================
Hits 8466 8466
Misses 260 260 ☔ View full report in Codecov by Sentry. |
app/views/rubygems/_aside.html.erb
Outdated
<h2 class="gem__ruby-version__heading t-list__heading"> | ||
<%= t('.gem_version_age') %>: | ||
<span class="gem__rubygem-version-age"> | ||
<p><%= time_ago_in_words(@latest_version.authored_at) %> ago</p> |
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.
@segiddins if I understand it well, this is going to break caching, since the page content will change in time even with no resource update. Would it be better to use JS solution like https://momentjs.com/docs/#/displaying/fromnow/?
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.
I have found out also https://github.com/basecamp/local_time.
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.
@robbyrussell would you mind to try to use local_time gem? Feel free to ping me if you need help.
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.
I'm sorry for not considering the caching aspect that is in production.
The install instructions for the current releases of local_time assume you might have importmaps and/or webpacker in place. Are there plans for this project to use either approach going forward? (and past versions on NPM)
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.
No, but can't you just import one of those (https://github.com/basecamp/local_time/tree/main/app/assets/javascripts) using current pipeline (require here - https://github.com/rubygems/rubygems.org/blob/master/app/assets/javascripts/application.js)?
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.
@robbyrussell there is PR introducing importmaps at #4392. Maybe it is worth to wait for that one to land before tackling this.
@robbyrussell import maps has landed already. Are you still interested in finishing this? |
Assigning so I can try to get this into the design refresh that is coming. |
30c984d
to
78508de
Compare
I have updated this to use |
.gem__rubygem-version-age { | ||
margin-top: 10px; | ||
margin-bottom: 30px; | ||
display: block; |
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.
Instead of this, maybe use a div instead of a span?
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.
it follows the rest, IMHO ok
Thanks @martinemde, I didn't see the last comments. If you need anything from me, don't hesitate to let me know |
Proposal: When you're looking at a particular Rubygem (version), it would be nice if we could convey the age of a particular gem version.
Example:
This change uses
time_ago_in_words
, and we pass in the authored_at timestamp.Considerations:
I'm not 100% confident in all the translations. I used Google Translate; it might help to have additional people review it.