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

update rounding rules for months/years in Duration #245

Merged
merged 3 commits into from
Feb 2, 2023

Conversation

keithamus
Copy link
Member

Currently the main release of relative time uses a 90% rounding rule for dates.

Relative times are colloquial and so every user interprets them in subtly different ways. There are a few techniques we have used in the past:

  • We go on the literal date unit. So at 12:01AM a post from 2 minutes prior would say "a day ago". A post on the 31st December, when read on 1st Jan, would say "last year", which feels inaccurate.
  • We round units to some amount, so if the delta is 2 minutes we say "2 minutes ago" until minutes can be rounded up to hours at which point we can say "1 hour ago". We can pick the threshold for this...
    • A threshold of 50% would mean something 30 minutes ago would be "1 hour ago", and something 6 months ago would say "last year". Having a low threshold for larger durations seems inaccurate.
    • A threshold of 90% would mean something 54 minutes ago would be "1 hour ago", something 11 months ago would be "last year". The downside to this is that occasionally you'll get rounding such as something being 20 months old saying "last year". Having a high threshold seems inaccurate.

Right now the 90% threshold is our latest iteration but it has problems with large units of time, chiefly months and years. At the time of writing, for example, dates from April 2021 will be marked as "last year" despite being much closer to "2 years ago".

This PR changes the logic just for months and years to use a similar mechanism to that described in Temporal.Duration.round. We take the relativeTo which is a date, and we execute calendar math on the date object - working out the delta of the current month and rounding based on calendar year instead of a rounding of months.

This means for dates that have a delta of months or years, they will perform calendar operations, and those dates will be relative to the calendar year. In other words a date from 2022 read in 2023 will only ever read as "last year" and not "2 years ago", meanwhile a date from 2021 read in 2023 will never say "last year" and will always say "2 years ago".

@keithamus keithamus requested a review from a team as a code owner February 1, 2023 18:07
@keithamus keithamus force-pushed the update-rounding-rules-for-months-years-in-duration branch from c550eda to 86ae3a7 Compare February 1, 2023 18:07
@keithamus keithamus force-pushed the update-rounding-rules-for-months-years-in-duration branch from 86ae3a7 to 18e4c89 Compare February 1, 2023 18:26
@primer-css
Copy link

👋 Hello and thanks for pinging us! This issue or PR has been added to our inbox and a Design Infrastructure first responder will review it soon.

  • 🎨 If this is a PR that includes a visual change, please make sure to add screenshots in the description or deploy this code to a lab machine with instructions for how to test.
  • If this is a PR that includes changes to an interaction, please include a video recording in the description.
  • ⚠️ If this is urgent, please visit us in #primer on Slack and tag the first responders listed in the channel topic.

@keithamus keithamus force-pushed the update-rounding-rules-for-months-years-in-duration branch from 18e4c89 to 70ccf67 Compare February 1, 2023 18:43
Copy link

@rezrah rezrah left a comment

Choose a reason for hiding this comment

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

Looks good, and a nice improvement. Thanks @keithamus.

I couldn't get all tests to pass locally, but CI verified no regressions.

@keithamus keithamus merged commit 9302a83 into main Feb 2, 2023
@keithamus keithamus deleted the update-rounding-rules-for-months-years-in-duration branch February 2, 2023 15:55
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.

3 participants